diff mbox

[2/5,AArch64] Generate dwarf information for -msign-return-address

Message ID 4df3fc1f-bf5c-b5a5-d372-0b41fb377700@foss.arm.com
State New
Headers show

Commit Message

Jiong Wang Jan. 6, 2017, 11:47 a.m. UTC
On 11/11/16 18:22, Jiong Wang wrote:
> This patch generate DWARF description for pointer authentication.  DWARF value

> expression is used to describe the authentication action.

>

> Please see the cover letter and AArch64 DWARF specification for the semantics

> of AArch64 DWARF operations.

>

> When authentication key index is A key, we use compact DWARF description which

> can largely save DWARF frame size, otherwise we fallback to general operator.

>

>

>

> Example

> ===

>

>      int

>      cal (int a, int b, int c)

>      {

>        return a + dec (b) + c;

>      }

>

> Compact DWARF description

> (-march=armv8.3-a -msign-return-address)

> ===

>

>    DW_CFA_advance_loc: 4 to 0000000000000004

>    DW_CFA_val_expression: r30 (x30) (DW_OP_AARCH64_paciasp)

>    DW_CFA_advance_loc: 4 to 0000000000000008

>    DW_CFA_val_expression: r30 (x30) (DW_OP_AARCH64_paciasp_deref: -24)

>

> General DWARF description

> ===

> (-march=armv8.3-a -msign-return-address -mpauth-key=b_key)

>

>    DW_CFA_advance_loc: 4 to 0000000000000004

>    DW_CFA_val_expression: r30 (x30) (DW_OP_breg30 (x30): 0; DW_OP_AARCH64_pauth: 18)

>    DW_CFA_advance_loc: 4 to 0000000000000008

>    DW_CFA_val_expression: r30 (x30) (DW_OP_dup; DW_OP_const1s: -24; DW_OP_plus; DW_OP_deref; DW_OP_AARCH64_pauth: 18)

>

>  From Linux kernel testing, -msign-return-address will introduce +24%

> .debug_frame size increase when signing all functions and using compact

> description, and about +45% .debug_frame size increase if using general

> description.

>

>

> gcc/

> 2016-11-11  Jiong Wang<jiong.wang@arm.com>

>

>          * config/aarch64/aarch64.h (aarch64_pauth_action_type): New enum.

>          * config/aarch64/aarch64.c (aarch64_attach_ra_auth_dwarf_note): New function.

>          (aarch64_attach_ra_auth_dwarf_general): New function.

>          (aarch64_attach_ra_auth_dwarf_shortcut): New function.

>          (aarch64_save_callee_saves): Generate dwarf information if LR is signed.

>          (aarch64_expand_prologue): Likewise.

>          (aarch64_expand_epilogue): Likewise.


This patch is an update on DWARF generation for return address signing.

According to new proposal, we simply needs to generate REG_CFA_WINDOW_SAVE
annotation.

gcc/

2017-01-06  Jiong Wang  <jiong.wang@arm.com>

         * config/aarch64/aarch64.c (aarch64_expand_prologue): Generate dwarf
         annotation (REG_CFA_WINDOW_SAVE) for return address signing.
         (aarch64_expand_epilogue): Likewise.

Comments

Richard Earnshaw (lists) Jan. 13, 2017, 4:09 p.m. UTC | #1
On 06/01/17 11:47, Jiong Wang wrote:
> On 11/11/16 18:22, Jiong Wang wrote:

>> This patch generate DWARF description for pointer authentication. 

>> DWARF value

>> expression is used to describe the authentication action.

>>

>> Please see the cover letter and AArch64 DWARF specification for the

>> semantics

>> of AArch64 DWARF operations.

>>

>> When authentication key index is A key, we use compact DWARF

>> description which

>> can largely save DWARF frame size, otherwise we fallback to general

>> operator.

>>

>>

>>

>> Example

>> ===

>>

>>      int

>>      cal (int a, int b, int c)

>>      {

>>        return a + dec (b) + c;

>>      }

>>

>> Compact DWARF description

>> (-march=armv8.3-a -msign-return-address)

>> ===

>>

>>    DW_CFA_advance_loc: 4 to 0000000000000004

>>    DW_CFA_val_expression: r30 (x30) (DW_OP_AARCH64_paciasp)

>>    DW_CFA_advance_loc: 4 to 0000000000000008

>>    DW_CFA_val_expression: r30 (x30) (DW_OP_AARCH64_paciasp_deref: -24)

>>

>> General DWARF description

>> ===

>> (-march=armv8.3-a -msign-return-address -mpauth-key=b_key)

>>

>>    DW_CFA_advance_loc: 4 to 0000000000000004

>>    DW_CFA_val_expression: r30 (x30) (DW_OP_breg30 (x30): 0;

>> DW_OP_AARCH64_pauth: 18)

>>    DW_CFA_advance_loc: 4 to 0000000000000008

>>    DW_CFA_val_expression: r30 (x30) (DW_OP_dup; DW_OP_const1s: -24;

>> DW_OP_plus; DW_OP_deref; DW_OP_AARCH64_pauth: 18)

>>

>>  From Linux kernel testing, -msign-return-address will introduce +24%

>> .debug_frame size increase when signing all functions and using compact

>> description, and about +45% .debug_frame size increase if using general

>> description.

>>

>>

>> gcc/

>> 2016-11-11  Jiong Wang<jiong.wang@arm.com>

>>

>>          * config/aarch64/aarch64.h (aarch64_pauth_action_type): New

>> enum.

>>          * config/aarch64/aarch64.c

>> (aarch64_attach_ra_auth_dwarf_note): New function.

>>          (aarch64_attach_ra_auth_dwarf_general): New function.

>>          (aarch64_attach_ra_auth_dwarf_shortcut): New function.

>>          (aarch64_save_callee_saves): Generate dwarf information if LR

>> is signed.

>>          (aarch64_expand_prologue): Likewise.

>>          (aarch64_expand_epilogue): Likewise.

> 

> This patch is an update on DWARF generation for return address signing.

> 

> According to new proposal, we simply needs to generate REG_CFA_WINDOW_SAVE

> annotation.

> 

> gcc/

> 

> 2017-01-06  Jiong Wang  <jiong.wang@arm.com>

> 

>         * config/aarch64/aarch64.c (aarch64_expand_prologue): Generate

> dwarf

>         annotation (REG_CFA_WINDOW_SAVE) for return address signing.

>         (aarch64_expand_epilogue): Likewise.

> 

> 


I don't think we should be overloading REG_CFA_WINDOW_SAVE internally in
the compiler -- it's one thing to do it in the dwarf output tables, but
quite another to be doing it elsewhere in the compiler.

Instead we should create a new reg note kind and use that, but in the
final dwarf output then emit the overloaded opcode.

R.

> 

> 2.patch

> 

> 

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c

> index 002895a167ce0deb45a5c1726527651af18bb4df..20ed79e5690f45ec121ef516245c686cc0cc82b5 100644

> --- a/gcc/config/aarch64/aarch64.c

> +++ b/gcc/config/aarch64/aarch64.c

> @@ -3553,7 +3553,11 @@ aarch64_expand_prologue (void)

