diff mbox series

[v3] libgcc: Use `-fasynchronous-unwind-tables' for LIB2_DIVMOD_FUNCS

Message ID alpine.LFD.2.21.2008191443580.24175@redsun52.ssa.fujisawa.hgst.com
State New
Headers show
Series [v3] libgcc: Use `-fasynchronous-unwind-tables' for LIB2_DIVMOD_FUNCS | expand

Commit Message

Maciej W. Rozycki Aug. 20, 2020, 6:45 p.m. UTC
Complement commit b932f770f70d ("x86_64 frame unwind info"), SVN r46374, 
<https://gcc.gnu.org/ml/gcc-patches/2001-10/msg00860.html>, and replace 
`-fexceptions -fnon-call-exceptions' with `-fasynchronous-unwind-tables' 
in LIB2_DIVMOD_FUNCS compilation flags so as to provide unwind tables 
for the affected functions while not pulling the unwinder proper, which 
is not required here.

Beyond saving program space it fixes a RISC-V glibc build error due to 
unsatisfied `malloc' and `free' references from the unwinder causing 
link errors with `ld.so' where libgcc has been built at -O0.

	gcc/
	* testsuite/gcc.target/arm/div64-unwinding.c: Rename to...
	* testsuite/gcc.dg/div64-unwinding.c: ... this.

	libgcc/
	* Makefile.in [!LIB2_DIVMOD_EXCEPTION_FLAGS]
	(LIB2_DIVMOD_EXCEPTION_FLAGS): Replace `-fexceptions
	-fnon-call-exceptions' with `-fasynchronous-unwind-tables'.
---
Hi,

 No change from v2 except for the removal of the ARM parts; hence no need 
to retest.  OK to apply?

  Maciej

Changes from v2:

- Removal of the ARM overrides removed.

Changes from v1:

- ChangeLog entries added.
---
 gcc/testsuite/gcc.dg/div64-unwinding.c         |   25 +++++++++++++++++++++++++
 gcc/testsuite/gcc.target/arm/div64-unwinding.c |   25 -------------------------
 libgcc/Makefile.in                             |    2 +-
 3 files changed, 26 insertions(+), 26 deletions(-)

gcc-libgcc-divmod-asynchronous-unwind-tables.diff

Comments

Kito Cheng Aug. 25, 2020, 1:41 a.m. UTC | #1
Hi Maciej:

Thanks for your patch, I tried to compile all target libraries with
-O0 and got link error as you describe, it's really confusing about
the error message, __pthread_mutex_lock and __pthread_mutex_unlock are
undefined, it turns out the root cause is the __umoddi3 called
_Unwind_Resume and pull-in lots of functions, so really appreciate you
digging out this issue.

After figure out what happen and reproduce what you see, I spend times
to dig out the reason why -fexceptions -fnon-call-exceptions is set
for div and mod routines, it was introduce at many years ago[1] and no
much comment on why doing this, but I guess it's because some ISAs
might generate exception/trap when divide-by-0, and then the execution
environment will convert that to an exception, so those files/routines
need to compile with such flags.

However RISC-V never causes a trap when divide-by-0, so I believe
-fexceptions -fnon-call-exceptions could be removed safely for RISC-V,
but I am not sure it's safe for all other targets or not.

I would suggest you set LIB2_DIVMOD_EXCEPTION_FLAGS to empty or
'-fasynchronous-unwind-tables' for RISC-V port and do not change the
default one in libgcc/Makefile.in.

[1] https://github.com/gcc-mirror/gcc/commit/fc6aa0a98a0c7d10d39dd9d1485d0996b84b1860#diff-f98ad72e54bdf47d90acbfafa467d802R132


On Fri, Aug 21, 2020 at 2:46 AM Maciej W. Rozycki via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Complement commit b932f770f70d ("x86_64 frame unwind info"), SVN r46374,
> <https://gcc.gnu.org/ml/gcc-patches/2001-10/msg00860.html>, and replace
> `-fexceptions -fnon-call-exceptions' with `-fasynchronous-unwind-tables'
> in LIB2_DIVMOD_FUNCS compilation flags so as to provide unwind tables
> for the affected functions while not pulling the unwinder proper, which
> is not required here.
>
> Beyond saving program space it fixes a RISC-V glibc build error due to
> unsatisfied `malloc' and `free' references from the unwinder causing
> link errors with `ld.so' where libgcc has been built at -O0.
>
>         gcc/
>         * testsuite/gcc.target/arm/div64-unwinding.c: Rename to...
>         * testsuite/gcc.dg/div64-unwinding.c: ... this.
>
>         libgcc/
>         * Makefile.in [!LIB2_DIVMOD_EXCEPTION_FLAGS]
>         (LIB2_DIVMOD_EXCEPTION_FLAGS): Replace `-fexceptions
>         -fnon-call-exceptions' with `-fasynchronous-unwind-tables'.
> ---
> Hi,
>
>  No change from v2 except for the removal of the ARM parts; hence no need
> to retest.  OK to apply?
>
>   Maciej
>
> Changes from v2:
>
> - Removal of the ARM overrides removed.
>
> Changes from v1:
>
> - ChangeLog entries added.
> ---
>  gcc/testsuite/gcc.dg/div64-unwinding.c         |   25 +++++++++++++++++++++++++
>  gcc/testsuite/gcc.target/arm/div64-unwinding.c |   25 -------------------------
>  libgcc/Makefile.in                             |    2 +-
>  3 files changed, 26 insertions(+), 26 deletions(-)
>
> gcc-libgcc-divmod-asynchronous-unwind-tables.diff
> Index: gcc/gcc/testsuite/gcc.dg/div64-unwinding.c
> ===================================================================
> --- /dev/null
> +++ gcc/gcc/testsuite/gcc.dg/div64-unwinding.c
> @@ -0,0 +1,25 @@
> +/* Performing a 64-bit division should not pull in the unwinder.  */
> +
> +/* { dg-do run { target { { ! *-*-linux* } && { ! *-*-uclinux* } } } } */
> +/* { dg-skip-if "load causes weak symbol resolution" { vxworks_kernel } } */
> +/* { dg-options "-O0" } */
> +
> +#include <stdlib.h>
> +
> +long long
> +foo (long long c, long long d)
> +{
> +  return c/d;
> +}
> +
> +long long x = 0;
> +long long y = 1;
> +
> +extern int (*_Unwind_RaiseException) (void *) __attribute__((weak));
> +
> +int main(void)
> +{
> +  if (&_Unwind_RaiseException != NULL)
> +    abort ();;
> +  return foo (x, y);
> +}
> Index: gcc/gcc/testsuite/gcc.target/arm/div64-unwinding.c
> ===================================================================
> --- gcc.orig/gcc/testsuite/gcc.target/arm/div64-unwinding.c
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -/* Performing a 64-bit division should not pull in the unwinder.  */
> -
> -/* { dg-do run { target { { ! *-*-linux* } && { ! *-*-uclinux* } } } } */
> -/* { dg-skip-if "load causes weak symbol resolution" { vxworks_kernel } } */
> -/* { dg-options "-O0" } */
> -
> -#include <stdlib.h>
> -
> -long long
> -foo (long long c, long long d)
> -{
> -  return c/d;
> -}
> -
> -long long x = 0;
> -long long y = 1;
> -
> -extern int (*_Unwind_RaiseException) (void *) __attribute__((weak));
> -
> -int main(void)
> -{
> -  if (&_Unwind_RaiseException != NULL)
> -    abort ();;
> -  return foo (x, y);
> -}
> Index: gcc/libgcc/Makefile.in
> ===================================================================
> --- gcc.orig/libgcc/Makefile.in
> +++ gcc/libgcc/Makefile.in
> @@ -533,7 +533,7 @@ endif
>  ifeq ($(LIB2_DIVMOD_EXCEPTION_FLAGS),)
>  # Provide default flags for compiling divmod functions, if they haven't been
>  # set already by a target-specific Makefile fragment.
> -LIB2_DIVMOD_EXCEPTION_FLAGS := -fexceptions -fnon-call-exceptions
> +LIB2_DIVMOD_EXCEPTION_FLAGS := -fasynchronous-unwind-tables
>  endif
>
>  # Build LIB2_DIVMOD_FUNCS.
Kito Cheng Aug. 25, 2020, 9:29 a.m. UTC | #2
Hi Maciej:

