On Fri, Feb 26, 2021 at 11:04 PM Iain Sandoe <i...@sandoe.co.uk> wrote: > > Hi > > This address one of the more long-standing and serious regressions > for Darwin. GCC emits unwind code by default on the assumption that > the unwinder will be (or have the same capability) as the one in the > current libgcc_s. For Darwin platforms, this is not the case - some > of them are based on the libgcc_s from GCC-4.2.1 and some are using > the unwinder provided by libunwind (part of the LLVM project). The > latter implementation has gradually adopted a section that deals with > GNU unwind. > > The most serious problem for some of the platform versions is in > handling DW_CFA_remember/restore_state pairs. The DWARF description > talks about these in terms of saving/restoring register rows; this is > what GCC originally did (and is what the unwinders do for the Darwin > versions based on libgcc_s). > > However, in r118068, this was changed so that not only the registers > but also the current frame address expression were saved. The unwind > code-gen assumes that the unwinder will do this; some of Darwin's unwinders > do not, leading to lockups etc. To date, the only solution has been to > replace > the system libgcc_s with a newer one which is not a usable solution for many > end-users (since that means overwritting the one provided with the system > installation). > > The fix here provides a target hook that allows the target to specify > that the CFA expression should be reinstated after a DW_CFA_restore. This > fixes the open PR (and also the closed WONTFIX of 44107). > > (As a matter of record, it also fixes reported Java issues if > backported to GCC-5). > > tested on *-darwin* and x86_64-linux-gnu, > OK for master / backports to open branches?
OK for master and branches after a while. Thanks, Richard. > thanks > Iain > > gcc/ChangeLog: > > PR target/44107 > PR target/48097 > * config/darwin-protos.h (darwin_should_restore_cfa_state): New. > * config/darwin.c (darwin_should_restore_cfa_state): New. > * config/darwin.h (TARGET_ASM_SHOULD_RESTORE_CFA_STATE): New. > * doc/tm.texi: Regenerated. > * doc/tm.texi.in: Document TARGET_ASM_SHOULD_RESTORE_CFA_STATE. > * dwarf2cfi.c (connect_traces): If the target requests, restore > the CFA expression after a DW_CFA_restore. > * target.def (TARGET_ASM_SHOULD_RESTORE_CFA_STATE): New hook. > --- > gcc/config/darwin-protos.h | 1 + > gcc/config/darwin.c | 10 ++++++++++ > gcc/config/darwin.h | 5 +++++ > gcc/doc/tm.texi | 4 ++++ > gcc/doc/tm.texi.in | 2 ++ > gcc/dwarf2cfi.c | 6 ++++++ > gcc/target.def | 14 ++++++++++++++ > 7 files changed, 42 insertions(+) > > diff --git a/gcc/config/darwin-protos.h b/gcc/config/darwin-protos.h > index 2120eb62c56..f5ef82456aa 100644 > --- a/gcc/config/darwin-protos.h > +++ b/gcc/config/darwin-protos.h > @@ -70,6 +70,7 @@ extern void darwin_non_lazy_pcrel (FILE *, rtx); > extern void darwin_emit_unwind_label (FILE *, tree, int, int); > extern void darwin_emit_except_table_label (FILE *); > extern rtx darwin_make_eh_symbol_indirect (rtx, bool); > +extern bool darwin_should_restore_cfa_state (void); > > extern void darwin_pragma_ignore (struct cpp_reader *); > extern void darwin_pragma_options (struct cpp_reader *); > diff --git a/gcc/config/darwin.c b/gcc/config/darwin.c > index 119f3195b63..e2e60bbf1b2 100644 > --- a/gcc/config/darwin.c > +++ b/gcc/config/darwin.c > @@ -2236,6 +2236,16 @@ darwin_make_eh_symbol_indirect (rtx orig, bool > ARG_UNUSED (pubvis)) > /*stub_p=*/false)); > } > > +/* The unwinders in earlier Darwin versions are based on an old version > + of libgcc_s and need current frame address stateto be reset after a > + DW_CFA_restore_state recovers the register values. */ > + > +bool > +darwin_should_restore_cfa_state (void) > +{ > + return generating_for_darwin_version <= 10; > +} > + > /* Return, and mark as used, the name of the stub for the mcount function. > Currently, this is only called by X86 code in the expansion of the > FUNCTION_PROFILER macro, when stubs are enabled. */ > diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h > index 5a9fb43f3c6..d2b2c141c8e 100644 > --- a/gcc/config/darwin.h > +++ b/gcc/config/darwin.h > @@ -614,6 +614,11 @@ extern GTY(()) int darwin_ms_struct; > /* Make an EH (personality or LDSA) symbol indirect as needed. */ > #define TARGET_ASM_MAKE_EH_SYMBOL_INDIRECT darwin_make_eh_symbol_indirect > > +/* Some of Darwin's unwinders need current frame address state to be reset > + after a DW_CFA_restore_state recovers the register values. */ > +#undef TARGET_ASM_SHOULD_RESTORE_CFA_STATE > +#define TARGET_ASM_SHOULD_RESTORE_CFA_STATE darwin_should_restore_cfa_state > + > /* Our profiling scheme doesn't LP labels and counter words. */ > > #define NO_PROFILE_COUNTERS 1 > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi > index 062785af1e2..a25ca6c78b5 100644 > --- a/gcc/doc/tm.texi > +++ b/gcc/doc/tm.texi > @@ -9551,6 +9551,10 @@ If necessary, modify personality and LSDA references > to handle indirection. The > True if the @code{TARGET_ASM_UNWIND_EMIT} hook should be called before the > assembly for @var{insn} has been emitted, false if the hook should be called > afterward. > @end deftypevr > > +@deftypefn {Target Hook} bool TARGET_ASM_SHOULD_RESTORE_CFA_STATE (void) > +For DWARF-based unwind frames, two CFI instructions provide for save and > restore of register state. GCC maintains the current frame address (CFA) > separately from the register bank but the unwinder in libgcc preserves this > state along with the registers (and this is expected by the code that writes > the unwind frames). This hook allows the target to specify that the CFA data > is not saved/restored along with the registers by the target unwinder so that > suitable additional instructions should be emitted to restore it. > +@end deftypefn > + > @node Exception Region Output > @subsection Assembler Commands for Exception Regions > > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in > index 3b19e6f4281..bf724dc093c 100644 > --- a/gcc/doc/tm.texi.in > +++ b/gcc/doc/tm.texi.in > @@ -6462,6 +6462,8 @@ the jump-table. > > @hook TARGET_ASM_UNWIND_EMIT_BEFORE_INSN > > +@hook TARGET_ASM_SHOULD_RESTORE_CFA_STATE > + > @node Exception Region Output > @subsection Assembler Commands for Exception Regions > > diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c > index c2533b091b2..2fa9f325360 100644 > --- a/gcc/dwarf2cfi.c > +++ b/gcc/dwarf2cfi.c > @@ -2848,6 +2848,12 @@ connect_traces (void) > cfi->dw_cfi_opc = DW_CFA_restore_state; > add_cfi (cfi); > > + /* If the target unwinder does not save the CFA as part of the > + register state, we need to restore it separately. */ > + if (targetm.asm_out.should_restore_cfa_state () > + && (cfi = def_cfa_0 (&old_row->cfa, &ti->beg_row->cfa))) > + add_cfi (cfi); > + > old_row = prev_ti->beg_row; > } > /* Otherwise, we'll simply change state from the previous end. */ > diff --git a/gcc/target.def b/gcc/target.def > index be7fcde961a..ab00657330a 100644 > --- a/gcc/target.def > +++ b/gcc/target.def > @@ -211,6 +211,20 @@ DEFHOOKPOD > be called afterward.", > bool, true) > > +/* Return true if the target needs extra instructions to restore the current > + frame address after a DW_CFA_restore_state opcode. */ > +DEFHOOK > +(should_restore_cfa_state, > + "For DWARF-based unwind frames, two CFI instructions provide for save and\ > + restore of register state. GCC maintains the current frame address (CFA)\ > + separately from the register bank but the unwinder in libgcc preserves this\ > + state along with the registers (and this is expected by the code that > writes\ > + the unwind frames). This hook allows the target to specify that the CFA > data\ > + is not saved/restored along with the registers by the target unwinder so > that\ > + suitable additional instructions should be emitted to restore it.", > + bool, (void), > + hook_bool_void_false) > + > /* Generate an internal label. > For now this is just a wrapper for ASM_GENERATE_INTERNAL_LABEL. */ > DEFHOOK_UNDOC > -- > 2.24.1 > >