>  

>    /* Sign return address for functions.  */

>    if (aarch64_return_address_signing_enabled ())

> -    emit_insn (gen_pacisp ());

> +    {

> +      insn = emit_insn (gen_pacisp ());

> +      add_reg_note (insn, REG_CFA_WINDOW_SAVE, const0_rtx);

> +      RTX_FRAME_RELATED_P (insn) = 1;

> +    }

>  

>    if (flag_stack_usage_info)

>      current_function_static_stack_size = frame_size;

> @@ -3698,7 +3702,11 @@ aarch64_expand_epilogue (bool for_sibcall)

>       want to use the CFA of the function which calls eh_return.  */

>    if (aarch64_return_address_signing_enabled ()

>        && (for_sibcall || !TARGET_ARMV8_3 || crtl->calls_eh_return))

> -    emit_insn (gen_autisp ());

> +    {

> +      insn = emit_insn (gen_autisp ());

> +      add_reg_note (insn, REG_CFA_WINDOW_SAVE, const0_rtx);

> +      RTX_FRAME_RELATED_P (insn) = 1;

> +    }

>  

>    /* Stack adjustment for exception handler.  */

>    if (crtl->calls_eh_return)

>
Jiong Wang Jan. 13, 2017, 6:02 p.m. UTC | #2
On 13/01/17 16:09, Richard Earnshaw (lists) wrote:
> On 06/01/17 11:47, Jiong Wang wrote:

>>

>> This patch is an update on DWARF generation for return address signing.

>>

>> According to new proposal, we simply needs to generate REG_CFA_WINDOW_SAVE

>> annotation.

>>

>> gcc/

>>

>> 2017-01-06  Jiong Wang  <jiong.wang@arm.com>

>>

>>          * config/aarch64/aarch64.c (aarch64_expand_prologue): Generate

>> dwarf

>>          annotation (REG_CFA_WINDOW_SAVE) for return address signing.

>>          (aarch64_expand_epilogue): Likewise.

>>

>>

> I don't think we should be overloading REG_CFA_WINDOW_SAVE internally in

> the compiler -- it's one thing to do it in the dwarf output tables, but

> quite another to be doing it elsewhere in the compiler.

>

> Instead we should create a new reg note kind and use that, but in the

> final dwarf output then emit the overloaded opcode.


I can see the reason for doing this is if you want to seperate the interpretion
of GCC CFA reg-note and the final DWARF CFA operation.  My understanding is all
reg notes defined in gcc/reg-note.def should have general meaning, even the
CFA_WINDOW_SAVE.  For those which are architecture specific we might need a
mechanism to define them in backend only.
    
For general reg-notes in gcc/reg-note.def, they are not always have the
corresponding standard DWARF CFA operation, for example CFA_WINDOW_SAVE,
therefore if we want to achieve what you described, I think we also need to
define a new target hook which maps a GCC CFA reg-note into architecture DWARF
CFA operation.

Regards,
Jiong
Jiong Wang Jan. 16, 2017, 2:29 p.m. UTC | #3
On 13/01/17 18:02, Jiong Wang wrote:
> On 13/01/17 16:09, Richard Earnshaw (lists) wrote:

>> On 06/01/17 11:47, Jiong Wang wrote:

>>>

>>> This patch is an update on DWARF generation for return address signing.

>>>

>>> According to new proposal, we simply needs to generate 

>>> REG_CFA_WINDOW_SAVE

>>> annotation.

>>>

>>> gcc/

>>>

>>> 2017-01-06  Jiong Wang  <jiong.wang@arm.com>

>>>

>>>          * config/aarch64/aarch64.c (aarch64_expand_prologue): Generate

>>> dwarf

>>>          annotation (REG_CFA_WINDOW_SAVE) for return address signing.

>>>          (aarch64_expand_epilogue): Likewise.

>>>

>>>

>> I don't think we should be overloading REG_CFA_WINDOW_SAVE internally in

>> the compiler -- it's one thing to do it in the dwarf output tables, but

>> quite another to be doing it elsewhere in the compiler.

>>

>> Instead we should create a new reg note kind and use that, but in the

>> final dwarf output then emit the overloaded opcode.

>

> I can see the reason for doing this is if you want to seperate the 

> interpretion

> of GCC CFA reg-note and the final DWARF CFA operation.  My 

> understanding is all

> reg notes defined in gcc/reg-note.def should have general meaning, 

> even the

> CFA_WINDOW_SAVE.  For those which are architecture specific we might 

> need a

> mechanism to define them in backend only.

>    For general reg-notes in gcc/reg-note.def, they are not always have 

> the

> corresponding standard DWARF CFA operation, for example CFA_WINDOW_SAVE,

> therefore if we want to achieve what you described, I think we also 

> need to

> define a new target hook which maps a GCC CFA reg-note into 

> architecture DWARF

> CFA operation.

>

> Regards,

> Jiong

>

>

Here is the patch.

Introduced one new target hook TARGET_DWARF_MAP_REGNOTE_TO_CFA.  The purpose is
to allow GCC to map DWARF CFA reg notes in reg-note.def, which looks to me have
generic meaning, into target private DWARF CFI if there is no standard DWARF CFI
mapping.

One new GCC reg-note REG_TOGGLE_RA_MANGLE introduced as well, currently, it's
only used by AArch64 to implement return address signing and is mapped to
AArch64's target private DWARF CFI.

Does this approach and implementation looks OK?

I can come up with seperate patches to define this hook on Sparc for
CFA_WINDOW_SAVE, and to remove redundant including of dwarf2.h although there is
"ifdef" protector in header file.

The default hook implementation "default_dwarf_map_regnote_to_cfa" in
targhooks.c used the types "enum reg_note" and "enum dwarf_call_frame_info"
which is not included in coretypes.h thus this patch has several change in
header files.  I have done X86 bootstrap to make sure no build breakage.  I'd
appreciate there is better ideas to handle these type define.

Thanks.

gcc/ChangeLog:

2017-01-16  Jiong Wang  <jiong.wang@arm.com>

         * target.def (dwarf_map_regnote_to_cfa): New hook.
         * targhooks.c (default_dwarf_map_regnote_to_cfa): Default implementation
         for TARGET_DWARF_MAP_REGNOTE_TO_CFA.
         * targhooks.h (default_dwarf_map_regnote_to_cfa): New declaration.
         * rtl.h (enum reg_note): Move enum reg_note to...
         * coretypes.h: ... here.
         (dwarf2.h): New include file.
         * reg-notes.def (CFA_TOGGLE_RA_MANGLE): New reg-note.
         * combine-stack-adj.c (no_unhandled_cfa): Handle
         REG_CFA_TOGGLE_RA_MANGLE.
         * dwarf2cfi.c (dwarf2out_frame_debug_cfa_toggle_ra_mangle): New
         function.
         (dwarf2out_frame_debug): Handle REG_CFA_TOGGLE_RA_MANGLE.
         * doc/tm.texi: Regenerate.
         * doc/tm.texi.in: Documents TARGET_DWARF_MAP_REGNOTE_TO_CFA.
         * config/aarch64/aarch64.c (aarch64_map_regnote_to_cfa): Implements
         TARGET_DWARF_MAP_REGNOTE_TO_CFA.
         (aarch64_expand_prologue): Generate DWARF info for return address
         signing.
         (aarch64_expand_epilogue): Likewise.
         (TARGET_DWARF_MAP_REGNOTE_TO_CFA): Define.diff --git a/gcc/target.def b/gcc/target.def