I just found the mail thread about div mod with -fnon-call-exceptions,
I think keeping the default LIB2_DIVMOD_EXCEPTION_FLAGS unchanged
should be the best way to go.

Non-call exceptions and libcalls
https://gcc.gnu.org/legacy-ml/gcc/2001-06/msg01108.html

Non-call exceptions and libcalls Part 2
https://gcc.gnu.org/legacy-ml/gcc/2001-07/msg00402.html


On Tue, Aug 25, 2020 at 9:41 AM Kito Cheng <kito.cheng@gmail.com> wrote:
>
> Hi Maciej:
>
> Thanks for your patch, I tried to compile all target libraries with
> -O0 and got link error as you describe, it's really confusing about
> the error message, __pthread_mutex_lock and __pthread_mutex_unlock are
> undefined, it turns out the root cause is the __umoddi3 called
> _Unwind_Resume and pull-in lots of functions, so really appreciate you
> digging out this issue.
>
> After figure out what happen and reproduce what you see, I spend times
> to dig out the reason why -fexceptions -fnon-call-exceptions is set
> for div and mod routines, it was introduce at many years ago[1] and no
> much comment on why doing this, but I guess it's because some ISAs
> might generate exception/trap when divide-by-0, and then the execution
> environment will convert that to an exception, so those files/routines
> need to compile with such flags.
>
> However RISC-V never causes a trap when divide-by-0, so I believe
> -fexceptions -fnon-call-exceptions could be removed safely for RISC-V,
> but I am not sure it's safe for all other targets or not.
>
> I would suggest you set LIB2_DIVMOD_EXCEPTION_FLAGS to empty or
> '-fasynchronous-unwind-tables' for RISC-V port and do not change the
> default one in libgcc/Makefile.in.
>
> [1] https://github.com/gcc-mirror/gcc/commit/fc6aa0a98a0c7d10d39dd9d1485d0996b84b1860#diff-f98ad72e54bdf47d90acbfafa467d802R132
>
>
> On Fri, Aug 21, 2020 at 2:46 AM Maciej W. Rozycki via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Complement commit b932f770f70d ("x86_64 frame unwind info"), SVN r46374,
> > <https://gcc.gnu.org/ml/gcc-patches/2001-10/msg00860.html>, and replace
> > `-fexceptions -fnon-call-exceptions' with `-fasynchronous-unwind-tables'
> > in LIB2_DIVMOD_FUNCS compilation flags so as to provide unwind tables
> > for the affected functions while not pulling the unwinder proper, which
> > is not required here.
> >
> > Beyond saving program space it fixes a RISC-V glibc build error due to
> > unsatisfied `malloc' and `free' references from the unwinder causing
> > link errors with `ld.so' where libgcc has been built at -O0.
> >
> >         gcc/
> >         * testsuite/gcc.target/arm/div64-unwinding.c: Rename to...
> >         * testsuite/gcc.dg/div64-unwinding.c: ... this.
> >
> >         libgcc/
> >         * Makefile.in [!LIB2_DIVMOD_EXCEPTION_FLAGS]
> >         (LIB2_DIVMOD_EXCEPTION_FLAGS): Replace `-fexceptions
> >         -fnon-call-exceptions' with `-fasynchronous-unwind-tables'.
> > ---
> > Hi,
> >
> >  No change from v2 except for the removal of the ARM parts; hence no need
> > to retest.  OK to apply?
> >
> >   Maciej
> >
> > Changes from v2:
> >
> > - Removal of the ARM overrides removed.
> >
> > Changes from v1:
> >
> > - ChangeLog entries added.
> > ---
> >  gcc/testsuite/gcc.dg/div64-unwinding.c         |   25 +++++++++++++++++++++++++
> >  gcc/testsuite/gcc.target/arm/div64-unwinding.c |   25 -------------------------
> >  libgcc/Makefile.in                             |    2 +-
> >  3 files changed, 26 insertions(+), 26 deletions(-)
> >
> > gcc-libgcc-divmod-asynchronous-unwind-tables.diff
> > Index: gcc/gcc/testsuite/gcc.dg/div64-unwinding.c
> > ===================================================================
> > --- /dev/null
> > +++ gcc/gcc/testsuite/gcc.dg/div64-unwinding.c
> > @@ -0,0 +1,25 @@
> > +/* Performing a 64-bit division should not pull in the unwinder.  */
> > +
> > +/* { dg-do run { target { { ! *-*-linux* } && { ! *-*-uclinux* } } } } */
> > +/* { dg-skip-if "load causes weak symbol resolution" { vxworks_kernel } } */
> > +/* { dg-options "-O0" } */
> > +
> > +#include <stdlib.h>
> > +
> > +long long
> > +foo (long long c, long long d)
> > +{
> > +  return c/d;
> > +}
> > +
> > +long long x = 0;
> > +long long y = 1;
> > +
> > +extern int (*_Unwind_RaiseException) (void *) __attribute__((weak));
> > +
> > +int main(void)
> > +{
> > +  if (&_Unwind_RaiseException != NULL)
> > +    abort ();;
> > +  return foo (x, y);
> > +}
> > Index: gcc/gcc/testsuite/gcc.target/arm/div64-unwinding.c
> > ===================================================================
> > --- gcc.orig/gcc/testsuite/gcc.target/arm/div64-unwinding.c
> > +++ /dev/null
> > @@ -1,25 +0,0 @@
> > -/* Performing a 64-bit division should not pull in the unwinder.  */
> > -
> > -/* { dg-do run { target { { ! *-*-linux* } && { ! *-*-uclinux* } } } } */
> > -/* { dg-skip-if "load causes weak symbol resolution" { vxworks_kernel } } */
> > -/* { dg-options "-O0" } */
> > -
> > -#include <stdlib.h>
> > -
> > -long long
> > -foo (long long c, long long d)
> > -{
> > -  return c/d;
> > -}
> > -
> > -long long x = 0;
> > -long long y = 1;
> > -
> > -extern int (*_Unwind_RaiseException) (void *) __attribute__((weak));
> > -
> > -int main(void)
> > -{
> > -  if (&_Unwind_RaiseException != NULL)
> > -    abort ();;
> > -  return foo (x, y);
> > -}
> > Index: gcc/libgcc/Makefile.in
> > ===================================================================
> > --- gcc.orig/libgcc/Makefile.in
> > +++ gcc/libgcc/Makefile.in
> > @@ -533,7 +533,7 @@ endif
> >  ifeq ($(LIB2_DIVMOD_EXCEPTION_FLAGS),)
> >  # Provide default flags for compiling divmod functions, if they haven't been
> >  # set already by a target-specific Makefile fragment.
> > -LIB2_DIVMOD_EXCEPTION_FLAGS := -fexceptions -fnon-call-exceptions
> > +LIB2_DIVMOD_EXCEPTION_FLAGS := -fasynchronous-unwind-tables
> >  endif
> >
> >  # Build LIB2_DIVMOD_FUNCS.
Maciej W. Rozycki Aug. 25, 2020, 4:32 p.m. UTC | #3
Hi Kito,

