[PATCH v2] libgcc: Use `-fasynchronous-unwind-tables' for LIB2_DIVMOD_FUNCS
Wed Aug 19 13:36:33 GMT 2020
On 19/08/2020 13:54, Maciej W. Rozycki via Gcc-patches wrote:
> On Tue, 18 Aug 2020, Richard Earnshaw 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.
>>> Remove the ARM overrides accordingly, retaining the hook infrastructure
>>> however, and make the ARM test case a generic one.
>> From a quick glance, I'm not convinced this is right for Arm, since the
>> Arm unwind format does not support anything other than call-based
>> exceptions. How did you test it?
> Surely no ARM target has been verified as I have no access to such
> configurations; I will appreciate if either you or someone else suitably
> equipped does that for the sake of cross-target code unification (= fewer
> special cases to maintain). This has been regression-tested with RISC-V
> and x86-64 targets, as noted in the two submissions.
> Are you trying to say that `-fasynchronous-unwind-tables' has no effect
> on ARM? This code does not throw exceptions, so any unwinding would only
> happen in contexts such as in GDB poking at this code or from a signal
> handler such as SIGALRM or SIGFPE (if ARM does ever send the latter signal
> for integer division operations; I don't know offhand). The GCC option is
> generic and is supposed to fully support such use cases regardless of the
> target chosen, so shouldn't the ARM backend be wired appropriately so as
> to use whatever unwind format is required to handle the use cases,
> regardless of whether the minimal format usually used is supported by the
> psABI or not?
> There is indeed a documented provision for not supporting the option:
> "Generate unwind table in DWARF format, if supported by target machine."
> however I infer that refers to the support of the DWARF format as a whole
> rather than specifically minimal unwind tables, that is if the DWARF
> format is supported (as opposed to say stabs or mdebug only), then the
> option shall generate an unwind table in that format.
> That said I'm of course happy to keep the ARM overrides if you consider
> them still necessary in the context of the generic change made. Let me
> know what you prefer, and if required, I will submit v3 with the ARM
> pieces removed.
So you've made a change to the Arm target, but not tested it. And
what's more didn't even bother to mention that fact.
If you make changes, you need to test them, particularly when there are
likely to be target-specific implications. If you can't test yourself
then you need to make that very clear in your submission.
There are Arm targets in the testfarm, so it's not really an excuse for
not doing testing.
More information about the Gcc-patches