index 0443390..6aaa9e6 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -3995,6 +3995,14 @@ the CFI label attached to the insn, @var{pattern} is the pattern of\n\
 the insn and @var{index} is @code{UNSPEC_INDEX} or @code{UNSPECV_INDEX}.",
  void, (const char *label, rtx pattern, int index), NULL)
 
+/* This target hook allows the backend to map GCC DWARF CFA reg-note to
+   architecture specific DWARF call frame instruction.  */
+DEFHOOK
+(dwarf_map_regnote_to_cfa,
+ "Maps the incoming GCC DWARF CFA reg-note to architecture specific DWARF call\
+ frame instruction.",
+ enum dwarf_call_frame_info, (enum reg_note), default_dwarf_map_regnote_to_cfa)
+
 /* ??? Documenting this hook requires a GFDL license grant.  */
 DEFHOOK_UNDOC
 (stdarg_optimize_hook,
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 2f2abd3..df07911 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1711,6 +1711,17 @@ default_dwarf_frame_reg_mode (int regno)
   return save_mode;
 }
 
+/* Determine the correct mode for a Dwarf frame register that represents
+   register REGNO.  */
+
+enum dwarf_call_frame_info
+default_dwarf_map_regnote_to_cfa (enum reg_note note)
+{
+  const char *name = GET_REG_NOTE_NAME (note);
+  error ("This target doesn't support %s", name);
+  gcc_unreachable ();
+}
+
 /* To be used by targets where reg_raw_mode doesn't return the right
    mode for registers used in apply_builtin_return and apply_builtin_arg.  */
 
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index a5565f5..6b1c47f 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -221,6 +221,8 @@ extern int default_jump_align_max_skip (rtx_insn *);
 extern section * default_function_section(tree decl, enum node_frequency freq,
 					  bool startup, bool exit);
 extern machine_mode default_dwarf_frame_reg_mode (int);
+extern enum dwarf_call_frame_info default_dwarf_map_regnote_to_cfa
+  (enum reg_note note);
 extern machine_mode default_get_reg_raw_mode (int);
 extern bool default_keep_leaf_when_profiled ();
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index b9a7989..2d3bcb9 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -1543,14 +1543,6 @@ inline rtvec rtx_jump_table_data::get_labels () const
    question.  */
 #define ENTRY_VALUE_EXP(RTX) (RTL_CHECKC1 (RTX, 0, ENTRY_VALUE).rt_rtx)
 
-enum reg_note
-{
-#define DEF_REG_NOTE(NAME) NAME,
-#include "reg-notes.def"
-#undef DEF_REG_NOTE
-  REG_NOTE_MAX
-};
-
 /* Define macros to extract and insert the reg-note kind in an EXPR_LIST.  */
 #define REG_NOTE_KIND(LINK) ((enum reg_note) GET_MODE (LINK))
 #define PUT_REG_NOTE_KIND(LINK, KIND) \
diff --git a/gcc/coretypes.h b/gcc/coretypes.h
index 8eb33cc..915ac5a 100644
--- a/gcc/coretypes.h
+++ b/gcc/coretypes.h
@@ -253,6 +253,14 @@ enum warn_strict_overflow_code
   WARN_STRICT_OVERFLOW_MAGNITUDE = 5
 };
 
+enum reg_note
+{
+#define DEF_REG_NOTE(NAME) NAME,
+#include "reg-notes.def"
+#undef DEF_REG_NOTE
+  REG_NOTE_MAX
+};
+
 /* The type of an alias set.  Code currently assumes that variables of
    this type can take the values 0 (the alias set which aliases
    everything) and -1 (sometimes indicating that the alias set is
@@ -364,6 +372,7 @@ typedef unsigned char uchar;
 #include "signop.h"
 #include "wide-int.h" 
 #include "double-int.h"
+#include "dwarf2.h"
 #include "real.h"
 #include "fixed-value.h"
 #include "hash-table.h"
diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def
index ead4a9f..175da11 100644
--- a/gcc/reg-notes.def
+++ b/gcc/reg-notes.def
@@ -177,6 +177,11 @@ REG_NOTE (CFA_WINDOW_SAVE)
    the rest of the compiler as a CALL_INSN.  */
 REG_NOTE (CFA_FLUSH_QUEUE)
 
+/* Attached to insns that are RTX_FRAME_RELATED_P, toggling the mangling status
+   of return address.  Currently it's only used by AArch64.  The argument is
+   ignored.  */
+REG_NOTE (CFA_TOGGLE_RA_MANGLE)
+
 /* Indicates what exception region an INSN belongs in.  This is used
    to indicate what region to which a call may throw.  REGION 0
    indicates that a call cannot throw at all.  REGION -1 indicates
diff --git a/gcc/combine-stack-adj.c b/gcc/combine-stack-adj.c
index 20cd59a..9ec14a3 100644
--- a/gcc/combine-stack-adj.c
+++ b/gcc/combine-stack-adj.c
@@ -208,6 +208,7 @@ no_unhandled_cfa (rtx_insn *insn)
       case REG_CFA_SET_VDRAP:
       case REG_CFA_WINDOW_SAVE:
       case REG_CFA_FLUSH_QUEUE:
+      case REG_CFA_TOGGLE_RA_MANGLE:
 	return false;
       }
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index 2748e2f..3f574b3 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -1336,6 +1336,17 @@ dwarf2out_frame_debug_cfa_window_save (void)
   add_cfi (cfi);
 }
 
+/* A subroutine of dwarf2out_frame_debug, process a REG_TOGGLE_RA_MANGLE.  */
+
+static void
+dwarf2out_frame_debug_cfa_toggle_ra_mangle (void)
+{
+  dw_cfi_ref cfi = new_cfi ();
+
+  cfi->dw_cfi_opc = targetm.dwarf_map_regnote_to_cfa (REG_CFA_TOGGLE_RA_MANGLE);
+  add_cfi (cfi);
+}
+
 /* Record call frame debugging information for an expression EXPR,
    which either sets SP or FP (adjusting how we calculate the frame
    address) or saves a register to the stack or another register.
@@ -2108,6 +2119,11 @@ dwarf2out_frame_debug (rtx_insn *insn)
 	handled_one = true;
 	break;
 
+      case REG_CFA_TOGGLE_RA_MANGLE:
+	dwarf2out_frame_debug_cfa_toggle_ra_mangle ();
+	handled_one = true;
+	break;
+
       default:
 	break;
       }
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 4b62e05..07227fc 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -9776,6 +9776,10 @@ exceptions are enabled, GCC will output this information not matter
 how you define @code{DWARF2_FRAME_INFO}.
 @end defmac
 
+@deftypefn {Target Hook} {enum dwarf_call_frame_info} TARGET_DWARF_MAP_REGNOTE_TO_CFA (enum @var{reg_note})
+Maps the incoming GCC DWARF CFA reg-note to architecture specific DWARF call frame instruction.
+@end deftypefn
+
 @deftypefn {Target Hook} {enum unwind_info_type} TARGET_DEBUG_UNWIND_INFO (void)
 This hook defines the mechanism that will be used for describing frame
 unwind information to the debugger.  Normally the hook will return
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index ea74d37..bdb3c51 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -7078,6 +7078,8 @@ exceptions are enabled, GCC will output this information not matter
 how you define @code{DWARF2_FRAME_INFO}.
 @end defmac
 
+@hook TARGET_DWARF_MAP_REGNOTE_TO_CFA
+
 @hook TARGET_DEBUG_UNWIND_INFO
 
 @defmac DWARF2_ASM_LINE_DEBUG_INFO

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 3bcad76..7c16457 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -983,6 +983,18 @@ aarch64_dbx_register_number (unsigned regno)
    return DWARF_FRAME_REGISTERS;
 }
 
+/* Implement target hook TARGET_DWARF_MAP_REGNOTE_TO_CFA.
+   Maps GCC reg-note NOTE into AArch64's architecture DWARF CFA instruction.
+   Currently, only REG_CFA_TOGGLE_RA_MANGLE is expected to reach here.  */
+
+enum dwarf_call_frame_info
+aarch64_map_regnote_to_cfa (enum reg_note note)
+{
+  gcc_assert (note == REG_CFA_TOGGLE_RA_MANGLE);
+
+  return DW_CFA_GNU_window_save;
+}
+
 /* Return TRUE if MODE is any of the large INT modes.  */
 static bool
 aarch64_vect_struct_mode_p (machine_mode mode)
