2018-02-02 22:45:41

by Sergei Trofimovich

[permalink] [raw]
Subject: [PATCH] ia64: fix ptrace(PTRACE_GETREGS) (unbreaks strace, gdb)

The strace breakage looks like that:
./strace: get_regs: get_regs_error: Input/output error

It happens because ia64 needs to load unwind tables
to read certain registers. Unwind tables fail to load
due to GCC quirk on the following code:

extern char __end_unwind[];
const struct unw_table_entry *end = (struct unw_table_entry *)table_end;
table->end = segment_base + end[-1].end_offset;

GCC does not generate correct code for this single memory
reference after constant propagation (see https://gcc.gnu.org/PR84184).
Two triggers are required for bad code generation:
- '__end_unwind' has alignment lower (char), than
'struct unw_table_entry' (8).
- symbol offset is negative.

This commit workarounds it by fixing alignment of '__end_unwind'.
While at it use hidden symbols to generate shorter gp-relative
relocations.

CC: Tony Luck <[email protected]>
CC: Fenghua Yu <[email protected]>
CC: [email protected]
CC: [email protected]
Bug: https://github.com/strace/strace/issues/33
Bug: https://gcc.gnu.org/PR84184
Reported-by: Émeric Maschino <[email protected]>
Signed-off-by: Sergei Trofimovich <[email protected]>
---
arch/ia64/include/asm/sections.h | 1 -
arch/ia64/kernel/unwind.c | 15 ++++++++++++++-
2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index f3481408594e..0fc4f1757a44 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -24,7 +24,6 @@ extern char __start_gate_mckinley_e9_patchlist[], __end_gate_mckinley_e9_patchli
extern char __start_gate_vtop_patchlist[], __end_gate_vtop_patchlist[];
extern char __start_gate_fsyscall_patchlist[], __end_gate_fsyscall_patchlist[];
extern char __start_gate_brl_fsys_bubble_down_patchlist[], __end_gate_brl_fsys_bubble_down_patchlist[];
-extern char __start_unwind[], __end_unwind[];
extern char __start_ivt_text[], __end_ivt_text[];

#undef dereference_function_descriptor
diff --git a/arch/ia64/kernel/unwind.c b/arch/ia64/kernel/unwind.c
index e04efa088902..025ba6700790 100644
--- a/arch/ia64/kernel/unwind.c
+++ b/arch/ia64/kernel/unwind.c
@@ -2243,7 +2243,20 @@ __initcall(create_gate_table);
void __init
unw_init (void)
{
- extern char __gp[];
+ #define __ia64_hidden __attribute__((visibility("hidden")))
+ /*
+ * We use hidden symbols to generate more efficient code using
+ * gp-relative addressing.
+ */
+ extern char __gp[] __ia64_hidden;
+ /*
+ * Unwind tables need to have proper alignment as init_unwind_table()
+ * uses negative offsets against '__end_unwind'.
+ * See https://gcc.gnu.org/PR84184
+ */
+ extern const struct unw_table_entry __start_unwind[] __ia64_hidden;
+ extern const struct unw_table_entry __end_unwind[] __ia64_hidden;
+ #undef __ia64_hidden
extern void unw_hash_index_t_is_too_narrow (void);
long i, off;

--
2.16.1



2018-02-02 22:51:46

by Luck, Tony

[permalink] [raw]
Subject: Re: [PATCH] ia64: fix ptrace(PTRACE_GETREGS) (unbreaks strace, gdb)

On Fri, Feb 02, 2018 at 10:12:24PM +0000, Sergei Trofimovich wrote:
> The strace breakage looks like that:
> ./strace: get_regs: get_regs_error: Input/output error
>
> It happens because ia64 needs to load unwind tables
> to read certain registers. Unwind tables fail to load
> due to GCC quirk on the following code:
>
> extern char __end_unwind[];
> const struct unw_table_entry *end = (struct unw_table_entry *)table_end;
> table->end = segment_base + end[-1].end_offset;
>
> GCC does not generate correct code for this single memory
> reference after constant propagation (see https://gcc.gnu.org/PR84184).

I'm not seeing this ... probably because I build with
a pre-historic 4.3.4 version of gcc.

Do you know which version(s) are affected? I'm not looking
for an exhaustive list, just the one on which you found this
would be good.

-Tony

2018-02-03 00:55:24

by Sergei Trofimovich

[permalink] [raw]
Subject: Re: [PATCH] ia64: fix ptrace(PTRACE_GETREGS) (unbreaks strace, gdb)

On Fri, 2 Feb 2018 14:22:32 -0800
"Luck, Tony" <[email protected]> wrote:

> On Fri, Feb 02, 2018 at 10:12:24PM +0000, Sergei Trofimovich wrote:
> > The strace breakage looks like that:
> > ./strace: get_regs: get_regs_error: Input/output error
> >
> > It happens because ia64 needs to load unwind tables
> > to read certain registers. Unwind tables fail to load
> > due to GCC quirk on the following code:
> >
> > extern char __end_unwind[];
> > const struct unw_table_entry *end = (struct unw_table_entry *)table_end;
> > table->end = segment_base + end[-1].end_offset;
> >
> > GCC does not generate correct code for this single memory
> > reference after constant propagation (see https://gcc.gnu.org/PR84184).
>
> I'm not seeing this ... probably because I build with
> a pre-historic 4.3.4 version of gcc.
>
> Do you know which version(s) are affected? I'm not looking
> for an exhaustive list, just the one on which you found this
> would be good.
>
> -Tony

Original bug https://bugs.gentoo.org/518130 claims regression appeared
around gcc-4.5. Locally am seeing the problem with gcc-6.4.0, gcc-7.2.0 and
gcc-8 (HEAD).

--

Sergei

2018-02-11 18:54:13

by Sergei Trofimovich

[permalink] [raw]
Subject: Re: [PATCH] ia64: fix ptrace(PTRACE_GETREGS) (unbreaks strace, gdb)

On Fri, 2 Feb 2018 23:02:20 +0000
Sergei Trofimovich <[email protected]> wrote:

> On Fri, 2 Feb 2018 14:22:32 -0800
> "Luck, Tony" <[email protected]> wrote:
>
> > On Fri, Feb 02, 2018 at 10:12:24PM +0000, Sergei Trofimovich wrote:
> > > The strace breakage looks like that:
> > > ./strace: get_regs: get_regs_error: Input/output error
> > >
> > > It happens because ia64 needs to load unwind tables
> > > to read certain registers. Unwind tables fail to load
> > > due to GCC quirk on the following code:
> > >
> > > extern char __end_unwind[];
> > > const struct unw_table_entry *end = (struct unw_table_entry *)table_end;
> > > table->end = segment_base + end[-1].end_offset;
> > >
> > > GCC does not generate correct code for this single memory
> > > reference after constant propagation (see https://gcc.gnu.org/PR84184).
> >
> > I'm not seeing this ... probably because I build with
> > a pre-historic 4.3.4 version of gcc.
> >
> > Do you know which version(s) are affected? I'm not looking
> > for an exhaustive list, just the one on which you found this
> > would be good.
> >
> > -Tony
>
> Original bug https://bugs.gentoo.org/518130 claims regression appeared
> around gcc-4.5. Locally am seeing the problem with gcc-6.4.0, gcc-7.2.0 and
> gcc-8 (HEAD).

Another report on the positive patch effect:

rx2600 boots successfully with this patch (did not without, my guess is due to
early access fault at bad address): https://bugs.gentoo.org/579278#c13

Tested-by: [email protected]

--

Sergei

2018-03-09 23:17:19

by Sergei Trofimovich

[permalink] [raw]
Subject: [PATCH v2, simpler] ia64: fix ptrace(PTRACE_GETREGS) (unbreaks strace, gdb)

The strace breakage looks like that:
./strace: get_regs: get_regs_error: Input/output error

It happens because ia64 needs to load unwind tables
to read certain registers in 'PTRACE_GETREGS'. Unwind
tables fail to load at kernel startup due to GCC quirk
on the following code (logged as PR 84184):

extern char __end_unwind[];
const struct unw_table_entry *end = (struct unw_table_entry *)table_end;
table->end = segment_base + end[-1].end_offset;

GCC does not generate correct code for this single memory
reference after constant propagation.
Two triggers are required for bad code generation:
- '__end_unwind' has alignment lower (char), than
'struct unw_table_entry' (8).
- symbol offset is negative.

This commit workarounds it by disabling inline on
init_unwind_table(). This way we avoid const-propagation
of '__end_unwind' and pass address via register.

Tested in ski (emulator) and on rx2600, rx3600 (real hardware).
In case of rx2600 it unbreaks booting.

This patch is a lighter version of patch
https://lkml.org/lkml/2018/2/2/914

CC: Tony Luck <[email protected]>
CC: Fenghua Yu <[email protected]>
CC: [email protected]
CC: [email protected]
Bug: https://github.com/strace/strace/issues/33
Bug: https://gcc.gnu.org/PR84184
Reported-by: Émeric Maschino <[email protected]>
Tested-by: [email protected]
Signed-off-by: Sergei Trofimovich <[email protected]>
---
arch/ia64/kernel/unwind.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/ia64/kernel/unwind.c b/arch/ia64/kernel/unwind.c
index e04efa088902..a18190bc99a9 100644
--- a/arch/ia64/kernel/unwind.c
+++ b/arch/ia64/kernel/unwind.c
@@ -2078,7 +2078,14 @@ unw_init_from_blocked_task (struct unw_frame_info *info, struct task_struct *t)
}
EXPORT_SYMBOL(unw_init_from_blocked_task);

-static void
+/*
+ * We use 'noinline' to evade GCC bug https://gcc.gnu.org/PR84184
+ * where gcc code generator emits incorrect code when '__end_unwind'
+ * is const-propagated to 'end[-1].end_offset' and gcc generates
+ * incorrect code. The prigger there is negative offset relative
+ * to externally-defined symbol.
+ */
+noinline static void
init_unwind_table (struct unw_table *table, const char *name, unsigned long segment_base,
unsigned long gp, const void *table_start, const void *table_end)
{
--
2.16.2


2018-08-04 21:34:45

by Sergei Trofimovich

[permalink] [raw]
Subject: Re: [PATCH v2, simpler] ia64: fix ptrace(PTRACE_GETREGS) (unbreaks strace, gdb)

On Fri, 9 Mar 2018 23:15:55 +0000
Sergei Trofimovich <[email protected]> wrote:

I tried to explain in more detail breakage mechanics
of unwinder and gcc code generation quirks at:
https://trofi.github.io/posts/210-ptrace-and-accidental-boot-fix-on-ia64.html
Hopefully it gives better intuition of code change
caused by both proposed patches.

I personally think v1 patch is slightly more robust.

--

Sergei

Subject: Re: [PATCH] ia64: fix ptrace(PTRACE_GETREGS) (unbreaks strace, gdb)

Hello!

Is there a chance that Sergey's patch from [1] could get applied upstream?

Currently, the kernel in Debian doesn't boot on the RX2800 and crashes very
early directly after the bootloader. I can also confirm that both strace
and gdb currently don't build on Debian/ia64 due to this particular bug.

Thanks,
Adrian

> [1] https://lkml.org/lkml/2018/2/2/914

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

Subject: Re: [PATCH] ia64: fix ptrace(PTRACE_GETREGS) (unbreaks strace, gdb)

Hi!

On 4/29/19 12:38 PM, John Paul Adrian Glaubitz wrote:
> Is there a chance that Sergey's patch from [1] could get applied upstream?

Apparently it's sufficient to rebuild the kernel with a gcc which has
this [1] fix applied. Will give this a try and report back.

Adrian

> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86984

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913