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

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

[Sergei Trofimovich]

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 <tony.luck@intel.com>
CC: Fenghua Yu <fenghua.yu@intel.com>
CC: linux-ia64@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Bug: https://github.com/strace/strace/issues/33
Bug: https://gcc.gnu.org/PR84184
Reported-by: Émeric Maschino <emeric.maschino@gmail.com>
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
 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

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

[Sergei Trofimovich]

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 <tony.luck@intel.com>
CC: Fenghua Yu <fenghua.yu@intel.com>
CC: linux-ia64@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Bug: https://github.com/strace/strace/issues/33
Bug: https://gcc.gnu.org/PR84184
Reported-by: Émeric Maschino <emeric.maschino@gmail.com>
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
 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

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

[Luck, Tony]

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

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

[Luck, Tony]

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

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

[Sergei Trofimovich]

On Fri, 2 Feb 2018 14:22:32 -0800
"Luck, Tony" <tony.luck@intel.com> 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

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

[Sergei Trofimovich]

On Fri, 2 Feb 2018 14:22:32 -0800
"Luck, Tony" <tony.luck@intel.com> 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

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

[Sergei Trofimovich]

On Fri, 2 Feb 2018 23:02:20 +0000
Sergei Trofimovich <slyfox@gentoo.org> wrote:

> On Fri, 2 Feb 2018 14:22:32 -0800
> "Luck, Tony" <tony.luck@intel.com> 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: stanton_arch@mail.com

-- 

  Sergei

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

[Sergei Trofimovich]

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 <tony.luck@intel.com>
CC: Fenghua Yu <fenghua.yu@intel.com>
CC: linux-ia64@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Bug: https://github.com/strace/strace/issues/33
Bug: https://gcc.gnu.org/PR84184
Reported-by: Émeric Maschino <emeric.maschino@gmail.com>
Tested-by: stanton_arch@mail.com
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
 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

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

[Sergei Trofimovich]

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 <tony.luck@intel.com>
CC: Fenghua Yu <fenghua.yu@intel.com>
CC: linux-ia64@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Bug: https://github.com/strace/strace/issues/33
Bug: https://gcc.gnu.org/PR84184
Reported-by: Émeric Maschino <emeric.maschino@gmail.com>
Tested-by: stanton_arch@mail.com
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
 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

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

[Sergei Trofimovich]

On Fri,  9 Mar 2018 23:15:55 +0000
Sergei Trofimovich <slyfox@gentoo.org> 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

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

[Sergei Trofimovich]

On Fri,  9 Mar 2018 23:15:55 +0000
Sergei Trofimovich <slyfox@gentoo.org> 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

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

[John Paul Adrian Glaubitz]

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 - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

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

[John Paul Adrian Glaubitz]

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 - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

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

[John Paul Adrian Glaubitz]

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†984

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

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

[John Paul Adrian Glaubitz]

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 - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913