@@ -3553,7 +3565,11 @@ aarch64_expand_prologue (void)
 
   /* Sign return address for functions.  */
   if (aarch64_return_address_signing_enabled ())
-    emit_insn (gen_pacisp ());
+    {
+      insn = emit_insn (gen_pacisp ());
+      add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
+      RTX_FRAME_RELATED_P (insn) = 1;
+    }
 
   if (flag_stack_usage_info)
     current_function_static_stack_size = frame_size;
@@ -3707,7 +3723,11 @@ aarch64_expand_epilogue (bool for_sibcall)
     */
   if (aarch64_return_address_signing_enabled ()
       && (for_sibcall || !TARGET_ARMV8_3 || crtl->calls_eh_return))
-    emit_insn (gen_autisp ());
+    {
+      insn = emit_insn (gen_autisp ());
+      add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
+      RTX_FRAME_RELATED_P (insn) = 1;
+    }
 
   /* Stack adjustment for exception handler.  */
   if (crtl->calls_eh_return)
@@ -14730,6 +14750,9 @@ aarch64_excess_precision (enum excess_precision_type type)
 #undef TARGET_C_EXCESS_PRECISION
 #define TARGET_C_EXCESS_PRECISION aarch64_excess_precision
 
+#undef TARGET_DWARF_MAP_REGNOTE_TO_CFA
+#define TARGET_DWARF_MAP_REGNOTE_TO_CFA aarch64_map_regnote_to_cfa
+
 #undef  TARGET_EXPAND_BUILTIN
 #define TARGET_EXPAND_BUILTIN aarch64_expand_builtin
 

Richard Earnshaw (lists) Jan. 17, 2017, 1:57 p.m. UTC | #4
On 16/01/17 14:29, Jiong Wang wrote:
> On 13/01/17 18:02, Jiong Wang wrote:

>> On 13/01/17 16:09, Richard Earnshaw (lists) wrote:

>>> On 06/01/17 11:47, Jiong Wang wrote:

>>>>

>>>> This patch is an update on DWARF generation for return address signing.

>>>>

>>>> According to new proposal, we simply needs to generate

>>>> REG_CFA_WINDOW_SAVE

>>>> annotation.

>>>>

>>>> gcc/

>>>>

>>>> 2017-01-06  Jiong Wang  <jiong.wang@arm.com>

>>>>

>>>>          * config/aarch64/aarch64.c (aarch64_expand_prologue): Generate

>>>> dwarf

>>>>          annotation (REG_CFA_WINDOW_SAVE) for return address signing.

>>>>          (aarch64_expand_epilogue): Likewise.

>>>>

>>>>

>>> I don't think we should be overloading REG_CFA_WINDOW_SAVE internally in

>>> the compiler -- it's one thing to do it in the dwarf output tables, but

>>> quite another to be doing it elsewhere in the compiler.

>>>

>>> Instead we should create a new reg note kind and use that, but in the

>>> final dwarf output then emit the overloaded opcode.

>>

>> I can see the reason for doing this is if you want to seperate the

>> interpretion

>> of GCC CFA reg-note and the final DWARF CFA operation.  My

>> understanding is all

>> reg notes defined in gcc/reg-note.def should have general meaning,

>> even the

>> CFA_WINDOW_SAVE.  For those which are architecture specific we might

>> need a

>> mechanism to define them in backend only.

>>    For general reg-notes in gcc/reg-note.def, they are not always have

>> the

>> corresponding standard DWARF CFA operation, for example CFA_WINDOW_SAVE,

>> therefore if we want to achieve what you described, I think we also

>> need to

>> define a new target hook which maps a GCC CFA reg-note into

>> architecture DWARF

>> CFA operation.

>>

>> Regards,

>> Jiong

>>

>>

> Here is the patch.

> 


Hmm, I really wasn't expecting any more than something like the
following in dwarf2cfi.c:

@@ -2098,7 +2098,9 @@ dwarf2out_frame_debug (rtx_insn *insn)
        handled_one = true;
        break;

+      case REG_CFA_TOGGLE_RA_MANGLE:
       case REG_CFA_WINDOW_SAVE:
+       /* We overload both of these operations onto the same DWARF
opcode.  */
        dwarf2out_frame_debug_cfa_window_save ();
        handled_one = true;
        break;

This keeps the two reg notes separate within the compiler, but emits the
same dwarf operation during final output.  This avoids the need for new
hooks or anything more complicated.

R.

> Introduced one new target hook TARGET_DWARF_MAP_REGNOTE_TO_CFA.  The

> purpose is

> to allow GCC to map DWARF CFA reg notes in reg-note.def, which looks to

> me have

> generic meaning, into target private DWARF CFI if there is no standard

> DWARF CFI

> mapping.

> 

> One new GCC reg-note REG_TOGGLE_RA_MANGLE introduced as well, currently,

> it's

> only used by AArch64 to implement return address signing and is mapped to

> AArch64's target private DWARF CFI.

> 

> Does this approach and implementation looks OK?

> 

> I can come up with seperate patches to define this hook on Sparc for

> CFA_WINDOW_SAVE, and to remove redundant including of dwarf2.h although

> there is

> "ifdef" protector in header file.

> 

> The default hook implementation "default_dwarf_map_regnote_to_cfa" in

> targhooks.c used the types "enum reg_note" and "enum dwarf_call_frame_info"

> which is not included in coretypes.h thus this patch has several change in

> header files.  I have done X86 bootstrap to make sure no build

> breakage.  I'd

> appreciate there is better ideas to handle these type define.

