[2/5,DWARF] Generate dwarf information for -msign-return-address by introducing new DWARF mapping hook

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.

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
>  
>