> I just found the mail thread about div mod with -fnon-call-exceptions,
> I think keeping the default LIB2_DIVMOD_EXCEPTION_FLAGS unchanged
> should be the best way to go.
> 
> Non-call exceptions and libcalls
> https://gcc.gnu.org/legacy-ml/gcc/2001-06/msg01108.html
> 
> Non-call exceptions and libcalls Part 2
> https://gcc.gnu.org/legacy-ml/gcc/2001-07/msg00402.html

 Thank you for your input.  I believe I had a look at these commits before 
I posted my original proposal.  Please note however that they both predate 
the addition of `-fasynchronous-unwind-tables', so clearly the option 
could not have been considered at the time the changes were accepted into 
GCC.

 Please note that, as observed by Andreas and Richard here: 
<https://gcc.gnu.org/pipermail/gcc/2020-July/233122.html> in no case we 
want to have full exception handling here, so we clearly need no 
`-fexceptions'; this libcall code won't itself ever call `throw'.

 Now it might be a bit unclear from documentation as to whether we want 
`-fnon-call-exceptions' or `-fasynchronous-unwind-tables', as it says that 
the former option makes GCC:

"    Generate code that allows trapping instructions to throw
     exceptions.  Note that this requires platform-specific runtime
     support that does not exist everywhere.  Moreover, it only allows
     _trapping_ instructions to throw exceptions, i.e. memory references
     or floating-point instructions.  It does not allow exceptions to be
     thrown from arbitrary signal handlers such as 'SIGALRM'."