> 

> Thanks.

> 

> gcc/ChangeLog:

> 

> 2017-01-16  Jiong Wang  <jiong.wang@arm.com>

> 

>         * target.def (dwarf_map_regnote_to_cfa): New hook.

>         * targhooks.c (default_dwarf_map_regnote_to_cfa): Default

> implementation

>         for TARGET_DWARF_MAP_REGNOTE_TO_CFA.

>         * targhooks.h (default_dwarf_map_regnote_to_cfa): New declaration.

>         * rtl.h (enum reg_note): Move enum reg_note to...

>         * coretypes.h: ... here.

>         (dwarf2.h): New include file.

>         * reg-notes.def (CFA_TOGGLE_RA_MANGLE): New reg-note.

>         * combine-stack-adj.c (no_unhandled_cfa): Handle

>         REG_CFA_TOGGLE_RA_MANGLE.

>         * dwarf2cfi.c (dwarf2out_frame_debug_cfa_toggle_ra_mangle): New

>         function.

>         (dwarf2out_frame_debug): Handle REG_CFA_TOGGLE_RA_MANGLE.

>         * doc/tm.texi: Regenerate.

>         * doc/tm.texi.in: Documents TARGET_DWARF_MAP_REGNOTE_TO_CFA.

>         * config/aarch64/aarch64.c (aarch64_map_regnote_to_cfa): Implements

>         TARGET_DWARF_MAP_REGNOTE_TO_CFA.

>         (aarch64_expand_prologue): Generate DWARF info for return address

>         signing.

>         (aarch64_expand_epilogue): Likewise.

>         (TARGET_DWARF_MAP_REGNOTE_TO_CFA): Define.

> 

> 

> 1.patch

> 

> 

> diff --git a/gcc/target.def b/gcc/target.def

> index 0443390..6aaa9e6 100644

> --- a/gcc/target.def

> +++ b/gcc/target.def

> @@ -3995,6 +3995,14 @@ the CFI label attached to the insn, @var{pattern} is the pattern of\n\

>  the insn and @var{index} is @code{UNSPEC_INDEX} or @code{UNSPECV_INDEX}.",

>   void, (const char *label, rtx pattern, int index), NULL)

>  

> +/* This target hook allows the backend to map GCC DWARF CFA reg-note to

> +   architecture specific DWARF call frame instruction.  */

> +DEFHOOK

> +(dwarf_map_regnote_to_cfa,

> + "Maps the incoming GCC DWARF CFA reg-note to architecture specific DWARF call\

> + frame instruction.",

> + enum dwarf_call_frame_info, (enum reg_note), default_dwarf_map_regnote_to_cfa)

> +

>  /* ??? Documenting this hook requires a GFDL license grant.  */

>  DEFHOOK_UNDOC

>  (stdarg_optimize_hook,

> diff --git a/gcc/targhooks.c b/gcc/targhooks.c

> index 2f2abd3..df07911 100644

> --- a/gcc/targhooks.c

> +++ b/gcc/targhooks.c

> @@ -1711,6 +1711,17 @@ default_dwarf_frame_reg_mode (int regno)

>    return save_mode;

>  }

>  

> +/* Determine the correct mode for a Dwarf frame register that represents

> +   register REGNO.  */

> +

> +enum dwarf_call_frame_info

> +default_dwarf_map_regnote_to_cfa (enum reg_note note)

> +{

> +  const char *name = GET_REG_NOTE_NAME (note);

> +  error ("This target doesn't support %s", name);

> +  gcc_unreachable ();

> +}

> +

>  /* To be used by targets where reg_raw_mode doesn't return the right

>     mode for registers used in apply_builtin_return and apply_builtin_arg.  */

>  

> diff --git a/gcc/targhooks.h b/gcc/targhooks.h

> index a5565f5..6b1c47f 100644

> --- a/gcc/targhooks.h

> +++ b/gcc/targhooks.h

> @@ -221,6 +221,8 @@ extern int default_jump_align_max_skip (rtx_insn *);

>  extern section * default_function_section(tree decl, enum node_frequency freq,

>  					  bool startup, bool exit);

>  extern machine_mode default_dwarf_frame_reg_mode (int);

> +extern enum dwarf_call_frame_info default_dwarf_map_regnote_to_cfa

> +  (enum reg_note note);

>  extern machine_mode default_get_reg_raw_mode (int);

>  extern bool default_keep_leaf_when_profiled ();

>  

> diff --git a/gcc/rtl.h b/gcc/rtl.h

> index b9a7989..2d3bcb9 100644

> --- a/gcc/rtl.h

> +++ b/gcc/rtl.h

> @@ -1543,14 +1543,6 @@ inline rtvec rtx_jump_table_data::get_labels () const

>     question.  */

>  #define ENTRY_VALUE_EXP(RTX) (RTL_CHECKC1 (RTX, 0, ENTRY_VALUE).rt_rtx)

>  

> -enum reg_note

> -{

> -#define DEF_REG_NOTE(NAME) NAME,

> -#include "reg-notes.def"

> -#undef DEF_REG_NOTE

> -  REG_NOTE_MAX

> -};

> -

>  /* Define macros to extract and insert the reg-note kind in an EXPR_LIST.  */

>  #define REG_NOTE_KIND(LINK) ((enum reg_note) GET_MODE (LINK))

>  #define PUT_REG_NOTE_KIND(LINK, KIND) \

> diff --git a/gcc/coretypes.h b/gcc/coretypes.h

> index 8eb33cc..915ac5a 100644

> --- a/gcc/coretypes.h

> +++ b/gcc/coretypes.h

> @@ -253,6 +253,14 @@ enum warn_strict_overflow_code

>    WARN_STRICT_OVERFLOW_MAGNITUDE = 5

>  };

>  

> +enum reg_note

> +{

> +#define DEF_REG_NOTE(NAME) NAME,

> +#include "reg-notes.def"

> +#undef DEF_REG_NOTE

> +  REG_NOTE_MAX

> +};

> +

>  /* The type of an alias set.  Code currently assumes that variables of

>     this type can take the values 0 (the alias set which aliases

>     everything) and -1 (sometimes indicating that the alias set is

> @@ -364,6 +372,7 @@ typedef unsigned char uchar;

>  #include "signop.h"

>  #include "wide-int.h" 

>  #include "double-int.h"

> +#include "dwarf2.h"

>  #include "real.h"

>  #include "fixed-value.h"

>  #include "hash-table.h"

> diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def

> index ead4a9f..175da11 100644

> --- a/gcc/reg-notes.def

> +++ b/gcc/reg-notes.def

> @@ -177,6 +177,11 @@ REG_NOTE (CFA_WINDOW_SAVE)

>     the rest of the compiler as a CALL_INSN.  */

>  REG_NOTE (CFA_FLUSH_QUEUE)

>  

> +/* Attached to insns that are RTX_FRAME_RELATED_P, toggling the mangling status

> +   of return address.  Currently it's only used by AArch64.  The argument is

> +   ignored.  */

> +REG_NOTE (CFA_TOGGLE_RA_MANGLE)

> +

