This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]