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

[Maciej W. Rozycki]

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