>  /* Indicates what exception region an INSN belongs in.  This is used

>     to indicate what region to which a call may throw.  REGION 0

>     indicates that a call cannot throw at all.  REGION -1 indicates

> diff --git a/gcc/combine-stack-adj.c b/gcc/combine-stack-adj.c

> index 20cd59a..9ec14a3 100644

> --- a/gcc/combine-stack-adj.c

> +++ b/gcc/combine-stack-adj.c

> @@ -208,6 +208,7 @@ no_unhandled_cfa (rtx_insn *insn)

>        case REG_CFA_SET_VDRAP:

>        case REG_CFA_WINDOW_SAVE:

>        case REG_CFA_FLUSH_QUEUE:

> +      case REG_CFA_TOGGLE_RA_MANGLE:

>  	return false;

>        }

> diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c

> index 2748e2f..3f574b3 100644

> --- a/gcc/dwarf2cfi.c

> +++ b/gcc/dwarf2cfi.c

> @@ -1336,6 +1336,17 @@ dwarf2out_frame_debug_cfa_window_save (void)

>    add_cfi (cfi);

>  }

>  

> +/* A subroutine of dwarf2out_frame_debug, process a REG_TOGGLE_RA_MANGLE.  */

> +

> +static void

> +dwarf2out_frame_debug_cfa_toggle_ra_mangle (void)

> +{

> +  dw_cfi_ref cfi = new_cfi ();

> +

> +  cfi->dw_cfi_opc = targetm.dwarf_map_regnote_to_cfa (REG_CFA_TOGGLE_RA_MANGLE);

> +  add_cfi (cfi);

> +}

> +

>  /* Record call frame debugging information for an expression EXPR,

>     which either sets SP or FP (adjusting how we calculate the frame

>     address) or saves a register to the stack or another register.

> @@ -2108,6 +2119,11 @@ dwarf2out_frame_debug (rtx_insn *insn)

>  	handled_one = true;

>  	break;

>  

> +      case REG_CFA_TOGGLE_RA_MANGLE:

> +	dwarf2out_frame_debug_cfa_toggle_ra_mangle ();

> +	handled_one = true;

> +	break;

> +

>        default:

>  	break;

>        }

> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi

> index 4b62e05..07227fc 100644

> --- a/gcc/doc/tm.texi

> +++ b/gcc/doc/tm.texi

> @@ -9776,6 +9776,10 @@ exceptions are enabled, GCC will output this information not matter

>  how you define @code{DWARF2_FRAME_INFO}.

>  @end defmac

>  

> +@deftypefn {Target Hook} {enum dwarf_call_frame_info} TARGET_DWARF_MAP_REGNOTE_TO_CFA (enum @var{reg_note})

> +Maps the incoming GCC DWARF CFA reg-note to architecture specific DWARF call frame instruction.

> +@end deftypefn

> +

>  @deftypefn {Target Hook} {enum unwind_info_type} TARGET_DEBUG_UNWIND_INFO (void)

>  This hook defines the mechanism that will be used for describing frame

>  unwind information to the debugger.  Normally the hook will return

> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in

> index ea74d37..bdb3c51 100644

> --- a/gcc/doc/tm.texi.in

> +++ b/gcc/doc/tm.texi.in

> @@ -7078,6 +7078,8 @@ exceptions are enabled, GCC will output this information not matter

>  how you define @code{DWARF2_FRAME_INFO}.

>  @end defmac

>  

> +@hook TARGET_DWARF_MAP_REGNOTE_TO_CFA

> +

>  @hook TARGET_DEBUG_UNWIND_INFO

>  

>  @defmac DWARF2_ASM_LINE_DEBUG_INFO

> 

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c

> index 3bcad76..7c16457 100644

> --- a/gcc/config/aarch64/aarch64.c

> +++ b/gcc/config/aarch64/aarch64.c

> @@ -983,6 +983,18 @@ aarch64_dbx_register_number (unsigned regno)

>     return DWARF_FRAME_REGISTERS;

>  }

>  

> +/* Implement target hook TARGET_DWARF_MAP_REGNOTE_TO_CFA.

> +   Maps GCC reg-note NOTE into AArch64's architecture DWARF CFA instruction.

> +   Currently, only REG_CFA_TOGGLE_RA_MANGLE is expected to reach here.  */

> +

> +enum dwarf_call_frame_info

> +aarch64_map_regnote_to_cfa (enum reg_note note)

> +{

> +  gcc_assert (note == REG_CFA_TOGGLE_RA_MANGLE);

> +

> +  return DW_CFA_GNU_window_save;

> +}

> +

>  /* Return TRUE if MODE is any of the large INT modes.  */

>  static bool

>  aarch64_vect_struct_mode_p (machine_mode mode)

> @@ -3553,7 +3565,11 @@ aarch64_expand_prologue (void)

>  

>    /* Sign return address for functions.  */

>    if (aarch64_return_address_signing_enabled ())

> -    emit_insn (gen_pacisp ());

> +    {

> +      insn = emit_insn (gen_pacisp ());

> +      add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);

> +      RTX_FRAME_RELATED_P (insn) = 1;

> +    }

>  

>    if (flag_stack_usage_info)

>      current_function_static_stack_size = frame_size;

> @@ -3707,7 +3723,11 @@ aarch64_expand_epilogue (bool for_sibcall)

>      */

>    if (aarch64_return_address_signing_enabled ()

>        && (for_sibcall || !TARGET_ARMV8_3 || crtl->calls_eh_return))

> -    emit_insn (gen_autisp ());

> +    {

> +      insn = emit_insn (gen_autisp ());

> +      add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);

> +      RTX_FRAME_RELATED_P (insn) = 1;

> +    }

>  

>    /* Stack adjustment for exception handler.  */

>    if (crtl->calls_eh_return)

> @@ -14730,6 +14750,9 @@ aarch64_excess_precision (enum excess_precision_type type)

>  #undef TARGET_C_EXCESS_PRECISION

>  #define TARGET_C_EXCESS_PRECISION aarch64_excess_precision

>  

> +#undef TARGET_DWARF_MAP_REGNOTE_TO_CFA

> +#define TARGET_DWARF_MAP_REGNOTE_TO_CFA aarch64_map_regnote_to_cfa

> +

>  #undef  TARGET_EXPAND_BUILTIN

>  #define TARGET_EXPAND_BUILTIN aarch64_expand_builtin

>  

>
Jiong Wang Jan. 17, 2017, 3:11 p.m. UTC | #5
On 17/01/17 13:57, Richard Earnshaw (lists) wrote:
> On 16/01/17 14:29, Jiong Wang wrote:

>>

>>> I can see the reason for doing this is if you want to seperate the

>>> interpretion

>>> of GCC CFA reg-note and the final DWARF CFA operation.  My

>>> understanding is all

>>> reg notes defined in gcc/reg-note.def should have general meaning,

>>> even the

>>> CFA_WINDOW_SAVE.  For those which are architecture specific we might

>>> need a

>>> mechanism to define them in backend only.

>>>     For general reg-notes in gcc/reg-note.def, they are not always have

>>> the

>>> corresponding standard DWARF CFA operation, for example CFA_WINDOW_SAVE,

>>> therefore if we want to achieve what you described, I think we also