Note the observation that arbitrary signal handlers (invoked at more inner 
a frame level, and necessarily built with `-fexceptions') are still not 
allowed to throw exceptions.  For that, as far as I understand it, you 
actually need `-fasynchronous-unwind-tables', which makes GCC:

"    Generate unwind table in DWARF format, if supported by target
     machine.  The table is exact at each instruction boundary, so it
     can be used for stack unwinding from asynchronous events (such as
     debugger or garbage collector)."

and therefore allows arbitrary signal handlers to throw exceptions, 
effectively making the option a superset of `-fexceptions'.  As libcall 
code can generally be implicitly invoked everywhere, we want people not to 
be restrained by it and let a exception thrown by e.g. a user-supplied 
SIGALRM handler propagate through the relevant libcall's stack frame, 
rather than just those exceptions the libcall itself might indirectly 
cause.

 Maybe I am missing something here, especially as `-fexceptions' mentions 
code generation, while `-fasynchronous-unwind-tables' only refers to 
unwind table generation, but then what would be the option to allow 
exceptions to be thrown from arbitrary signal handlers rather than those 
for memory references or floating-point instructions (where by a special 
provision integer division falls as well)?

 My understanding has been it is `-fasynchronous-unwind-tables', but I'll 
be gladly straightened out otherwise.  If I am indeed right, then perhaps 
the documentation could be clarified and expanded a bit.

 Barring evidence to the contrary I maintain the change I have proposed is 
correct, and not only removes the RISC-V `ld.so' build issue, but it fixes 
the handling of asynchronous events arriving in the middle of the relevant 
libcalls for all platforms as well.

 Please let me know if you have any further questions, comments or 
concerns.

  Maciej
Kito Cheng Aug. 26, 2020, 3:42 a.m. UTC | #4
Hi Maciej:

Thanks for your explanation, I am OK with this change for the RISC-V port now,
but I think I don't have permission to approve this patch since it's
more than RISC-V port specific,
maybe you need approval from Richard Biener.


On Wed, Aug 26, 2020 at 12:33 AM Maciej W. Rozycki via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi Kito,
>
> > I just found the mail thread about div mod with -fnon-call-exceptions,
> > I think keeping the default LIB2_DIVMOD_EXCEPTION_FLAGS unchanged
> > should be the best way to go.
> >
> > Non-call exceptions and libcalls
> > https://gcc.gnu.org/legacy-ml/gcc/2001-06/msg01108.html
> >
> > Non-call exceptions and libcalls Part 2
> > https://gcc.gnu.org/legacy-ml/gcc/2001-07/msg00402.html
>
>  Thank you for your input.  I believe I had a look at these commits before
> I posted my original proposal.  Please note however that they both predate
> the addition of `-fasynchronous-unwind-tables', so clearly the option
> could not have been considered at the time the changes were accepted into
> GCC.
>
>  Please note that, as observed by Andreas and Richard here:
> <https://gcc.gnu.org/pipermail/gcc/2020-July/233122.html> in no case we
> want to have full exception handling here, so we clearly need no
> `-fexceptions'; this libcall code won't itself ever call `throw'.
>
>  Now it might be a bit unclear from documentation as to whether we want
> `-fnon-call-exceptions' or `-fasynchronous-unwind-tables', as it says that
> the former option makes GCC:
>
> "    Generate code that allows trapping instructions to throw
>      exceptions.  Note that this requires platform-specific runtime
>      support that does not exist everywhere.  Moreover, it only allows
>      _trapping_ instructions to throw exceptions, i.e. memory references
>      or floating-point instructions.  It does not allow exceptions to be
>      thrown from arbitrary signal handlers such as 'SIGALRM'."
>
> Note the observation that arbitrary signal handlers (invoked at more inner
> a frame level, and necessarily built with `-fexceptions') are still not
> allowed to throw exceptions.  For that, as far as I understand it, you
> actually need `-fasynchronous-unwind-tables', which makes GCC:
>
> "    Generate unwind table in DWARF format, if supported by target
>      machine.  The table is exact at each instruction boundary, so it
>      can be used for stack unwinding from asynchronous events (such as
>      debugger or garbage collector)."
>
> and therefore allows arbitrary signal handlers to throw exceptions,
> effectively making the option a superset of `-fexceptions'.  As libcall
> code can generally be implicitly invoked everywhere, we want people not to
> be restrained by it and let a exception thrown by e.g. a user-supplied
> SIGALRM handler propagate through the relevant libcall's stack frame,
> rather than just those exceptions the libcall itself might indirectly
> cause.
>
>  Maybe I am missing something here, especially as `-fexceptions' mentions
> code generation, while `-fasynchronous-unwind-tables' only refers to
> unwind table generation, but then what would be the option to allow
> exceptions to be thrown from arbitrary signal handlers rather than those
> for memory references or floating-point instructions (where by a special
> provision integer division falls as well)?
>
>  My understanding has been it is `-fasynchronous-unwind-tables', but I'll
> be gladly straightened out otherwise.  If I am indeed right, then perhaps
> the documentation could be clarified and expanded a bit.
>
>  Barring evidence to the contrary I maintain the change I have proposed is
> correct, and not only removes the RISC-V `ld.so' build issue, but it fixes
> the handling of asynchronous events arriving in the middle of the relevant
> libcalls for all platforms as well.
>
>  Please let me know if you have any further questions, comments or
> concerns.
>
>   Maciej
Richard Biener Aug. 26, 2020, 11:08 a.m. UTC | #5
On Tue, Aug 25, 2020 at 6:32 PM Maciej W. Rozycki <macro@wdc.com> wrote:
>
> Hi Kito,
>
> > I just found the mail thread about div mod with -fnon-call-exceptions,
> > I think keeping the default LIB2_DIVMOD_EXCEPTION_FLAGS unchanged
> > should be the best way to go.
> >
> > Non-call exceptions and libcalls
> > https://gcc.gnu.org/legacy-ml/gcc/2001-06/msg01108.html
> >
> > Non-call exceptions and libcalls Part 2
> > https://gcc.gnu.org/legacy-ml/gcc/2001-07/msg00402.html
>
>  Thank you for your input.  I believe I had a look at these commits before
> I posted my original proposal.  Please note however that they both predate
> the addition of `-fasynchronous-unwind-tables', so clearly the option
> could not have been considered at the time the changes were accepted into
> GCC.
>
>  Please note that, as observed by Andreas and Richard here:
> <https://gcc.gnu.org/pipermail/gcc/2020-July/233122.html> in no case we
> want to have full exception handling here, so we clearly need no
> `-fexceptions'; this libcall code won't itself ever call `throw'.
>
>  Now it might be a bit unclear from documentation as to whether we want
> `-fnon-call-exceptions' or `-fasynchronous-unwind-tables', as it says that
> the former option makes GCC:
>
> "    Generate code that allows trapping instructions to throw
>      exceptions.  Note that this requires platform-specific runtime
>      support that does not exist everywhere.  Moreover, it only allows
>      _trapping_ instructions to throw exceptions, i.e. memory references
>      or floating-point instructions.  It does not allow exceptions to be
>      thrown from arbitrary signal handlers such as 'SIGALRM'."
>
> Note the observation that arbitrary signal handlers (invoked at more inner
> a frame level, and necessarily built with `-fexceptions') are still not
> allowed to throw exceptions.  For that, as far as I understand it, you
> actually need `-fasynchronous-unwind-tables', which makes GCC:
>
> "    Generate unwind table in DWARF format, if supported by target
>      machine.  The table is exact at each instruction boundary, so it
>      can be used for stack unwinding from asynchronous events (such as
>      debugger or garbage collector)."
>
> and therefore allows arbitrary signal handlers to throw exceptions,
> effectively making the option a superset of `-fexceptions'.  As libcall
> code can generally be implicitly invoked everywhere, we want people not to
> be restrained by it and let a exception thrown by e.g. a user-supplied
> SIGALRM handler propagate through the relevant libcall's stack frame,
> rather than just those exceptions the libcall itself might indirectly
> cause.
>
>  Maybe I am missing something here, especially as `-fexceptions' mentions
> code generation, while `-fasynchronous-unwind-tables' only refers to
> unwind table generation, but then what would be the option to allow
> exceptions to be thrown from arbitrary signal handlers rather than those
> for memory references or floating-point instructions (where by a special
> provision integer division falls as well)?
>
>  My understanding has been it is `-fasynchronous-unwind-tables', but I'll
> be gladly straightened out otherwise.  If I am indeed right, then perhaps
> the documentation could be clarified and expanded a bit.
>
>  Barring evidence to the contrary I maintain the change I have proposed is
> correct, and not only removes the RISC-V `ld.so' build issue, but it fixes
> the handling of asynchronous events arriving in the middle of the relevant
> libcalls for all platforms as well.
>
>  Please let me know if you have any further questions, comments or
> concerns.

You only need -fexceptions for that, then you can throw; from a signal handler
for example.  If you want to be able to catch the exception somewhere up
the call chain all intermediate code needs to be compiled so that unwinding
from asynchronous events is possible - -fasynchronous-unwind-tables.

So -fasynchronous-unwind-tables is about unwinding.  -f[non-call]-exceptions
is about throw/catch.  Clearly libgcc does neither throw nor catch but with
async events we might need to unwind from inside it.

Now I don't know about the arm situation but if arm cannot do async unwinding
then even -fexceptions won't help it here - libgcc still does not throw.

Richard.

>
>   Maciej
Jakub Jelinek Aug. 26, 2020, 11:33 a.m. UTC | #6
On Wed, Aug 26, 2020 at 01:08:00PM +0200, Richard Biener via Gcc-patches wrote:
> You only need -fexceptions for that, then you can throw; from a signal handler
> for example.  If you want to be able to catch the exception somewhere up
> the call chain all intermediate code needs to be compiled so that unwinding
> from asynchronous events is possible - -fasynchronous-unwind-tables.
> 
> So -fasynchronous-unwind-tables is about unwinding.  -f[non-call]-exceptions
> is about throw/catch.  Clearly libgcc does neither throw nor catch but with
> async events we might need to unwind from inside it.

In C code -f{,non-call-}exceptions is also about whether cleanup attribute
will work or not.  But I think in libgcc we don't really use it, especially
not in the division/modulo code, not even indirectly from libc headers like
pthread_cleanup_* macros.

	Jakub
Maciej W. Rozycki Aug. 28, 2020, 3:40 p.m. UTC | #7
On Wed, 26 Aug 2020, Jakub Jelinek wrote:

> On Wed, Aug 26, 2020 at 01:08:00PM +0200, Richard Biener via Gcc-patches wrote:
> > You only need -fexceptions for that, then you can throw; from a signal handler
> > for example.  If you want to be able to catch the exception somewhere up
> > the call chain all intermediate code needs to be compiled so that unwinding
> > from asynchronous events is possible - -fasynchronous-unwind-tables.
> > 
> > So -fasynchronous-unwind-tables is about unwinding.  -f[non-call]-exceptions
> > is about throw/catch.  Clearly libgcc does neither throw nor catch but with
> > async events we might need to unwind from inside it.

 Thank you for the clarification.

 As far as `-fexceptions' and `-fasynchronous-unwind-tables' are concerned 
it aligns with my understanding, i.e. in this specific scenario we need 
`-fasynchronous-unwind-tables' for libcalls (all of them) so that an 
exception thrown from a signal handler can unwind through our code, and it 
is the signal handler that needs `-fexceptions' to be able to throw an 
exception, and then another piece of code upwards the call chain also 
needs `-fexceptions' to catch it.

 I'm still unclear as to `-fnon-call-exceptions' as what you write,
combined with documentation for said option seems to imply that it causes
a signal handler to be installed for SIGBUS/SIGSEGV/SIGFPE, however I seem
unable to locate such code except for the Ada frontend.

 The option does cause certain instruction classes, as per `may_trap_p_1', 
to be annotated for exception handling and prevent them from being 
optimised away, but it is not clear to me what the difference is between 
the case where a piece of code has been built with `-fnon-call-exceptions' 
and `-fasynchronous-unwind-tables', and an instruction within (that hasn't 
been optimised away) traps into an exception handler that throws an 
exception.  I.e. what the extra annotation is used for (obviously being 
optimised away or not is a significat difference).

 IIUC in both cases the exact causing instruction can be identified, 
especially as I note that internally `-fnon-call-exceptions' implies 
`-fasynchronous-unwind-tables':

  if (flag_non_call_exceptions)
    flag_asynchronous_unwind_tables = 1;

(which I suspect might be worth documenting).  

> In C code -f{,non-call-}exceptions is also about whether cleanup attribute
> will work or not.  But I think in libgcc we don't really use it, especially
> not in the division/modulo code, not even indirectly from libc headers like
> pthread_cleanup_* macros.

 Thank you for your observation, I didn't know of the cleanup attribute 
before.

 So what shall we do about my patch?

  Maciej
Ramana Radhakrishnan Aug. 28, 2020, 3:47 p.m. UTC | #8
On Wed, Aug 26, 2020 at 12:08 PM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Tue, Aug 25, 2020 at 6:32 PM Maciej W. Rozycki <macro@wdc.com> wrote:
> >
> > Hi Kito,
> >
> > > I just found the mail thread about div mod with -fnon-call-exceptions,
> > > I think keeping the default LIB2_DIVMOD_EXCEPTION_FLAGS unchanged
> > > should be the best way to go.
> > >
> > > Non-call exceptions and libcalls
> > > https://gcc.gnu.org/legacy-ml/gcc/2001-06/msg01108.html
> > >
> > > Non-call exceptions and libcalls Part 2
> > > https://gcc.gnu.org/legacy-ml/gcc/2001-07/msg00402.html
> >
> >  Thank you for your input.  I believe I had a look at these commits before
> > I posted my original proposal.  Please note however that they both predate
> > the addition of `-fasynchronous-unwind-tables', so clearly the option
> > could not have been considered at the time the changes were accepted into
> > GCC.
> >
> >  Please note that, as observed by Andreas and Richard here:
> > <https://gcc.gnu.org/pipermail/gcc/2020-July/233122.html> in no case we
> > want to have full exception handling here, so we clearly need no
> > `-fexceptions'; this libcall code won't itself ever call `throw'.
> >
> >  Now it might be a bit unclear from documentation as to whether we want
> > `-fnon-call-exceptions' or `-fasynchronous-unwind-tables', as it says that
> > the former option makes GCC:
> >
> > "    Generate code that allows trapping instructions to throw
> >      exceptions.  Note that this requires platform-specific runtime
> >      support that does not exist everywhere.  Moreover, it only allows
> >      _trapping_ instructions to throw exceptions, i.e. memory references
> >      or floating-point instructions.  It does not allow exceptions to be
> >      thrown from arbitrary signal handlers such as 'SIGALRM'."
> >
> > Note the observation that arbitrary signal handlers (invoked at more inner
> > a frame level, and necessarily built with `-fexceptions') are still not
> > allowed to throw exceptions.  For that, as far as I understand it, you
> > actually need `-fasynchronous-unwind-tables', which makes GCC:
> >
> > "    Generate unwind table in DWARF format, if supported by target
> >      machine.  The table is exact at each instruction boundary, so it
> >      can be used for stack unwinding from asynchronous events (such as
> >      debugger or garbage collector)."
> >
> > and therefore allows arbitrary signal handlers to throw exceptions,
> > effectively making the option a superset of `-fexceptions'.  As libcall
> > code can generally be implicitly invoked everywhere, we want people not to
> > be restrained by it and let a exception thrown by e.g. a user-supplied
> > SIGALRM handler propagate through the relevant libcall's stack frame,
> > rather than just those exceptions the libcall itself might indirectly
> > cause.
> >
> >  Maybe I am missing something here, especially as `-fexceptions' mentions
> > code generation, while `-fasynchronous-unwind-tables' only refers to
> > unwind table generation, but then what would be the option to allow
> > exceptions to be thrown from arbitrary signal handlers rather than those
> > for memory references or floating-point instructions (where by a special
> > provision integer division falls as well)?
> >
> >  My understanding has been it is `-fasynchronous-unwind-tables', but I'll
> > be gladly straightened out otherwise.  If I am indeed right, then perhaps
> > the documentation could be clarified and expanded a bit.
> >
> >  Barring evidence to the contrary I maintain the change I have proposed is
> > correct, and not only removes the RISC-V `ld.so' build issue, but it fixes
> > the handling of asynchronous events arriving in the middle of the relevant
> > libcalls for all platforms as well.
> >
> >  Please let me know if you have any further questions, comments or
> > concerns.
>
> You only need -fexceptions for that, then you can throw; from a signal handler
> for example.  If you want to be able to catch the exception somewhere up
> the call chain all intermediate code needs to be compiled so that unwinding
> from asynchronous events is possible - -fasynchronous-unwind-tables.
>
> So -fasynchronous-unwind-tables is about unwinding.  -f[non-call]-exceptions
> is about throw/catch.  Clearly libgcc does neither throw nor catch but with
> async events we might need to unwind from inside it.
>
> Now I don't know about the arm situation but if arm cannot do async unwinding
> then even -fexceptions won't help it here - libgcc still does not throw.

On Arm as in the AArch32 port,  async unwinding will not work as those
can't be expressed in the EH format tables.

regards
Ramana

>
> Richard.
>
> >
> >   Maciej
Jakub Jelinek Aug. 28, 2020, 5:04 p.m. UTC | #9
On Fri, Aug 28, 2020 at 04:40:48PM +0100, Maciej W. Rozycki wrote:
>  As far as `-fexceptions' and `-fasynchronous-unwind-tables' are concerned 
> it aligns with my understanding, i.e. in this specific scenario we need 
> `-fasynchronous-unwind-tables' for libcalls (all of them) so that an 

-fasynchronous-unwind-tables is solely about whether one can unwind through
the code, but not necessarily that EH will work.  If it is not turned on,
non-call exceptions will certainly not work properly, because unwind info
if present at all will only be correct on call insns and not guaranteed on
anything else.

-fexceptions enables EH handling in whatever function it is turned on,
entries in .gcc_except_table as well as a personality routine will be added
for it, calls from it which can throw (including special calls like throw)
will be marked there and cause EH regions, similarly EH pads will be added
for anything where an exception needs to be caught or where a cleanup needs
to be run before continuing with the unwinding (e.g. destructors or cleanup
attribute).  But still, it only expects that call can throw, not arbitrary
instructions, so those won't be covered necessarily in EH regions, in the IL
there won't be EH edges etc.

-fnon-call-exceptions is like -fexceptions, except in addition to calls
(that may throw) it considers also possibly trapping instructions as
something that can throw.  So way too many more EH edges, EH regions etc.

Now, I'd say if for some trapping instruction in a function we want to throw
an exception from a signal handler, surely the signal handler needs to be
built with -fexceptions, but as long as the function that contains the
trapping instruction doesn't really need to catch the exception (including
running a cleanup in it and continuing with the exception), i.e. when the
exception is thrown from the signal handler and caught in some caller of the
function with the trapping instruction, I'd say all that needs to be done
is the function with the trapping instruction be compiled with
-fasynchronous-unwind-tables and the callers of it that are still not meant
to catch it should be compiled with -funwind-tables (or
-fasynchronous-unwind-tables) and only the function that should catch it or
run cleanups should be compiled with -fexceptions (no need for
-fnon-call-exceptions in that case, but of course not disallowed).
This is for DWARF EH, I have no idea how ARM, or IA64 EH works, so it might
be different for those.

So, if Ada has testsuite coverage for EH throwing on division or modulo by
zero and your patch doesn't break Ada on at least some common DWARF using target
and ARM (let's ignore IA64), I'd say replacing
LIB2_DIVMOD_EXCEPTION_FLAGS := -fexceptions -fnon-call-exceptions
with
LIB2_DIVMOD_EXCEPTION_FLAGS := -fasynchronous-unwind-tables
looks reasonable to me.

	Jakub
Richard Biener Aug. 31, 2020, 8:04 a.m. UTC | #10
On Fri, Aug 28, 2020 at 5:47 PM Ramana Radhakrishnan
<ramana.gcc@googlemail.com> wrote:
>
> On Wed, Aug 26, 2020 at 12:08 PM Richard Biener via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > On Tue, Aug 25, 2020 at 6:32 PM Maciej W. Rozycki <macro@wdc.com> wrote:
> > >
> > > Hi Kito,
> > >
> > > > I just found the mail thread about div mod with -fnon-call-exceptions,
> > > > I think keeping the default LIB2_DIVMOD_EXCEPTION_FLAGS unchanged
> > > > should be the best way to go.
> > > >
> > > > Non-call exceptions and libcalls
> > > > https://gcc.gnu.org/legacy-ml/gcc/2001-06/msg01108.html
> > > >
> > > > Non-call exceptions and libcalls Part 2
> > > > https://gcc.gnu.org/legacy-ml/gcc/2001-07/msg00402.html
> > >
> > >  Thank you for your input.  I believe I had a look at these commits before
> > > I posted my original proposal.  Please note however that they both predate
> > > the addition of `-fasynchronous-unwind-tables', so clearly the option
> > > could not have been considered at the time the changes were accepted into
> > > GCC.
> > >
> > >  Please note that, as observed by Andreas and Richard here:
> > > <https://gcc.gnu.org/pipermail/gcc/2020-July/233122.html> in no case we
> > > want to have full exception handling here, so we clearly need no
> > > `-fexceptions'; this libcall code won't itself ever call `throw'.
> > >
> > >  Now it might be a bit unclear from documentation as to whether we want
> > > `-fnon-call-exceptions' or `-fasynchronous-unwind-tables', as it says that
> > > the former option makes GCC:
> > >
> > > "    Generate code that allows trapping instructions to throw
> > >      exceptions.  Note that this requires platform-specific runtime
> > >      support that does not exist everywhere.  Moreover, it only allows
> > >      _trapping_ instructions to throw exceptions, i.e. memory references
> > >      or floating-point instructions.  It does not allow exceptions to be
> > >      thrown from arbitrary signal handlers such as 'SIGALRM'."
> > >
> > > Note the observation that arbitrary signal handlers (invoked at more inner
> > > a frame level, and necessarily built with `-fexceptions') are still not
> > > allowed to throw exceptions.  For that, as far as I understand it, you
> > > actually need `-fasynchronous-unwind-tables', which makes GCC:
> > >
> > > "    Generate unwind table in DWARF format, if supported by target
> > >      machine.  The table is exact at each instruction boundary, so it
> > >      can be used for stack unwinding from asynchronous events (such as
> > >      debugger or garbage collector)."
> > >
> > > and therefore allows arbitrary signal handlers to throw exceptions,
> > > effectively making the option a superset of `-fexceptions'.  As libcall
> > > code can generally be implicitly invoked everywhere, we want people not to
> > > be restrained by it and let a exception thrown by e.g. a user-supplied
> > > SIGALRM handler propagate through the relevant libcall's stack frame,
> > > rather than just those exceptions the libcall itself might indirectly
> > > cause.
> > >
> > >  Maybe I am missing something here, especially as `-fexceptions' mentions
> > > code generation, while `-fasynchronous-unwind-tables' only refers to
> > > unwind table generation, but then what would be the option to allow
> > > exceptions to be thrown from arbitrary signal handlers rather than those
> > > for memory references or floating-point instructions (where by a special
> > > provision integer division falls as well)?
> > >
> > >  My understanding has been it is `-fasynchronous-unwind-tables', but I'll
> > > be gladly straightened out otherwise.  If I am indeed right, then perhaps
> > > the documentation could be clarified and expanded a bit.
> > >
> > >  Barring evidence to the contrary I maintain the change I have proposed is
> > > correct, and not only removes the RISC-V `ld.so' build issue, but it fixes
> > > the handling of asynchronous events arriving in the middle of the relevant
> > > libcalls for all platforms as well.
> > >
> > >  Please let me know if you have any further questions, comments or
> > > concerns.
> >
> > You only need -fexceptions for that, then you can throw; from a signal handler
> > for example.  If you want to be able to catch the exception somewhere up
> > the call chain all intermediate code needs to be compiled so that unwinding
> > from asynchronous events is possible - -fasynchronous-unwind-tables.
> >
> > So -fasynchronous-unwind-tables is about unwinding.  -f[non-call]-exceptions
> > is about throw/catch.  Clearly libgcc does neither throw nor catch but with
> > async events we might need to unwind from inside it.
> >
> > Now I don't know about the arm situation but if arm cannot do async unwinding
> > then even -fexceptions won't help it here - libgcc still does not throw.
>
> On Arm as in the AArch32 port,  async unwinding will not work as those
> can't be expressed in the EH format tables.

And surely building libgcc with -fexceptions does not change that either.

Richard.

> regards
> Ramana
>
> >
> > Richard.
> >
> > >
> > >   Maciej
Maciej W. Rozycki Aug. 31, 2020, 3:26 p.m. UTC | #11
On Fri, 28 Aug 2020, Jakub Jelinek wrote:

> >  As far as `-fexceptions' and `-fasynchronous-unwind-tables' are concerned 
> > it aligns with my understanding, i.e. in this specific scenario we need 
> > `-fasynchronous-unwind-tables' for libcalls (all of them) so that an 
> 
> -fasynchronous-unwind-tables is solely about whether one can unwind through
> the code, but not necessarily that EH will work.  If it is not turned on,
> non-call exceptions will certainly not work properly, because unwind info
> if present at all will only be correct on call insns and not guaranteed on
> anything else.

 Ack, this is what my understanding has been.

> -fexceptions enables EH handling in whatever function it is turned on,
> entries in .gcc_except_table as well as a personality routine will be added
> for it, calls from it which can throw (including special calls like throw)
> will be marked there and cause EH regions, similarly EH pads will be added
> for anything where an exception needs to be caught or where a cleanup needs
> to be run before continuing with the unwinding (e.g. destructors or cleanup
> attribute).  But still, it only expects that call can throw, not arbitrary
> instructions, so those won't be covered necessarily in EH regions, in the IL
> there won't be EH edges etc.
> 
> -fnon-call-exceptions is like -fexceptions, except in addition to calls
> (that may throw) it considers also possibly trapping instructions as
> something that can throw.  So way too many more EH edges, EH regions etc.

 OK, I believe I have realised what we do here.

 Obviously there may be multiple `try'/`catch' blocks in a program, and in 
a given function even.  Catching an exception essentially means matching 
the PC recorded for execution resumption (the return PC of a callee or a 
signal handler, which may come from various places according to the psABI 
in use, like the link register, a stack slot in the frame below, etc.) 
against the PC range(s) associated with a `try' block and passing control 
to the associated `catch' block.  For that obviously a set of exception 
data tables is required.

 Now, where `-fexceptions' only has been used we only record return PCs of 
call instructions in exception data tables, rather than recording the PC 
span of the whole `try' block, which in the course of optimisation might 
have been split into multiple disjoint ranges.  This is I gather to reduce 
the size of the tables and consequently the amount of processing required 
in EH, but the downside is an exception triggering at another place still 
within the `try' block will not be caught and will instead be passed up 
the frame chain.

 So where we want to also catch exceptions on instructions other than 
calls the `-fnon-call-exceptions' option can be used, in which case PC 
values associated with instructions from additional classes as per 
`may_trap_p_1' will be added to exception tables, making them larger.

 Still there is no way to catch exceptions on absolutely all instructions 
within a `try' block, as we have no option for that, IIUC, but the caller 
of the enclosing function can with the use of a `try' block around the 
call.

 I wasn't aware we have such optimisations/simplifications/restrictions in 
place and I honestly thought a `try' block was always completely covered.  
That is what I remember language semantics implied, but it has been long 
since I last looked at that, and no doubt standards have evolved and may 
have given leeway for compilers to only partially cover `try' blocks as 
they see fit for performance gain.  It is not clear from the description 
of `-fexceptions' or `-fnon-call-exceptions' options in our manual either.

 Perhaps it is general knowledge supposed to be held by software 
developers that I somehow missed previously, but it seems to me like it 
wouldn't hurt if it was actually mentioned in the description of said 
options, so if you agree with me, than I'll think and propose an update.

> Now, I'd say if for some trapping instruction in a function we want to throw
> an exception from a signal handler, surely the signal handler needs to be
> built with -fexceptions, but as long as the function that contains the
> trapping instruction doesn't really need to catch the exception (including
> running a cleanup in it and continuing with the exception), i.e. when the
> exception is thrown from the signal handler and caught in some caller of the
> function with the trapping instruction, I'd say all that needs to be done
> is the function with the trapping instruction be compiled with
> -fasynchronous-unwind-tables and the callers of it that are still not meant
> to catch it should be compiled with -funwind-tables (or
> -fasynchronous-unwind-tables) and only the function that should catch it or
> run cleanups should be compiled with -fexceptions (no need for
> -fnon-call-exceptions in that case, but of course not disallowed).

 Right, given what you have written and my observations above, and that 
the libcalls affected neither throw nor catch exceptions themselves, I 
conclude my change is correct.

> This is for DWARF EH, I have no idea how ARM, or IA64 EH works, so it might
> be different for those.

 Ack.

> So, if Ada has testsuite coverage for EH throwing on division or modulo by
> zero and your patch doesn't break Ada on at least some common DWARF using target
> and ARM (let's ignore IA64), I'd say replacing
> LIB2_DIVMOD_EXCEPTION_FLAGS := -fexceptions -fnon-call-exceptions
> with
> LIB2_DIVMOD_EXCEPTION_FLAGS := -fasynchronous-unwind-tables
> looks reasonable to me.

 Unfortunately today is my last day at Western Digital (and a bank holiday 
in England even) so regrettably I won't be able to run such investigation 
or verification, not at least in the foreseeable future.  Perhaps someone 
else can, and I will surely appreciate that, as it looks to me like a move 
in the right direction.

 NB v3 of the change does not affect ARM, leaving that target's options 
intact, so I suppose no ARM verification will be actually required.

  Maciej
diff mbox series

Patch

Index: gcc/gcc/testsuite/gcc.dg/div64-unwinding.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.dg/div64-unwinding.c
@@ -0,0 +1,25 @@ 
+/* Performing a 64-bit division should not pull in the unwinder.  */
+
+/* { dg-do run { target { { ! *-*-linux* } && { ! *-*-uclinux* } } } } */
+/* { dg-skip-if "load causes weak symbol resolution" { vxworks_kernel } } */
+/* { dg-options "-O0" } */
+
+#include <stdlib.h>
+
+long long
+foo (long long c, long long d)
+{
+  return c/d;
+}
+
+long long x = 0;
+long long y = 1;
+
+extern int (*_Unwind_RaiseException) (void *) __attribute__((weak));
+
+int main(void)
+{
+  if (&_Unwind_RaiseException != NULL)
+    abort ();;
+  return foo (x, y);
+}
Index: gcc/gcc/testsuite/gcc.target/arm/div64-unwinding.c
===================================================================
--- gcc.orig/gcc/testsuite/gcc.target/arm/div64-unwinding.c
+++ /dev/null
@@ -1,25 +0,0 @@ 
-/* Performing a 64-bit division should not pull in the unwinder.  */
-
-/* { dg-do run { target { { ! *-*-linux* } && { ! *-*-uclinux* } } } } */
-/* { dg-skip-if "load causes weak symbol resolution" { vxworks_kernel } } */
-/* { dg-options "-O0" } */
-
-#include <stdlib.h>
-
-long long
-foo (long long c, long long d)
-{
-  return c/d;
-}
-
-long long x = 0;
-long long y = 1;
-
-extern int (*_Unwind_RaiseException) (void *) __attribute__((weak));
-
-int main(void)
-{
-  if (&_Unwind_RaiseException != NULL)
-    abort ();;
-  return foo (x, y);
-}
Index: gcc/libgcc/Makefile.in
===================================================================
--- gcc.orig/libgcc/Makefile.in
+++ gcc/libgcc/Makefile.in
@@ -533,7 +533,7 @@  endif
 ifeq ($(LIB2_DIVMOD_EXCEPTION_FLAGS),)
 # Provide default flags for compiling divmod functions, if they haven't been
 # set already by a target-specific Makefile fragment.
-LIB2_DIVMOD_EXCEPTION_FLAGS := -fexceptions -fnon-call-exceptions
+LIB2_DIVMOD_EXCEPTION_FLAGS := -fasynchronous-unwind-tables
 endif
 
 # Build LIB2_DIVMOD_FUNCS.