[PATCH] arm: Fix up arm installed unwind.h for use in pedantic modes [PR93615]

[PATCH] arm: Fix up arm installed unwind.h for use in pedantic modes [PR93615]

[Jakub Jelinek]

Hi!

As the following testcase shows, unwind.h on ARM can't be (starting with GCC
10) compiled with -std=c* modes, only -std=gnu* modes.
The problem is it uses asm keyword, which isn't a keyword in those modes
(system headers vs. non-system ones don't make a difference here).
glibc and other installed headers use __asm or __asm__ keywords instead that
work fine in both standard and gnu modes.

While there, as it is an installed header, I think it is also wrong to
completely ignore any identifier namespace rules.
The generic unwind.h defines just _Unwind* namespace identifiers plus
_sleb128_t/_uleb128_t (but e.g. unlike libstdc++/glibc headers doesn't
uglify operand names), the ARM unwind.h is much worse here.  I've just
changed the gnu_Unwind_Find_got function at least not be in user identifier
namespace, but perhaps it would be good to go further and rename e.g.
#define UNWIND_STACK_REG 13
#define UNWIND_POINTER_REG 12
#define FDPIC_REGNUM 9
#define STR(x) #x
#define XSTR(x) STR(x)
or e.g.
  typedef _Unwind_Reason_Code (*personality_routine) (_Unwind_State,
      _Unwind_Control_Block *, _Unwind_Context *);
in unwind-arm-common.h.

Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?

2020-02-07  Jakub Jelinek  <jakub@redhat.com>

	PR target/93615
	* config/arm/unwind-arm.h (gnu_Unwind_Find_got): Rename to ...
	(_Unwind_gnu_Find_got): ... this.  Use __asm instead of asm.  Remove
	trailing :s in asm.  Formatting fixes.
	(_Unwind_decode_typeinfo_ptr): Adjust caller.

	* gcc.dg/pr93615.c: New test.

--- libgcc/config/arm/unwind-arm.h.jj	2020-01-12 11:54:38.616380172 +0100
+++ libgcc/config/arm/unwind-arm.h	2020-02-06 16:16:54.244624408 +0100
@@ -43,19 +43,15 @@ extern "C" {
 #endif
 _Unwind_Ptr __attribute__((weak)) __gnu_Unwind_Find_got (_Unwind_Ptr);
 
-static inline _Unwind_Ptr gnu_Unwind_Find_got (_Unwind_Ptr ptr)
+static inline _Unwind_Ptr _Unwind_gnu_Find_got (_Unwind_Ptr ptr)
 {
     _Unwind_Ptr res;
 
     if (__gnu_Unwind_Find_got)
-	res =  __gnu_Unwind_Find_got (ptr);
+	res = __gnu_Unwind_Find_got (ptr);
     else
-      {
-	asm volatile ("mov %[result], r" XSTR(FDPIC_REGNUM)
-		      : [result]"=r" (res)
-		      :
-		      :);
-      }
+	__asm volatile ("mov %[result], r" XSTR(FDPIC_REGNUM)
+			: [result] "=r" (res));
 
     return res;
 }
@@ -75,7 +71,7 @@ static inline _Unwind_Ptr gnu_Unwind_Fin
 #if __FDPIC__
       /* For FDPIC, we store the offset of the GOT entry.  */
       /* So, first get GOT from dynamic linker and then use indirect access.  */
-      tmp += gnu_Unwind_Find_got (ptr);
+      tmp += _Unwind_gnu_Find_got (ptr);
       tmp = *(_Unwind_Word *) tmp;
 #elif (defined(linux) && !defined(__uClinux__)) || defined(__NetBSD__) \
     || defined(__FreeBSD__) || defined(__fuchsia__)
--- gcc/testsuite/gcc.dg/pr93615.c.jj	2020-02-06 22:40:00.921472574 +0100
+++ gcc/testsuite/gcc.dg/pr93615.c	2020-02-06 22:39:52.937591443 +0100
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-std=c11" } */
+/* { dg-require-effective-target exceptions } */
+
+#include <unwind.h>
+
+int
+main ()
+{
+  return 0;
+}

	Jakub

Re: [PATCH] arm: Fix up arm installed unwind.h for use in pedantic modes [PR93615]

[Ramana Radhakrishnan]

On Fri, Feb 7, 2020 at 8:19 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> As the following testcase shows, unwind.h on ARM can't be (starting with GCC
> 10) compiled with -std=c* modes, only -std=gnu* modes.
> The problem is it uses asm keyword, which isn't a keyword in those modes
> (system headers vs. non-system ones don't make a difference here).
> glibc and other installed headers use __asm or __asm__ keywords instead that
> work fine in both standard and gnu modes.
>
> While there, as it is an installed header, I think it is also wrong to
> completely ignore any identifier namespace rules.
> The generic unwind.h defines just _Unwind* namespace identifiers plus
> _sleb128_t/_uleb128_t (but e.g. unlike libstdc++/glibc headers doesn't
> uglify operand names), the ARM unwind.h is much worse here.  I've just
> changed the gnu_Unwind_Find_got function at least not be in user identifier
> namespace, but perhaps it would be good to go further and rename e.g.
> #define UNWIND_STACK_REG 13
> #define UNWIND_POINTER_REG 12
> #define FDPIC_REGNUM 9
> #define STR(x) #x
> #define XSTR(x) STR(x)
> or e.g.
>   typedef _Unwind_Reason_Code (*personality_routine) (_Unwind_State,
>       _Unwind_Control_Block *, _Unwind_Context *);
> in unwind-arm-common.h.
>
> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?
>
> 2020-02-07  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/93615
>         * config/arm/unwind-arm.h (gnu_Unwind_Find_got): Rename to ...
>         (_Unwind_gnu_Find_got): ... this.  Use __asm instead of asm.  Remove
>         trailing :s in asm.  Formatting fixes.
>         (_Unwind_decode_typeinfo_ptr): Adjust caller.
>
>         * gcc.dg/pr93615.c: New test.
>

Ok, thanks jakub

Ramana

> --- libgcc/config/arm/unwind-arm.h.jj   2020-01-12 11:54:38.616380172 +0100
> +++ libgcc/config/arm/unwind-arm.h      2020-02-06 16:16:54.244624408 +0100
> @@ -43,19 +43,15 @@ extern "C" {
>  #endif
>  _Unwind_Ptr __attribute__((weak)) __gnu_Unwind_Find_got (_Unwind_Ptr);
>
> -static inline _Unwind_Ptr gnu_Unwind_Find_got (_Unwind_Ptr ptr)
> +static inline _Unwind_Ptr _Unwind_gnu_Find_got (_Unwind_Ptr ptr)
>  {
>      _Unwind_Ptr res;
>
>      if (__gnu_Unwind_Find_got)
> -       res =  __gnu_Unwind_Find_got (ptr);
> +       res = __gnu_Unwind_Find_got (ptr);
>      else
> -      {
> -       asm volatile ("mov %[result], r" XSTR(FDPIC_REGNUM)
> -                     : [result]"=r" (res)
> -                     :
> -                     :);
> -      }
> +       __asm volatile ("mov %[result], r" XSTR(FDPIC_REGNUM)
> +                       : [result] "=r" (res));
>
>      return res;
>  }
> @@ -75,7 +71,7 @@ static inline _Unwind_Ptr gnu_Unwind_Fin
>  #if __FDPIC__
>        /* For FDPIC, we store the offset of the GOT entry.  */
>        /* So, first get GOT from dynamic linker and then use indirect access.  */
> -      tmp += gnu_Unwind_Find_got (ptr);
> +      tmp += _Unwind_gnu_Find_got (ptr);
>        tmp = *(_Unwind_Word *) tmp;
>  #elif (defined(linux) && !defined(__uClinux__)) || defined(__NetBSD__) \
>      || defined(__FreeBSD__) || defined(__fuchsia__)
> --- gcc/testsuite/gcc.dg/pr93615.c.jj   2020-02-06 22:40:00.921472574 +0100
> +++ gcc/testsuite/gcc.dg/pr93615.c      2020-02-06 22:39:52.937591443 +0100
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=c11" } */
> +/* { dg-require-effective-target exceptions } */
> +
> +#include <unwind.h>
> +
> +int
> +main ()
> +{
> +  return 0;
> +}
>
>         Jakub
>

Re: [PATCH] arm: Fix up arm installed unwind.h for use in pedantic modes [PR93615]

[Christophe Lyon]

[-- Attachment #1: Type: text/plain, Size: 6737 bytes --]

Hi,

On Fri, 7 Feb 2020 at 09:19, Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> As the following testcase shows, unwind.h on ARM can't be (starting with GCC
> 10) compiled with -std=c* modes, only -std=gnu* modes.
> The problem is it uses asm keyword, which isn't a keyword in those modes
> (system headers vs. non-system ones don't make a difference here).
> glibc and other installed headers use __asm or __asm__ keywords instead that
> work fine in both standard and gnu modes.
>
> While there, as it is an installed header, I think it is also wrong to
> completely ignore any identifier namespace rules.
> The generic unwind.h defines just _Unwind* namespace identifiers plus
> _sleb128_t/_uleb128_t (but e.g. unlike libstdc++/glibc headers doesn't
> uglify operand names), the ARM unwind.h is much worse here.  I've just
> changed the gnu_Unwind_Find_got function at least not be in user identifier
> namespace, but perhaps it would be good to go further and rename e.g.
> #define UNWIND_STACK_REG 13
> #define UNWIND_POINTER_REG 12
> #define FDPIC_REGNUM 9
> #define STR(x) #x
> #define XSTR(x) STR(x)
> or e.g.
>   typedef _Unwind_Reason_Code (*personality_routine) (_Unwind_State,
>       _Unwind_Control_Block *, _Unwind_Context *);
> in unwind-arm-common.h.
>
> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?
>
> 2020-02-07  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/93615
>         * config/arm/unwind-arm.h (gnu_Unwind_Find_got): Rename to ...
>         (_Unwind_gnu_Find_got): ... this.  Use __asm instead of asm.  Remove
>         trailing :s in asm.  Formatting fixes.
>         (_Unwind_decode_typeinfo_ptr): Adjust caller.
>
>         * gcc.dg/pr93615.c: New test.
>

This broke the build of GCC for the arm-none-uclinuxfdpiceabi target (FDPIC).

The attached patch fixes this by updating other uses of gnu_Unwind_Find_got.

OK?

Thanks,

Christophe

From 667227c7d3d5c3e471be02adc0ce7dc2c4ebfd25 Mon Sep 17 00:00:00 2001
From: Christophe Lyon <christophe.lyon@linaro.org>
Date: Mon, 10 Feb 2020 12:54:39 +0000
Subject: [PATCH 1/1] arm: Fix up arm installed unwind.h for use in pedantic
 modes [PR93615]

Commit r10-6500-g811a475ea3fcc55ee4aea7c81171891ef19dfc25 broke the
GCC build for arm-none-uclinuxfdpiceabi, as it forgot to update some
uses of gnu_Unwind_Find_got.

2020-02-10  Christophe Lyon  <christophe.lyon@linaro.org>

libgcc/
* unwind-arm-common.inc: Replace uses of gnu_Unwind_Find_got with
_Unwind_gnu_Find_got.
* unwind-pe.h: Likewise.
---
 libgcc/unwind-arm-common.inc | 8 ++++----
 libgcc/unwind-pe.h           | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libgcc/unwind-arm-common.inc b/libgcc/unwind-arm-common.inc
index 3c774b8..31a072b 100644
--- a/libgcc/unwind-arm-common.inc
+++ b/libgcc/unwind-arm-common.inc
@@ -419,7 +419,7 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw
return_address)
       UCB_PR_ADDR (ucbp) = selfrel_offset31 (ucbp->pr_cache.ehtp);
 #if __FDPIC__
       UCB_PR_GOT (ucbp)
- = (unsigned int) gnu_Unwind_Find_got ((_Unwind_Ptr) UCB_PR_ADDR (ucbp));
+ = (unsigned int) _Unwind_gnu_Find_got ((_Unwind_Ptr) UCB_PR_ADDR (ucbp));
 #endif
     }
   return _URC_OK;
@@ -462,7 +462,7 @@ unwind_phase2 (_Unwind_Control_Block * ucbp,
phase2_vrs * vrs)

 #if __FDPIC__
   /* r9 could have been lost due to PLT jump.  Restore correct value.  */
-  vrs->core.r[FDPIC_REGNUM] = gnu_Unwind_Find_got (VRS_PC (vrs));
+  vrs->core.r[FDPIC_REGNUM] = _Unwind_gnu_Find_got (VRS_PC (vrs));
 #endif

   uw_restore_core_regs (vrs, &vrs->core);
@@ -562,7 +562,7 @@ unwind_phase2_forced (_Unwind_Control_Block *ucbp,
phase2_vrs *entry_vrs,

 #if __FDPIC__
   /* r9 could have been lost due to PLT jump.  Restore correct value.  */
-  saved_vrs.core.r[FDPIC_REGNUM] = gnu_Unwind_Find_got (VRS_PC (&saved_vrs));
+  saved_vrs.core.r[FDPIC_REGNUM] = _Unwind_gnu_Find_got (VRS_PC (&saved_vrs));
 #endif

   uw_restore_core_regs (&saved_vrs, &saved_vrs.core);
@@ -698,7 +698,7 @@ __gnu_Unwind_Resume (_Unwind_Control_Block * ucbp,
phase2_vrs * entry_vrs)
       /* Upload the registers to enter the landing pad.  */
 #if __FDPIC__
       /* r9 could have been lost due to PLT jump.  Restore correct value.  */
-      entry_vrs->core.r[FDPIC_REGNUM] = gnu_Unwind_Find_got (VRS_PC
(entry_vrs));
+      entry_vrs->core.r[FDPIC_REGNUM] = _Unwind_gnu_Find_got (VRS_PC
(entry_vrs));
 #endif
       uw_restore_core_regs (entry_vrs, &entry_vrs->core);

diff --git a/libgcc/unwind-pe.h b/libgcc/unwind-pe.h
index a336127..a6b4bff 100644
--- a/libgcc/unwind-pe.h
+++ b/libgcc/unwind-pe.h
@@ -267,7 +267,7 @@ read_encoded_value_with_base (unsigned char
encoding, _Unwind_Ptr base,
       into account.  */
    if ((encoding & DW_EH_PE_pcrel) && (encoding & DW_EH_PE_indirect))
      {
-       result += gnu_Unwind_Find_got ((_Unwind_Ptr) u);
+       result += _Unwind_gnu_Find_got ((_Unwind_Ptr) u);
        result = *(_Unwind_Internal_Ptr *) result;
      }
    else
-- 
2.7.4


> --- libgcc/config/arm/unwind-arm.h.jj   2020-01-12 11:54:38.616380172 +0100
> +++ libgcc/config/arm/unwind-arm.h      2020-02-06 16:16:54.244624408 +0100
> @@ -43,19 +43,15 @@ extern "C" {
>  #endif
>  _Unwind_Ptr __attribute__((weak)) __gnu_Unwind_Find_got (_Unwind_Ptr);
>
> -static inline _Unwind_Ptr gnu_Unwind_Find_got (_Unwind_Ptr ptr)
> +static inline _Unwind_Ptr _Unwind_gnu_Find_got (_Unwind_Ptr ptr)
>  {
>      _Unwind_Ptr res;
>
>      if (__gnu_Unwind_Find_got)
> -       res =  __gnu_Unwind_Find_got (ptr);
> +       res = __gnu_Unwind_Find_got (ptr);
>      else
> -      {
> -       asm volatile ("mov %[result], r" XSTR(FDPIC_REGNUM)
> -                     : [result]"=r" (res)
> -                     :
> -                     :);
> -      }
> +       __asm volatile ("mov %[result], r" XSTR(FDPIC_REGNUM)
> +                       : [result] "=r" (res));
>
>      return res;
>  }
> @@ -75,7 +71,7 @@ static inline _Unwind_Ptr gnu_Unwind_Fin
>  #if __FDPIC__
>        /* For FDPIC, we store the offset of the GOT entry.  */
>        /* So, first get GOT from dynamic linker and then use indirect access.  */
> -      tmp += gnu_Unwind_Find_got (ptr);
> +      tmp += _Unwind_gnu_Find_got (ptr);
>        tmp = *(_Unwind_Word *) tmp;
>  #elif (defined(linux) && !defined(__uClinux__)) || defined(__NetBSD__) \
>      || defined(__FreeBSD__) || defined(__fuchsia__)
> --- gcc/testsuite/gcc.dg/pr93615.c.jj   2020-02-06 22:40:00.921472574 +0100
> +++ gcc/testsuite/gcc.dg/pr93615.c      2020-02-06 22:39:52.937591443 +0100
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=c11" } */
> +/* { dg-require-effective-target exceptions } */
> +
> +#include <unwind.h>
> +
> +int
> +main ()
> +{
> +  return 0;
> +}
>
>         Jakub
>

[-- Attachment #2: 0001-arm-Fix-up-arm-installed-unwind.h-for-use-in-pedanti.patch --]
[-- Type: text/x-patch, Size: 3072 bytes --]

From 667227c7d3d5c3e471be02adc0ce7dc2c4ebfd25 Mon Sep 17 00:00:00 2001
From: Christophe Lyon <christophe.lyon@linaro.org>
Date: Mon, 10 Feb 2020 12:54:39 +0000
Subject: [PATCH 1/1] arm: Fix up arm installed unwind.h for use in pedantic
 modes [PR93615]

Commit r10-6500-g811a475ea3fcc55ee4aea7c81171891ef19dfc25 broke the GCC build for arm-none-uclinuxfdpiceabi, as it forgot to update some uses of gnu_Unwind_Find_got.

2020-02-10  Christophe Lyon  <christophe.lyon@linaro.org>

	libgcc/
	* unwind-arm-common.inc: Replace uses of gnu_Unwind_Find_got with
	_Unwind_gnu_Find_got.
	* unwind-pe.h: Likewise.
---
 libgcc/unwind-arm-common.inc | 8 ++++----
 libgcc/unwind-pe.h           | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libgcc/unwind-arm-common.inc b/libgcc/unwind-arm-common.inc
index 3c774b8..31a072b 100644
--- a/libgcc/unwind-arm-common.inc
+++ b/libgcc/unwind-arm-common.inc
@@ -419,7 +419,7 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw return_address)
       UCB_PR_ADDR (ucbp) = selfrel_offset31 (ucbp->pr_cache.ehtp);
 #if __FDPIC__
       UCB_PR_GOT (ucbp)
-	= (unsigned int) gnu_Unwind_Find_got ((_Unwind_Ptr) UCB_PR_ADDR (ucbp));
+	= (unsigned int) _Unwind_gnu_Find_got ((_Unwind_Ptr) UCB_PR_ADDR (ucbp));
 #endif
     }
   return _URC_OK;
@@ -462,7 +462,7 @@ unwind_phase2 (_Unwind_Control_Block * ucbp, phase2_vrs * vrs)
 
 #if __FDPIC__
   /* r9 could have been lost due to PLT jump.  Restore correct value.  */
-  vrs->core.r[FDPIC_REGNUM] = gnu_Unwind_Find_got (VRS_PC (vrs));
+  vrs->core.r[FDPIC_REGNUM] = _Unwind_gnu_Find_got (VRS_PC (vrs));
 #endif
 
   uw_restore_core_regs (vrs, &vrs->core);
@@ -562,7 +562,7 @@ unwind_phase2_forced (_Unwind_Control_Block *ucbp, phase2_vrs *entry_vrs,
 
 #if __FDPIC__
   /* r9 could have been lost due to PLT jump.  Restore correct value.  */
-  saved_vrs.core.r[FDPIC_REGNUM] = gnu_Unwind_Find_got (VRS_PC (&saved_vrs));
+  saved_vrs.core.r[FDPIC_REGNUM] = _Unwind_gnu_Find_got (VRS_PC (&saved_vrs));
 #endif
 
   uw_restore_core_regs (&saved_vrs, &saved_vrs.core);
@@ -698,7 +698,7 @@ __gnu_Unwind_Resume (_Unwind_Control_Block * ucbp, phase2_vrs * entry_vrs)
       /* Upload the registers to enter the landing pad.  */
 #if __FDPIC__
       /* r9 could have been lost due to PLT jump.  Restore correct value.  */
-      entry_vrs->core.r[FDPIC_REGNUM] = gnu_Unwind_Find_got (VRS_PC (entry_vrs));
+      entry_vrs->core.r[FDPIC_REGNUM] = _Unwind_gnu_Find_got (VRS_PC (entry_vrs));
 #endif
       uw_restore_core_regs (entry_vrs, &entry_vrs->core);
 
diff --git a/libgcc/unwind-pe.h b/libgcc/unwind-pe.h
index a336127..a6b4bff 100644
--- a/libgcc/unwind-pe.h
+++ b/libgcc/unwind-pe.h
@@ -267,7 +267,7 @@ read_encoded_value_with_base (unsigned char encoding, _Unwind_Ptr base,
 	     into account.  */
 	  if ((encoding & DW_EH_PE_pcrel) && (encoding & DW_EH_PE_indirect))
 	    {
-	      result += gnu_Unwind_Find_got ((_Unwind_Ptr) u);
+	      result += _Unwind_gnu_Find_got ((_Unwind_Ptr) u);
 	      result = *(_Unwind_Internal_Ptr *) result;
 	    }
 	  else
-- 
2.7.4

Re: [PATCH] arm: Fix up arm installed unwind.h for use in pedantic modes [PR93615]

[Jakub Jelinek]

On Mon, Feb 10, 2020 at 02:03:47PM +0100, Christophe Lyon wrote:
> > 2020-02-07  Jakub Jelinek  <jakub@redhat.com>
> >
> >         PR target/93615
> >         * config/arm/unwind-arm.h (gnu_Unwind_Find_got): Rename to ...
> >         (_Unwind_gnu_Find_got): ... this.  Use __asm instead of asm.  Remove
> >         trailing :s in asm.  Formatting fixes.
> >         (_Unwind_decode_typeinfo_ptr): Adjust caller.
> >
> >         * gcc.dg/pr93615.c: New test.
> >
> 
> This broke the build of GCC for the arm-none-uclinuxfdpiceabi target (FDPIC).

Oops, sorry.

> The attached patch fixes this by updating other uses of gnu_Unwind_Find_got.
> 
> OK?

Looks obvious to me.

	Jakub

Re: [PATCH] arm: Fix up arm installed unwind.h for use in pedantic modes [PR93615]

[Christophe Lyon]

On Mon, 10 Feb 2020 at 14:26, Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Feb 10, 2020 at 02:03:47PM +0100, Christophe Lyon wrote:
> > > 2020-02-07  Jakub Jelinek  <jakub@redhat.com>
> > >
> > >         PR target/93615
> > >         * config/arm/unwind-arm.h (gnu_Unwind_Find_got): Rename to ...
> > >         (_Unwind_gnu_Find_got): ... this.  Use __asm instead of asm.  Remove
> > >         trailing :s in asm.  Formatting fixes.
> > >         (_Unwind_decode_typeinfo_ptr): Adjust caller.
> > >
> > >         * gcc.dg/pr93615.c: New test.
> > >
> >
> > This broke the build of GCC for the arm-none-uclinuxfdpiceabi target (FDPIC).
>
> Oops, sorry.
>
> > The attached patch fixes this by updating other uses of gnu_Unwind_Find_got.
> >
> > OK?
>
> Looks obvious to me.

OK, thanks, pushed as r10-6546-g5602b48b2ed84feb1a5792e3d73b00b42138ca6e

>
>         Jakub
>