>>> need to

>>> define a new target hook which maps a GCC CFA reg-note into

>>> architecture DWARF

>>> CFA operation.

>>>

>>> Regards,

>>> Jiong

>>>

>>>

>> Here is the patch.

>>

> Hmm, I really wasn't expecting any more than something like the

> following in dwarf2cfi.c:

>

> @@ -2098,7 +2098,9 @@ dwarf2out_frame_debug (rtx_insn *insn)

>          handled_one = true;

>          break;

>

> +      case REG_CFA_TOGGLE_RA_MANGLE:

>         case REG_CFA_WINDOW_SAVE:

> +       /* We overload both of these operations onto the same DWARF

> opcode.  */

>          dwarf2out_frame_debug_cfa_window_save ();

>          handled_one = true;

>          break;

>

> This keeps the two reg notes separate within the compiler, but emits the

> same dwarf operation during final output.  This avoids the need for new

> hooks or anything more complicated.


This was my initial thoughts and the patch would be very small as you've
demonstrated.  I later moved to this complexer patch as I am thinking it's
better to completely treat notes in reg-notes.def as having generic meaning and
maps them to standard DWARF CFA if there is, otherwise maps them to target
private DWARF CFA through this new hook.  This give other targets a chance to
map, for example REG_CFA_TOGGLE_RA_MANGLE, to their architecture DWARF number.

The introduction of new hook looks be very low risk in this stage, the only
painful thing is the header file needs to be reorganized as we need to use some
DWARF type and reg-note type in targhooks.c.

Anyway, if the new hook patch is too heavy, I have attached the the simplified
version which simply defines the new REG_CFA_TOGGLE_RA_MANGLE and maps to same
code of REG_CFA_WINDOW_SAVE.


gcc/

2017-01-17  Jiong Wang  <jiong.wang@arm.com>

         * reg-notes.def (CFA_TOGGLE_RA_MANGLE): New reg-note.
         * combine-stack-adj.c (no_unhandled_cfa): Handle
         REG_CFA_TOGGLE_RA_MANGLE.
         * dwarf2cfi.c
         (dwarf2out_frame_debug): Handle REG_CFA_TOGGLE_RA_MANGLE.
         * config/aarch64/aarch64.c (aarch64_expand_prologue): Generates DWARF
         info for return address signing.
         (aarch64_expand_epilogue): Likewise.diff --git a/gcc/combine-stack-adj.c b/gcc/combine-stack-adj.c
index 20cd59ad08329e9f4f834bfc01d6f9ccc4485283..9ec14a3e44363f35f6419c38233ce5eebddd3458 100644
--- a/gcc/combine-stack-adj.c
+++ b/gcc/combine-stack-adj.c
@@ -208,6 +208,7 @@ no_unhandled_cfa (rtx_insn *insn)
       case REG_CFA_SET_VDRAP:
       case REG_CFA_WINDOW_SAVE:
       case REG_CFA_FLUSH_QUEUE:
+      case REG_CFA_TOGGLE_RA_MANGLE:
 	return false;
       }
 
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 3bcad76b68b6ea7c9d75d150d79c45fb74d6bf0d..6451b08191cf1a44aed502930da8603111f6e8ca 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3553,7 +3553,11 @@ aarch64_expand_prologue (void)
 
   /* Sign return address for functions.  */
   if (aarch64_return_address_signing_enabled ())
-    emit_insn (gen_pacisp ());
+    {
+      insn = emit_insn (gen_pacisp ());
+      add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
+      RTX_FRAME_RELATED_P (insn) = 1;
+    }
 
   if (flag_stack_usage_info)
     current_function_static_stack_size = frame_size;
@@ -3707,7 +3711,11 @@ aarch64_expand_epilogue (bool for_sibcall)
     */
   if (aarch64_return_address_signing_enabled ()
       && (for_sibcall || !TARGET_ARMV8_3 || crtl->calls_eh_return))
-    emit_insn (gen_autisp ());
+    {
+      insn = emit_insn (gen_autisp ());
+      add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
+      RTX_FRAME_RELATED_P (insn) = 1;
+    }
 
   /* Stack adjustment for exception handler.  */
   if (crtl->calls_eh_return)
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index 2748e2fa48e4794181496b26df9b51b7e51e7b84..2a527c9fecab091dccb417492e5dbb2ade244be2 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -2098,7 +2098,9 @@ dwarf2out_frame_debug (rtx_insn *insn)
 	handled_one = true;
 	break;
 
+      case REG_CFA_TOGGLE_RA_MANGLE:
       case REG_CFA_WINDOW_SAVE:
+	/* We overload both of these operations onto the same DWARF opcode.  */
 	dwarf2out_frame_debug_cfa_window_save ();
 	handled_one = true;
 	break;
diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def
index ead4a9f58e8621288ee765e029c673640fdf38f4..175da119b6a534b04bd154f2c69dd087afd474ea 100644
--- a/gcc/reg-notes.def
+++ b/gcc/reg-notes.def
@@ -177,6 +177,11 @@ REG_NOTE (CFA_WINDOW_SAVE)
    the rest of the compiler as a CALL_INSN.  */
 REG_NOTE (CFA_FLUSH_QUEUE)
 
+/* Attached to insns that are RTX_FRAME_RELATED_P, toggling the mangling status
+   of return address.  Currently it's only used by AArch64.  The argument is
+   ignored.  */
+REG_NOTE (CFA_TOGGLE_RA_MANGLE)
+
 /* Indicates what exception region an INSN belongs in.  This is used
    to indicate what region to which a call may throw.  REGION 0
    indicates that a call cannot throw at all.  REGION -1 indicates

Richard Earnshaw (lists) Jan. 19, 2017, 2:06 p.m. UTC | #6
On 17/01/17 15:11, Jiong Wang wrote:
> 

> 

> On 17/01/17 13:57, Richard Earnshaw (lists) wrote:

>> On 16/01/17 14:29, Jiong Wang wrote:

>>>

>>>> I can see the reason for doing this is if you want to seperate the

>>>> interpretion

>>>> of GCC CFA reg-note and the final DWARF CFA operation.  My

>>>> understanding is all

>>>> reg notes defined in gcc/reg-note.def should have general meaning,

>>>> even the

>>>> CFA_WINDOW_SAVE.  For those which are architecture specific we might

>>>> need a

>>>> mechanism to define them in backend only.

>>>>     For general reg-notes in gcc/reg-note.def, they are not always have

>>>> the

>>>> corresponding standard DWARF CFA operation, for example

>>>> CFA_WINDOW_SAVE,

>>>> therefore if we want to achieve what you described, I think we also

>>>> need to

>>>> define a new target hook which maps a GCC CFA reg-note into

>>>> architecture DWARF

>>>> CFA operation.

>>>>

>>>> Regards,

>>>> Jiong

>>>>

>>>>

>>> Here is the patch.

>>>

>> Hmm, I really wasn't expecting any more than something like the

>> following in dwarf2cfi.c:

>>

>> @@ -2098,7 +2098,9 @@ dwarf2out_frame_debug (rtx_insn *insn)

>>          handled_one = true;

>>          break;

>>

>> +      case REG_CFA_TOGGLE_RA_MANGLE:

>>         case REG_CFA_WINDOW_SAVE:

>> +       /* We overload both of these operations onto the same DWARF

>> opcode.  */

