Message ID | 20240806140744.1082602-3-matthieu.longo@arm.com |
---|---|
State | New |
Headers | show |
Series | dwarf2: add hooks for architecture-specific CFIs | expand |
Matthieu Longo <matthieu.longo@arm.com> writes: > Architecture-specific CFI directives are currently declared an processed > among others architecture-independent CFI directives in gcc/dwarf2* files. > This approach creates confusion, specifically in the case of DWARF > instructions in the vendor space and using the same instruction code. > > Such a clash currently happen between DW_CFA_GNU_window_save (used on > SPARC) and DW_CFA_AARCH64_negate_ra_state (used on AArch64), and both > having the same instruction code 0x2d. > Then AArch64 compilers generates a SPARC CFI directive (.cfi_window_save) > instead of .cfi_negate_ra_state, contrarilly to what is expected in > [DWARF for the Arm 64-bit Architecture (AArch64)](https://github.com/ > ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst). > > This refactoring does not solve completely the problem, but improve the > situation by moving some of the processing of those directives (more > specifically their output in the assembly) to the backend via 2 target > hooks: > - DW_CFI_OPRND1_DESC: parse the first operand of the directive (if any). > - OUTPUT_CFI_DIRECTIVE: output the CFI directive as a string. > > Additionally, this patch also contains a renaming of an enum used for > return address mangling on AArch64. > > gcc/ChangeLog: > > * config/aarch64/aarch64.cc > (aarch64_output_cfi_directive): New hook for CFI directives. > (aarch64_dw_cfi_oprnd1_desc): Same. > (TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive. > (TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc. > * config/sparc/sparc.cc > (sparc_output_cfi_directive): New hook for CFI directives. > (sparc_dw_cfi_oprnd1_desc): Same. > (TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive. > (TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc. > * coretypes.h > (struct dw_cfi_node): Forward declaration of CFI type from > gcc/dwarf2out.h. > (enum dw_cfi_oprnd_type): Same. > * doc/tm.texi: Regenerated from doc/tm.texi.in. > * doc/tm.texi.in: Add doc for OUTPUT_CFI_DIRECTIVE and > DW_CFI_OPRND1_DESC. > * dwarf2cfi.cc > (struct dw_cfi_row): Update the description for window_save > and ra_mangled. > (cfi_equal_p): Adapt parameter of dw_cfi_oprnd1_desc. > (dwarf2out_frame_debug_cfa_negate_ra_state): Use AArch64 CFI > directive instead of the SPARC one. > (change_cfi_row): Use the right CFI directive's name for RA > mangling. > (output_cfi): Remove explicit architecture-specific CFI > directive DW_CFA_GNU_window_save that falls into default case. > (output_cfi_directive): Use target hook as default. > * dwarf2out.cc (dw_cfi_oprnd1_desc): Use target hook as default. > * dwarf2out.h (enum dw_cfi_oprnd_type): specify underlying type > of enum to allow forward declaration. > (dw_cfi_oprnd1_desc): Change type of parameter. > (output_cfi_directive): Use dw_cfi_ref instead of struct > dw_cfi_node *. > * target.def: Documentation for new hooks. > > libffi/ChangeLog: > > * include/ffi_cfi.h (cfi_negate_ra_state): Declare AArch64 cfi > directive. > > libgcc/ChangeLog: > > * config/aarch64/aarch64-asm.h (PACIASP): Replace SPARC CFI > directive by AArch64 one. > (AUTIASP): Same. > > libitm/ChangeLog: > > * config/aarch64/sjlj.S: Replace SPARC CFI directive by > AArch64 one. > > gcc/testsuite/ChangeLog: > > * g++.target/aarch64/pr94515-1.C: Replace SPARC CFI directive by > AArch64 one. > * g++.target/aarch64/pr94515-2.C: Same. > --- > gcc/config/aarch64/aarch64.cc | 32 +++++++++++++++++ > gcc/config/sparc/sparc.cc | 36 ++++++++++++++++++++ > gcc/coretypes.h | 6 ++++ > gcc/doc/tm.texi | 28 +++++++++++++++ > gcc/doc/tm.texi.in | 17 +++++++++ > gcc/dwarf2cfi.cc | 29 ++++++---------- > gcc/dwarf2out.cc | 13 ++++--- > gcc/dwarf2out.h | 10 +++--- > gcc/target.def | 20 +++++++++++ > gcc/testsuite/g++.target/aarch64/pr94515-1.C | 6 ++-- > gcc/testsuite/g++.target/aarch64/pr94515-2.C | 2 +- > libffi/include/ffi_cfi.h | 2 ++ > libgcc/config/aarch64/aarch64-asm.h | 4 +-- > libitm/config/aarch64/sjlj.S | 10 +++--- > 14 files changed, 176 insertions(+), 39 deletions(-) Generally looks good to me. Most of the comments below are very minor, but there's also a question about the hook interface. > > [...] > @@ -12621,6 +12630,33 @@ sparc_output_dwarf_dtprel (FILE *file, int size, rtx x) > fputs (")", file); > } > > +/* This is called from gcc/dwarf2cfi.cc via TARGET_OUTPUT_CFI_DIRECTIVE. > + It outputs SPARC-specific CFI directives (if any). */ I think we should use the same comments as for aarch64, i.e.: /* Implement TARGET_OUTPUT_CFI_DIRECTIVE. */ ... /* Implement TARGET_DW_CFI_OPRND1_DESC. */ The idea is that the hook descriptions in target.def/tm.texi should be self-contained. > +static bool > +sparc_output_cfi_directive (FILE *f, dw_cfi_ref cfi) > +{ > + if (cfi->dw_cfi_opc == DW_CFA_GNU_window_save) > + { > + fprintf (f, "\t.cfi_window_save\n"); > + return true; > + } > + return false; > +} > + > +/* This is called from gcc/dwarf2out.cc via TARGET_DW_CFI_OPRND1_DESC. > + It describes for the GTY machinery what parts of the first operand > + is used. */ > +static bool > +sparc_dw_cfi_oprnd1_desc (dw_cfi_ref cfi, dw_cfi_oprnd_type_ref oprnd_type) > +{ > + if (cfi->dw_cfi_opc == DW_CFA_GNU_window_save) > + { > + oprnd_type = dw_cfi_oprnd_unused; > + return true; > + } > + return false; > +} > + > /* Do whatever processing is required at the end of a file. */ > > static void > diff --git a/gcc/coretypes.h b/gcc/coretypes.h > index 0544bb8fd97..3b622961b7a 100644 > --- a/gcc/coretypes.h > +++ b/gcc/coretypes.h > @@ -141,6 +141,12 @@ struct gomp_single; > struct gomp_target; > struct gomp_teams; > > +/* Forward declaration of CFI's and DWARF's types. */ > +struct dw_cfi_node; > +using dw_cfi_ref = struct dw_cfi_node *; It'd be good to remove the corresponding typedef in dwarf2out.h. > +enum dw_cfi_oprnd_type: unsigned int; > +using dw_cfi_oprnd_type_ref = enum dw_cfi_oprnd_type &; IMO it'd better to avoid adding this, and instead just use dw_cfi_oprnd_type & directly. I think the use of typedefs for pointers (as for dw_cfi_ref) was more popular when the codebase was written in C, and where the typedef often saved on a struct/union/enum tag. I don't think it's something we should use for C++ references since it hides the fact that: void foo (dw_cfi_ref x, dw_cfi_ref y) { ... x = y; ... } is a normal copy whereas: void foo (dw_cfi_oprnd_type_ref x, dw_cfi_oprnd_type_ref y) { ... x = y; ... } isn't. > + > /* Subclasses of symtab_node, using indentation to show the class > hierarchy. */ > > [...] > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in > index 64cea3b1eda..051217d68b1 100644 > --- a/gcc/doc/tm.texi.in > +++ b/gcc/doc/tm.texi.in > @@ -3108,7 +3108,20 @@ used in .eh_frame or .debug_frame is different from that used in other > debug info sections. Given a GCC hard register number, this macro > should return the .eh_frame register number. The default is > @code{DEBUGGER_REGNO (@var{regno})}. > +@end defmac > + > + > +@defmac OUTPUT_CFI_DIRECTIVE (@var{f}, @var{cfi}) > + > +Define this macro if the target has additional CFI directives. Return > +true if an architecture-specific directive was found, false otherwise. > +@end defmac > + > +@defmac DW_CFI_OPRND1_DESC (@var{cfi}, @var{oprnd_type}) > > +Define this macro if the target has additional CFI directives. Return > +true if an architecture-specific directive was found and @var{oprnd_type} > +is set, false otherwise and @var{oprnd_type} is not modified. > @end defmac The new hooks are proper target hooks rather than macros (which is good!) so there's no need to have and document macros as well. (Macros are a hold-over from before target hooks existed.) > @defmac DWARF2_FRAME_REG_OUT (@var{regno}, @var{for_eh}) > [...] > @@ -1547,17 +1549,13 @@ dwarf2out_frame_debug_cfa_window_save (void) > cur_row->window_save = true; > } > > -/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_NEGATE_RA_STATE. > - Note: DW_CFA_GNU_window_save dwarf opcode is reused for toggling RA mangle > - state, this is a target specific operation on AArch64 and can only be used > - on other targets if they don't use the window save operation otherwise. */ > +/*A subroutine of dwarf2out_frame_debug, process REG_CFA_NEGATE_RA_STATE. */ Nit: should be a space before "A subroutine ...". > > static void > dwarf2out_frame_debug_cfa_negate_ra_state (void) > { > dw_cfi_ref cfi = new_cfi (); > - > - cfi->dw_cfi_opc = DW_CFA_GNU_window_save; > + cfi->dw_cfi_opc = DW_CFA_AARCH64_negate_ra_state; > add_cfi (cfi); > cur_row->ra_mangled = !cur_row->ra_mangled; > } > [...] > diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc > index 357efaa5990..260593421df 100644 > --- a/gcc/dwarf2out.cc > +++ b/gcc/dwarf2out.cc > @@ -516,12 +516,11 @@ switch_to_frame_table_section (int for_eh, bool back) > /* Describe for the GTY machinery what parts of dw_cfi_oprnd1 are used. */ > > enum dw_cfi_oprnd_type > -dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi) > +dw_cfi_oprnd1_desc (dw_cfi_ref cfi) > { > - switch (cfi) > + switch (cfi->dw_cfi_opc) > { > case DW_CFA_nop: > - case DW_CFA_GNU_window_save: > case DW_CFA_remember_state: > case DW_CFA_restore_state: > return dw_cfi_oprnd_unused; > @@ -557,7 +556,13 @@ dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi) > return dw_cfi_oprnd_loc; > > default: > - gcc_unreachable (); > + { > + dw_cfi_oprnd_type oprnd_type; > + if (targetm.dw_cfi_oprnd1_desc (cfi, oprnd_type)) Could you say why passing opcode isn't enough? I guess this is to do with some of the changes that are coming later? I think the correspondong: > - return (cfi_oprnd_equal_p (dw_cfi_oprnd1_desc (opc), > + return (cfi_oprnd_equal_p (dw_cfi_oprnd1_desc (a), > &a->dw_cfi_oprnd1, &b->dw_cfi_oprnd1) > && cfi_oprnd_equal_p (dw_cfi_oprnd2_desc (opc), > &a->dw_cfi_oprnd2, &b->dw_cfi_oprnd2)); makes the corretness of cfi_equal_p a bit fuzzier/less obvious, since it seems that dw_cfi_oprnd1_desc (a) is checking something about a that is not checked for b. Thanks, Richard > + return oprnd_type; > + else > + gcc_unreachable (); > + } > } > } >
On 2024-08-13 16:31, Richard Sandiford wrote: > Matthieu Longo <matthieu.longo@arm.com> writes: >> Architecture-specific CFI directives are currently declared an processed >> among others architecture-independent CFI directives in gcc/dwarf2* files. >> This approach creates confusion, specifically in the case of DWARF >> instructions in the vendor space and using the same instruction code. >> >> Such a clash currently happen between DW_CFA_GNU_window_save (used on >> SPARC) and DW_CFA_AARCH64_negate_ra_state (used on AArch64), and both >> having the same instruction code 0x2d. >> Then AArch64 compilers generates a SPARC CFI directive (.cfi_window_save) >> instead of .cfi_negate_ra_state, contrarilly to what is expected in >> [DWARF for the Arm 64-bit Architecture (AArch64)](https://github.com/ >> ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst). >> >> This refactoring does not solve completely the problem, but improve the >> situation by moving some of the processing of those directives (more >> specifically their output in the assembly) to the backend via 2 target >> hooks: >> - DW_CFI_OPRND1_DESC: parse the first operand of the directive (if any). >> - OUTPUT_CFI_DIRECTIVE: output the CFI directive as a string. >> >> Additionally, this patch also contains a renaming of an enum used for >> return address mangling on AArch64. >> >> gcc/ChangeLog: >> >> * config/aarch64/aarch64.cc >> (aarch64_output_cfi_directive): New hook for CFI directives. >> (aarch64_dw_cfi_oprnd1_desc): Same. >> (TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive. >> (TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc. >> * config/sparc/sparc.cc >> (sparc_output_cfi_directive): New hook for CFI directives. >> (sparc_dw_cfi_oprnd1_desc): Same. >> (TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive. >> (TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc. >> * coretypes.h >> (struct dw_cfi_node): Forward declaration of CFI type from >> gcc/dwarf2out.h. >> (enum dw_cfi_oprnd_type): Same. >> * doc/tm.texi: Regenerated from doc/tm.texi.in. >> * doc/tm.texi.in: Add doc for OUTPUT_CFI_DIRECTIVE and >> DW_CFI_OPRND1_DESC. >> * dwarf2cfi.cc >> (struct dw_cfi_row): Update the description for window_save >> and ra_mangled. >> (cfi_equal_p): Adapt parameter of dw_cfi_oprnd1_desc. >> (dwarf2out_frame_debug_cfa_negate_ra_state): Use AArch64 CFI >> directive instead of the SPARC one. >> (change_cfi_row): Use the right CFI directive's name for RA >> mangling. >> (output_cfi): Remove explicit architecture-specific CFI >> directive DW_CFA_GNU_window_save that falls into default case. >> (output_cfi_directive): Use target hook as default. >> * dwarf2out.cc (dw_cfi_oprnd1_desc): Use target hook as default. >> * dwarf2out.h (enum dw_cfi_oprnd_type): specify underlying type >> of enum to allow forward declaration. >> (dw_cfi_oprnd1_desc): Change type of parameter. >> (output_cfi_directive): Use dw_cfi_ref instead of struct >> dw_cfi_node *. >> * target.def: Documentation for new hooks. >> >> libffi/ChangeLog: >> >> * include/ffi_cfi.h (cfi_negate_ra_state): Declare AArch64 cfi >> directive. >> >> libgcc/ChangeLog: >> >> * config/aarch64/aarch64-asm.h (PACIASP): Replace SPARC CFI >> directive by AArch64 one. >> (AUTIASP): Same. >> >> libitm/ChangeLog: >> >> * config/aarch64/sjlj.S: Replace SPARC CFI directive by >> AArch64 one. >> >> gcc/testsuite/ChangeLog: >> >> * g++.target/aarch64/pr94515-1.C: Replace SPARC CFI directive by >> AArch64 one. >> * g++.target/aarch64/pr94515-2.C: Same. >> --- >> gcc/config/aarch64/aarch64.cc | 32 +++++++++++++++++ >> gcc/config/sparc/sparc.cc | 36 ++++++++++++++++++++ >> gcc/coretypes.h | 6 ++++ >> gcc/doc/tm.texi | 28 +++++++++++++++ >> gcc/doc/tm.texi.in | 17 +++++++++ >> gcc/dwarf2cfi.cc | 29 ++++++---------- >> gcc/dwarf2out.cc | 13 ++++--- >> gcc/dwarf2out.h | 10 +++--- >> gcc/target.def | 20 +++++++++++ >> gcc/testsuite/g++.target/aarch64/pr94515-1.C | 6 ++-- >> gcc/testsuite/g++.target/aarch64/pr94515-2.C | 2 +- >> libffi/include/ffi_cfi.h | 2 ++ >> libgcc/config/aarch64/aarch64-asm.h | 4 +-- >> libitm/config/aarch64/sjlj.S | 10 +++--- >> 14 files changed, 176 insertions(+), 39 deletions(-) > > Generally looks good to me. Most of the comments below are very minor, > but there's also a question about the hook interface. > >> >> [...] >> @@ -12621,6 +12630,33 @@ sparc_output_dwarf_dtprel (FILE *file, int size, rtx x) >> fputs (")", file); >> } >> >> +/* This is called from gcc/dwarf2cfi.cc via TARGET_OUTPUT_CFI_DIRECTIVE. >> + It outputs SPARC-specific CFI directives (if any). */ > > I think we should use the same comments as for aarch64, i.e.: > > /* Implement TARGET_OUTPUT_CFI_DIRECTIVE. */ > ... > /* Implement TARGET_DW_CFI_OPRND1_DESC. */ > > The idea is that the hook descriptions in target.def/tm.texi should be > self-contained. > >> +static bool >> +sparc_output_cfi_directive (FILE *f, dw_cfi_ref cfi) >> +{ >> + if (cfi->dw_cfi_opc == DW_CFA_GNU_window_save) >> + { >> + fprintf (f, "\t.cfi_window_save\n"); >> + return true; >> + } >> + return false; >> +} >> + >> +/* This is called from gcc/dwarf2out.cc via TARGET_DW_CFI_OPRND1_DESC. >> + It describes for the GTY machinery what parts of the first operand >> + is used. */ >> +static bool >> +sparc_dw_cfi_oprnd1_desc (dw_cfi_ref cfi, dw_cfi_oprnd_type_ref oprnd_type) >> +{ >> + if (cfi->dw_cfi_opc == DW_CFA_GNU_window_save) >> + { >> + oprnd_type = dw_cfi_oprnd_unused; >> + return true; >> + } >> + return false; >> +} >> + >> /* Do whatever processing is required at the end of a file. */ >> >> static void >> diff --git a/gcc/coretypes.h b/gcc/coretypes.h >> index 0544bb8fd97..3b622961b7a 100644 >> --- a/gcc/coretypes.h >> +++ b/gcc/coretypes.h >> @@ -141,6 +141,12 @@ struct gomp_single; >> struct gomp_target; >> struct gomp_teams; >> >> +/* Forward declaration of CFI's and DWARF's types. */ >> +struct dw_cfi_node; >> +using dw_cfi_ref = struct dw_cfi_node *; > > It'd be good to remove the corresponding typedef in dwarf2out.h. > >> +enum dw_cfi_oprnd_type: unsigned int; >> +using dw_cfi_oprnd_type_ref = enum dw_cfi_oprnd_type &; > > IMO it'd better to avoid adding this, and instead just use > > dw_cfi_oprnd_type & > > directly. > > I think the use of typedefs for pointers (as for dw_cfi_ref) was more > popular when the codebase was written in C, and where the typedef often > saved on a struct/union/enum tag. I don't think it's something we > should use for C++ references since it hides the fact that: > > void foo (dw_cfi_ref x, dw_cfi_ref y) { ... x = y; ... } > > is a normal copy whereas: > > void foo (dw_cfi_oprnd_type_ref x, dw_cfi_oprnd_type_ref y) { ... x = y; ... } > > isn't. > >> + >> /* Subclasses of symtab_node, using indentation to show the class >> hierarchy. */ >> >> [...] >> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in >> index 64cea3b1eda..051217d68b1 100644 >> --- a/gcc/doc/tm.texi.in >> +++ b/gcc/doc/tm.texi.in >> @@ -3108,7 +3108,20 @@ used in .eh_frame or .debug_frame is different from that used in other >> debug info sections. Given a GCC hard register number, this macro >> should return the .eh_frame register number. The default is >> @code{DEBUGGER_REGNO (@var{regno})}. >> +@end defmac >> + >> + >> +@defmac OUTPUT_CFI_DIRECTIVE (@var{f}, @var{cfi}) >> + >> +Define this macro if the target has additional CFI directives. Return >> +true if an architecture-specific directive was found, false otherwise. >> +@end defmac >> + >> +@defmac DW_CFI_OPRND1_DESC (@var{cfi}, @var{oprnd_type}) >> >> +Define this macro if the target has additional CFI directives. Return >> +true if an architecture-specific directive was found and @var{oprnd_type} >> +is set, false otherwise and @var{oprnd_type} is not modified. >> @end defmac > > The new hooks are proper target hooks rather than macros (which is good!) > so there's no need to have and document macros as well. (Macros are a > hold-over from before target hooks existed.) > >> @defmac DWARF2_FRAME_REG_OUT (@var{regno}, @var{for_eh}) >> [...] >> @@ -1547,17 +1549,13 @@ dwarf2out_frame_debug_cfa_window_save (void) >> cur_row->window_save = true; >> } >> >> -/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_NEGATE_RA_STATE. >> - Note: DW_CFA_GNU_window_save dwarf opcode is reused for toggling RA mangle >> - state, this is a target specific operation on AArch64 and can only be used >> - on other targets if they don't use the window save operation otherwise. */ >> +/*A subroutine of dwarf2out_frame_debug, process REG_CFA_NEGATE_RA_STATE. */ > > Nit: should be a space before "A subroutine ...". > >> >> static void >> dwarf2out_frame_debug_cfa_negate_ra_state (void) >> { >> dw_cfi_ref cfi = new_cfi (); >> - >> - cfi->dw_cfi_opc = DW_CFA_GNU_window_save; >> + cfi->dw_cfi_opc = DW_CFA_AARCH64_negate_ra_state; >> add_cfi (cfi); >> cur_row->ra_mangled = !cur_row->ra_mangled; >> } >> [...] >> diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc >> index 357efaa5990..260593421df 100644 >> --- a/gcc/dwarf2out.cc >> +++ b/gcc/dwarf2out.cc >> @@ -516,12 +516,11 @@ switch_to_frame_table_section (int for_eh, bool back) >> /* Describe for the GTY machinery what parts of dw_cfi_oprnd1 are used. */ >> >> enum dw_cfi_oprnd_type >> -dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi) >> +dw_cfi_oprnd1_desc (dw_cfi_ref cfi) >> { >> - switch (cfi) >> + switch (cfi->dw_cfi_opc) >> { >> case DW_CFA_nop: >> - case DW_CFA_GNU_window_save: >> case DW_CFA_remember_state: >> case DW_CFA_restore_state: >> return dw_cfi_oprnd_unused; >> @@ -557,7 +556,13 @@ dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi) >> return dw_cfi_oprnd_loc; >> >> default: >> - gcc_unreachable (); >> + { >> + dw_cfi_oprnd_type oprnd_type; >> + if (targetm.dw_cfi_oprnd1_desc (cfi, oprnd_type)) > > Could you say why passing opcode isn't enough? I guess this is to do > with some of the changes that are coming later? > > I think the correspondong: > >> - return (cfi_oprnd_equal_p (dw_cfi_oprnd1_desc (opc), >> + return (cfi_oprnd_equal_p (dw_cfi_oprnd1_desc (a), >> &a->dw_cfi_oprnd1, &b->dw_cfi_oprnd1) >> && cfi_oprnd_equal_p (dw_cfi_oprnd2_desc (opc), >> &a->dw_cfi_oprnd2, &b->dw_cfi_oprnd2)); > > makes the corretness of cfi_equal_p a bit fuzzier/less obvious, > since it seems that dw_cfi_oprnd1_desc (a) is checking something > about a that is not checked for b. > > Thanks, > Richard > >> + return oprnd_type; >> + else >> + gcc_unreachable (); >> + } >> } >> } >> I addressed all the comments in the next revision. Matthieu
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index 0af5d85c36f..1f87779f40a 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -59,6 +59,7 @@ #include "opts.h" #include "gimplify.h" #include "dwarf2.h" +#include "dwarf2out.h" #include "gimple-iterator.h" #include "tree-vectorizer.h" #include "aarch64-cost-tables.h" @@ -1449,6 +1450,31 @@ aarch64_dwarf_frame_reg_mode (int regno) return default_dwarf_frame_reg_mode (regno); } +/* Implement TARGET_OUTPUT_CFI_DIRECTIVE. */ +static bool +aarch64_output_cfi_directive (FILE *f, dw_cfi_ref cfi) +{ + bool found = false; + if (cfi->dw_cfi_opc == DW_CFA_AARCH64_negate_ra_state) + { + fprintf (f, "\t.cfi_negate_ra_state\n"); + found = true; + } + return found; +} + +/* Implement TARGET_DW_CFI_OPRND1_DESC. */ +static bool +aarch64_dw_cfi_oprnd1_desc(dw_cfi_ref cfi, dw_cfi_oprnd_type_ref oprnd_type) +{ + if (cfi->dw_cfi_opc == DW_CFA_AARCH64_negate_ra_state) + { + oprnd_type = dw_cfi_oprnd_unused; + return true; + } + return false; +} + /* If X is a CONST_DOUBLE, return its bit representation as a constant integer, otherwise return X unmodified. */ static rtx @@ -30750,6 +30776,12 @@ aarch64_libgcc_floating_mode_supported_p #undef TARGET_DWARF_FRAME_REG_MODE #define TARGET_DWARF_FRAME_REG_MODE aarch64_dwarf_frame_reg_mode +#undef TARGET_OUTPUT_CFI_DIRECTIVE +#define TARGET_OUTPUT_CFI_DIRECTIVE aarch64_output_cfi_directive + +#undef TARGET_DW_CFI_OPRND1_DESC +#define TARGET_DW_CFI_OPRND1_DESC aarch64_dw_cfi_oprnd1_desc + #undef TARGET_PROMOTED_TYPE #define TARGET_PROMOTED_TYPE aarch64_promoted_type diff --git a/gcc/config/sparc/sparc.cc b/gcc/config/sparc/sparc.cc index 9282fb43b44..4de2f1895b6 100644 --- a/gcc/config/sparc/sparc.cc +++ b/gcc/config/sparc/sparc.cc @@ -61,6 +61,7 @@ along with GCC; see the file COPYING3. If not see #include "builtins.h" #include "tree-vector-builder.h" #include "opts.h" +#include "dwarf2out.h" /* This file should be included last. */ #include "target-def.h" @@ -681,6 +682,8 @@ static rtx sparc_libcall_value (machine_mode, const_rtx); static bool sparc_function_value_regno_p (const unsigned int); static unsigned HOST_WIDE_INT sparc_asan_shadow_offset (void); static void sparc_output_dwarf_dtprel (FILE *, int, rtx) ATTRIBUTE_UNUSED; +static bool sparc_output_cfi_directive (FILE *, dw_cfi_ref); +static bool sparc_dw_cfi_oprnd1_desc (dw_cfi_ref, dw_cfi_oprnd_type_ref); static void sparc_file_end (void); static bool sparc_frame_pointer_required (void); static bool sparc_can_eliminate (const int, const int); @@ -878,6 +881,12 @@ char sparc_hard_reg_printed[8]; #define TARGET_ASM_OUTPUT_DWARF_DTPREL sparc_output_dwarf_dtprel #endif +#undef TARGET_OUTPUT_CFI_DIRECTIVE +#define TARGET_OUTPUT_CFI_DIRECTIVE sparc_output_cfi_directive + +#undef TARGET_DW_CFI_OPRND1_DESC +#define TARGET_DW_CFI_OPRND1_DESC sparc_dw_cfi_oprnd1_desc + #undef TARGET_ASM_FILE_END #define TARGET_ASM_FILE_END sparc_file_end @@ -12621,6 +12630,33 @@ sparc_output_dwarf_dtprel (FILE *file, int size, rtx x) fputs (")", file); } +/* This is called from gcc/dwarf2cfi.cc via TARGET_OUTPUT_CFI_DIRECTIVE. + It outputs SPARC-specific CFI directives (if any). */ +static bool +sparc_output_cfi_directive (FILE *f, dw_cfi_ref cfi) +{ + if (cfi->dw_cfi_opc == DW_CFA_GNU_window_save) + { + fprintf (f, "\t.cfi_window_save\n"); + return true; + } + return false; +} + +/* This is called from gcc/dwarf2out.cc via TARGET_DW_CFI_OPRND1_DESC. + It describes for the GTY machinery what parts of the first operand + is used. */ +static bool +sparc_dw_cfi_oprnd1_desc (dw_cfi_ref cfi, dw_cfi_oprnd_type_ref oprnd_type) +{ + if (cfi->dw_cfi_opc == DW_CFA_GNU_window_save) + { + oprnd_type = dw_cfi_oprnd_unused; + return true; + } + return false; +} + /* Do whatever processing is required at the end of a file. */ static void diff --git a/gcc/coretypes.h b/gcc/coretypes.h index 0544bb8fd97..3b622961b7a 100644 --- a/gcc/coretypes.h +++ b/gcc/coretypes.h @@ -141,6 +141,12 @@ struct gomp_single; struct gomp_target; struct gomp_teams; +/* Forward declaration of CFI's and DWARF's types. */ +struct dw_cfi_node; +using dw_cfi_ref = struct dw_cfi_node *; +enum dw_cfi_oprnd_type: unsigned int; +using dw_cfi_oprnd_type_ref = enum dw_cfi_oprnd_type &; + /* Subclasses of symtab_node, using indentation to show the class hierarchy. */ diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index c7535d07f4d..1645e1cb0ce 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -3911,7 +3911,20 @@ used in .eh_frame or .debug_frame is different from that used in other debug info sections. Given a GCC hard register number, this macro should return the .eh_frame register number. The default is @code{DEBUGGER_REGNO (@var{regno})}. +@end defmac + + +@defmac OUTPUT_CFI_DIRECTIVE (@var{f}, @var{cfi}) + +Define this macro if the target has additional CFI directives. Return +true if an architecture-specific directive was found, false otherwise. +@end defmac +@defmac DW_CFI_OPRND1_DESC (@var{cfi}, @var{oprnd_type}) + +Define this macro if the target has additional CFI directives. Return +true if an architecture-specific directive was found and @var{oprnd_type} +is set, false otherwise and @var{oprnd_type} is not modified. @end defmac @defmac DWARF2_FRAME_REG_OUT (@var{regno}, @var{for_eh}) @@ -10039,6 +10052,21 @@ used to return a smaller mode than the raw mode to prevent call clobbered parts of a register altering the frame register size @end deftypefn +@deftypefn {Target Hook} bool TARGET_OUTPUT_CFI_DIRECTIVE (FILE * @var{f}, dw_cfi_ref @var{cfi}) +This hook handles architecture-specific CFI directives and prints +them out to the assembly file @var{f}. +Return true if a architecture-specific directive was found, false +otherwise. +@end deftypefn + +@deftypefn {Target Hook} bool TARGET_DW_CFI_OPRND1_DESC (dw_cfi_ref @var{cfi}, dw_cfi_oprnd_type_ref @var{oprnd_type}) +This hook informs the caller what the architecture-specific directives +takes as a first operand. +Return true if a architecture-specific directive was found and +@var{oprnd_type} is set, false otherwise and @var{oprnd_type} is not +modified. +@end deftypefn + @deftypefn {Target Hook} void TARGET_INIT_DWARF_REG_SIZES_EXTRA (tree @var{address}) If some registers are represented in Dwarf-2 unwind information in multiple pieces, define this hook to fill in information about the diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 64cea3b1eda..051217d68b1 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -3108,7 +3108,20 @@ used in .eh_frame or .debug_frame is different from that used in other debug info sections. Given a GCC hard register number, this macro should return the .eh_frame register number. The default is @code{DEBUGGER_REGNO (@var{regno})}. +@end defmac + + +@defmac OUTPUT_CFI_DIRECTIVE (@var{f}, @var{cfi}) + +Define this macro if the target has additional CFI directives. Return +true if an architecture-specific directive was found, false otherwise. +@end defmac + +@defmac DW_CFI_OPRND1_DESC (@var{cfi}, @var{oprnd_type}) +Define this macro if the target has additional CFI directives. Return +true if an architecture-specific directive was found and @var{oprnd_type} +is set, false otherwise and @var{oprnd_type} is not modified. @end defmac @defmac DWARF2_FRAME_REG_OUT (@var{regno}, @var{for_eh}) @@ -6612,6 +6625,10 @@ the target supports DWARF 2 frame unwind information. @hook TARGET_DWARF_FRAME_REG_MODE +@hook TARGET_OUTPUT_CFI_DIRECTIVE + +@hook TARGET_DW_CFI_OPRND1_DESC + @hook TARGET_INIT_DWARF_REG_SIZES_EXTRA @hook TARGET_ASM_TTYPE diff --git a/gcc/dwarf2cfi.cc b/gcc/dwarf2cfi.cc index 4ad9acbd6fd..6c80e0b17bd 100644 --- a/gcc/dwarf2cfi.cc +++ b/gcc/dwarf2cfi.cc @@ -69,10 +69,12 @@ struct GTY(()) dw_cfi_row /* The expressions for any register column that is saved. */ cfi_vec reg_save; - /* True if the register window is saved. */ + /* Sparc extension for DW_CFA_GNU_window_save. + True if the register window is saved. */ bool window_save; - /* True if the return address is in a mangled state. */ + /* Aarch64 extension for DW_CFA_AARCH64_negate_ra_state. + True if the return address is in a mangled state. */ bool ra_mangled; }; @@ -816,7 +818,7 @@ cfi_equal_p (dw_cfi_ref a, dw_cfi_ref b) /* Compare the two operands, re-using the type of the operands as already exposed elsewhere. */ - return (cfi_oprnd_equal_p (dw_cfi_oprnd1_desc (opc), + return (cfi_oprnd_equal_p (dw_cfi_oprnd1_desc (a), &a->dw_cfi_oprnd1, &b->dw_cfi_oprnd1) && cfi_oprnd_equal_p (dw_cfi_oprnd2_desc (opc), &a->dw_cfi_oprnd2, &b->dw_cfi_oprnd2)); @@ -1547,17 +1549,13 @@ dwarf2out_frame_debug_cfa_window_save (void) cur_row->window_save = true; } -/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_NEGATE_RA_STATE. - Note: DW_CFA_GNU_window_save dwarf opcode is reused for toggling RA mangle - state, this is a target specific operation on AArch64 and can only be used - on other targets if they don't use the window save operation otherwise. */ +/*A subroutine of dwarf2out_frame_debug, process REG_CFA_NEGATE_RA_STATE. */ static void dwarf2out_frame_debug_cfa_negate_ra_state (void) { dw_cfi_ref cfi = new_cfi (); - - cfi->dw_cfi_opc = DW_CFA_GNU_window_save; + cfi->dw_cfi_opc = DW_CFA_AARCH64_negate_ra_state; add_cfi (cfi); cur_row->ra_mangled = !cur_row->ra_mangled; } @@ -2426,8 +2424,7 @@ change_cfi_row (dw_cfi_row *old_row, dw_cfi_row *new_row) dw_cfi_ref cfi = new_cfi (); gcc_assert (!old_row->window_save && !new_row->window_save); - /* DW_CFA_GNU_window_save is reused for toggling RA mangle state. */ - cfi->dw_cfi_opc = DW_CFA_GNU_window_save; + cfi->dw_cfi_opc = DW_CFA_AARCH64_negate_ra_state; add_cfi (cfi); } } @@ -3516,9 +3513,6 @@ output_cfi (dw_cfi_ref cfi, dw_fde_ref fde, int for_eh) dw2_asm_output_data_sleb128 (off, NULL); break; - case DW_CFA_GNU_window_save: - break; - case DW_CFA_def_cfa_expression: case DW_CFA_expression: case DW_CFA_val_expression: @@ -3631,10 +3625,6 @@ output_cfi_directive (FILE *f, dw_cfi_ref cfi) } break; - case DW_CFA_GNU_window_save: - fprintf (f, "\t.cfi_window_save\n"); - break; - case DW_CFA_def_cfa_expression: case DW_CFA_expression: case DW_CFA_val_expression: @@ -3651,7 +3641,8 @@ output_cfi_directive (FILE *f, dw_cfi_ref cfi) break; default: - gcc_unreachable (); + if (!targetm.output_cfi_directive (f, cfi)) + gcc_unreachable (); } } diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc index 357efaa5990..260593421df 100644 --- a/gcc/dwarf2out.cc +++ b/gcc/dwarf2out.cc @@ -516,12 +516,11 @@ switch_to_frame_table_section (int for_eh, bool back) /* Describe for the GTY machinery what parts of dw_cfi_oprnd1 are used. */ enum dw_cfi_oprnd_type -dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi) +dw_cfi_oprnd1_desc (dw_cfi_ref cfi) { - switch (cfi) + switch (cfi->dw_cfi_opc) { case DW_CFA_nop: - case DW_CFA_GNU_window_save: case DW_CFA_remember_state: case DW_CFA_restore_state: return dw_cfi_oprnd_unused; @@ -557,7 +556,13 @@ dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi) return dw_cfi_oprnd_loc; default: - gcc_unreachable (); + { + dw_cfi_oprnd_type oprnd_type; + if (targetm.dw_cfi_oprnd1_desc (cfi, oprnd_type)) + return oprnd_type; + else + gcc_unreachable (); + } } } diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h index a424981b911..da0b0bf0203 100644 --- a/gcc/dwarf2out.h +++ b/gcc/dwarf2out.h @@ -38,7 +38,7 @@ typedef struct dw_wide_int *dw_wide_int_ptr; and address fields are provided as possible operands; their use is selected by the opcode field. */ -enum dw_cfi_oprnd_type { +enum dw_cfi_oprnd_type: unsigned int { dw_cfi_oprnd_unused, dw_cfi_oprnd_reg_num, dw_cfi_oprnd_offset, @@ -46,6 +46,7 @@ enum dw_cfi_oprnd_type { dw_cfi_oprnd_loc, dw_cfi_oprnd_cfa_loc }; +using dw_cfi_oprnd_type_ref = enum dw_cfi_oprnd_type &; typedef union GTY(()) { unsigned int GTY ((tag ("dw_cfi_oprnd_reg_num"))) dw_cfi_reg_num; @@ -58,7 +59,7 @@ typedef union GTY(()) { struct GTY(()) dw_cfi_node { enum dwarf_call_frame_info dw_cfi_opc; - dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc (%1.dw_cfi_opc)"))) + dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc (&%1)"))) dw_cfi_oprnd1; dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc (%1.dw_cfi_opc)"))) dw_cfi_oprnd2; @@ -368,12 +369,11 @@ extern void output_cfi (dw_cfi_ref, dw_fde_ref, int); extern GTY(()) cfi_vec cie_cfi_vec; /* Interface from dwarf2*.c to the rest of the compiler. */ -extern enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc - (enum dwarf_call_frame_info cfi); +extern enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc (dw_cfi_ref cfi); extern enum dw_cfi_oprnd_type dw_cfi_oprnd2_desc (enum dwarf_call_frame_info cfi); -extern void output_cfi_directive (FILE *f, struct dw_cfi_node *cfi); +extern void output_cfi_directive (FILE *f, dw_cfi_ref cfi); extern void dwarf2out_emit_cfi (dw_cfi_ref cfi); diff --git a/gcc/target.def b/gcc/target.def index 3de1aad4c84..6a6470f202a 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -4114,6 +4114,26 @@ clobbered parts of a register altering the frame register size", machine_mode, (int regno), default_dwarf_frame_reg_mode) +/* Print out architecture-specific CFI directives to the assembly file. */ +DEFHOOK +(output_cfi_directive, + "This hook handles architecture-specific CFI directives and prints\n\ +them out to the assembly file @var{f}.\n\ +Return true if a architecture-specific directive was found, false\n\ +otherwise.", + bool, (FILE * f, dw_cfi_ref cfi), + hook_bool_void_false) + +DEFHOOK +(dw_cfi_oprnd1_desc, + "This hook informs the caller what the architecture-specific directives\n\ +takes as a first operand.\n\ +Return true if a architecture-specific directive was found and\n\ +@var{oprnd_type} is set, false otherwise and @var{oprnd_type} is not\n\ +modified.", + bool, (dw_cfi_ref cfi, dw_cfi_oprnd_type_ref oprnd_type), + hook_bool_void_false) + /* If expand_builtin_init_dwarf_reg_sizes needs to fill in table entries not corresponding directly to registers below FIRST_PSEUDO_REGISTER, this hook should generate the necessary diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-1.C b/gcc/testsuite/g++.target/aarch64/pr94515-1.C index 20ae81215fc..d5c114a83a8 100644 --- a/gcc/testsuite/g++.target/aarch64/pr94515-1.C +++ b/gcc/testsuite/g++.target/aarch64/pr94515-1.C @@ -1,4 +1,4 @@ -/* PR target/94515. Check .cfi_window_save with multiple return paths. */ +/* PR target/94515. Check .cfi_negate_ra_state with multiple return paths. */ /* { dg-do run } */ /* { dg-require-effective-target lp64 } */ /* { dg-additional-options "-O2 --save-temps" } */ @@ -38,7 +38,7 @@ int main () } /* This check only works if there are two return paths in test and - cfi_window_save is used for both instead of cfi_remember_state + cfi_negate_ra_state is used for both instead of cfi_remember_state plus cfi_restore_state. This is currently the case with -O2. */ -/* { dg-final { scan-assembler-times {\t\.cfi_window_save\n} 4 } } */ +/* { dg-final { scan-assembler-times {\t\.cfi_negate_ra_state\n} 4 } } */ diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-2.C b/gcc/testsuite/g++.target/aarch64/pr94515-2.C index e73df499070..f4a3333beed 100644 --- a/gcc/testsuite/g++.target/aarch64/pr94515-2.C +++ b/gcc/testsuite/g++.target/aarch64/pr94515-2.C @@ -1,4 +1,4 @@ -/* PR target/94515. Check .cfi_window_save with multiple return paths. */ +/* PR target/94515. Check .cfi_negate_ra_state with multiple return paths. */ /* { dg-do run } */ /* { dg-require-effective-target lp64 } */ /* { dg-additional-options "-O2 -mbranch-protection=pac-ret" } */ diff --git a/libffi/include/ffi_cfi.h b/libffi/include/ffi_cfi.h index f4c292d0040..2beb165c9a1 100644 --- a/libffi/include/ffi_cfi.h +++ b/libffi/include/ffi_cfi.h @@ -46,6 +46,7 @@ # define cfi_remember_state .cfi_remember_state # define cfi_restore_state .cfi_restore_state # define cfi_window_save .cfi_window_save +# define cfi_negate_ra_state .cfi_negate_ra_state # define cfi_personality(enc, exp) .cfi_personality enc, exp # define cfi_lsda(enc, exp) .cfi_lsda enc, exp # define cfi_escape(...) .cfi_escape __VA_ARGS__ @@ -68,6 +69,7 @@ # define cfi_remember_state # define cfi_restore_state # define cfi_window_save +# define cfi_negate_ra_state # define cfi_personality(enc, exp) # define cfi_lsda(enc, exp) # define cfi_escape(...) diff --git a/libgcc/config/aarch64/aarch64-asm.h b/libgcc/config/aarch64/aarch64-asm.h index 83c2e5944b3..d8ab91d52f1 100644 --- a/libgcc/config/aarch64/aarch64-asm.h +++ b/libgcc/config/aarch64/aarch64-asm.h @@ -50,8 +50,8 @@ #if __ARM_FEATURE_PAC_DEFAULT & 3 # define PAC_FLAG FEATURE_1_PAC -# define PACIASP hint 25; .cfi_window_save -# define AUTIASP hint 29; .cfi_window_save +# define PACIASP hint 25; .cfi_negate_ra_state +# define AUTIASP hint 29; .cfi_negate_ra_state #else # define PAC_FLAG 0 # define PACIASP diff --git a/libitm/config/aarch64/sjlj.S b/libitm/config/aarch64/sjlj.S index 6b248f7c040..aeffd4d1070 100644 --- a/libitm/config/aarch64/sjlj.S +++ b/libitm/config/aarch64/sjlj.S @@ -31,20 +31,20 @@ #define AUTIBSP hint 31 #if defined(HAVE_AS_CFI_PSEUDO_OP) && defined(__GCC_HAVE_DWARF2_CFI_ASM) -# define cfi_window_save .cfi_window_save -# define cfi_b_key_frame .cfi_b_key_frame +# define cfi_negate_ra_state .cfi_negate_ra_state +# define cfi_b_key_frame .cfi_b_key_frame #else -# define cfi_window_save +# define cfi_negate_ra_state # define cfi_b_key_frame #endif #if __ARM_FEATURE_PAC_DEFAULT & 1 -# define CFI_PAC_TOGGLE cfi_window_save +# define CFI_PAC_TOGGLE cfi_negate_ra_state # define CFI_PAC_KEY # define PAC_AND_BTI PACIASP # define AUT AUTIASP #elif __ARM_FEATURE_PAC_DEFAULT & 2 -# define CFI_PAC_TOGGLE cfi_window_save +# define CFI_PAC_TOGGLE cfi_negate_ra_state # define CFI_PAC_KEY cfi_b_key_frame # define PAC_AND_BTI PACIBSP # define AUT AUTIBSP