>>          dwarf2out_frame_debug_cfa_window_save ();

>>          handled_one = true;

>>          break;

>>

>> This keeps the two reg notes separate within the compiler, but emits the

>> same dwarf operation during final output.  This avoids the need for new

>> hooks or anything more complicated.

> 

> This was my initial thoughts and the patch would be very small as you've

> demonstrated.  I later moved to this complexer patch as I am thinking it's

> better to completely treat notes in reg-notes.def as having generic

> meaning and

> maps them to standard DWARF CFA if there is, otherwise maps them to target

> private DWARF CFA through this new hook.  This give other targets a

> chance to

> map, for example REG_CFA_TOGGLE_RA_MANGLE, to their architecture DWARF

> number.

> 

> The introduction of new hook looks be very low risk in this stage, the only

> painful thing is the header file needs to be reorganized as we need to

> use some

> DWARF type and reg-note type in targhooks.c.

> 

> Anyway, if the new hook patch is too heavy, I have attached the the

> simplified

> version which simply defines the new REG_CFA_TOGGLE_RA_MANGLE and maps

> to same

> code of REG_CFA_WINDOW_SAVE.

> 

> 


Yes, this is much more like what I had in mind.

OK.

R.

> gcc/

> 

> 2017-01-17  Jiong Wang  <jiong.wang@arm.com>

> 

>         * reg-notes.def (CFA_TOGGLE_RA_MANGLE): New reg-note.

>         * combine-stack-adj.c (no_unhandled_cfa): Handle

>         REG_CFA_TOGGLE_RA_MANGLE.

>         * dwarf2cfi.c

>         (dwarf2out_frame_debug): Handle REG_CFA_TOGGLE_RA_MANGLE.

>         * config/aarch64/aarch64.c (aarch64_expand_prologue): Generates

> DWARF

>         info for return address signing.

>         (aarch64_expand_epilogue): Likewise.

> 

> 

> k.patch

> 

> 

> diff --git a/gcc/combine-stack-adj.c b/gcc/combine-stack-adj.c

> index 20cd59ad08329e9f4f834bfc01d6f9ccc4485283..9ec14a3e44363f35f6419c38233ce5eebddd3458 100644

> --- a/gcc/combine-stack-adj.c

> +++ b/gcc/combine-stack-adj.c

> @@ -208,6 +208,7 @@ no_unhandled_cfa (rtx_insn *insn)

>        case REG_CFA_SET_VDRAP:

>        case REG_CFA_WINDOW_SAVE:

>        case REG_CFA_FLUSH_QUEUE:

> +      case REG_CFA_TOGGLE_RA_MANGLE:

>  	return false;

>        }

>  

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c

> index 3bcad76b68b6ea7c9d75d150d79c45fb74d6bf0d..6451b08191cf1a44aed502930da8603111f6e8ca 100644

> --- a/gcc/config/aarch64/aarch64.c

> +++ b/gcc/config/aarch64/aarch64.c

> @@ -3553,7 +3553,11 @@ aarch64_expand_prologue (void)

>  

>    /* Sign return address for functions.  */

>    if (aarch64_return_address_signing_enabled ())

> -    emit_insn (gen_pacisp ());

> +    {

> +      insn = emit_insn (gen_pacisp ());

> +      add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);

> +      RTX_FRAME_RELATED_P (insn) = 1;

> +    }

>  

>    if (flag_stack_usage_info)

>      current_function_static_stack_size = frame_size;

> @@ -3707,7 +3711,11 @@ aarch64_expand_epilogue (bool for_sibcall)

>      */

>    if (aarch64_return_address_signing_enabled ()

>        && (for_sibcall || !TARGET_ARMV8_3 || crtl->calls_eh_return))

> -    emit_insn (gen_autisp ());

> +    {

> +      insn = emit_insn (gen_autisp ());

> +      add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);

> +      RTX_FRAME_RELATED_P (insn) = 1;

> +    }

>  

>    /* Stack adjustment for exception handler.  */

>    if (crtl->calls_eh_return)

> diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c

> index 2748e2fa48e4794181496b26df9b51b7e51e7b84..2a527c9fecab091dccb417492e5dbb2ade244be2 100644

> --- a/gcc/dwarf2cfi.c

> +++ b/gcc/dwarf2cfi.c

> @@ -2098,7 +2098,9 @@ dwarf2out_frame_debug (rtx_insn *insn)

>  	handled_one = true;

>  	break;

>  

> +      case REG_CFA_TOGGLE_RA_MANGLE:

>        case REG_CFA_WINDOW_SAVE:

> +	/* We overload both of these operations onto the same DWARF opcode.  */

>  	dwarf2out_frame_debug_cfa_window_save ();

>  	handled_one = true;

>  	break;

> diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def

> index ead4a9f58e8621288ee765e029c673640fdf38f4..175da119b6a534b04bd154f2c69dd087afd474ea 100644

> --- a/gcc/reg-notes.def

> +++ b/gcc/reg-notes.def

> @@ -177,6 +177,11 @@ REG_NOTE (CFA_WINDOW_SAVE)

>     the rest of the compiler as a CALL_INSN.  */

>  REG_NOTE (CFA_FLUSH_QUEUE)

>  

> +/* Attached to insns that are RTX_FRAME_RELATED_P, toggling the mangling status

> +   of return address.  Currently it's only used by AArch64.  The argument is

> +   ignored.  */

> +REG_NOTE (CFA_TOGGLE_RA_MANGLE)

> +

>  /* Indicates what exception region an INSN belongs in.  This is used

>     to indicate what region to which a call may throw.  REGION 0

>     indicates that a call cannot throw at all.  REGION -1 indicates

>
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 002895a167ce0deb45a5c1726527651af18bb4df..20ed79e5690f45ec121ef516245c686cc0cc82b5 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3553,7 +3553,11 @@  aarch64_expand_prologue (void)
 
   /* Sign return address for functions.  */
   if (aarch64_return_address_signing_enabled ())
-    emit_insn (gen_pacisp ());
+    {
+      insn = emit_insn (gen_pacisp ());
+      add_reg_note (insn, REG_CFA_WINDOW_SAVE, const0_rtx);
+      RTX_FRAME_RELATED_P (insn) = 1;
+    }
 
   if (flag_stack_usage_info)
     current_function_static_stack_size = frame_size;
@@ -3698,7 +3702,11 @@  aarch64_expand_epilogue (bool for_sibcall)
      want to use the CFA of the function which calls eh_return.  */
   if (aarch64_return_address_signing_enabled ()
       && (for_sibcall || !TARGET_ARMV8_3 || crtl->calls_eh_return))
-    emit_insn (gen_autisp ());
+    {
+      insn = emit_insn (gen_autisp ());
+      add_reg_note (insn, REG_CFA_WINDOW_SAVE, const0_rtx);
+      RTX_FRAME_RELATED_P (insn) = 1;
+    }
 
   /* Stack adjustment for exception handler.  */
   if (crtl->calls_eh_return)