[PATCH 4/4] gas: sframe: handle .cfi_same_value

[Indu Bhagat]

On 5/13/25 8:51 AM, Jens Remus wrote:
> On 13.05.2025 15:13, Jan Beulich wrote:
>> On 11.05.2025 09:35, Indu Bhagat wrote:
>>> Fix PR gas/32953 - sframe: incorrect handling of .cfi_same_value in gas
>>>
>>> As per documentation, .cfi_same_value indicates that: Current value of
>>> register is the same like in the previous frame, i.e. no restoration
>>> needed.  SFrame has no means to encode this information.
>>>
>>> Skip generating FDE, but warn if it is for a register of interest.
>>> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
>>> index a87d464fd5f..f989627193f 100644
>>> --- a/gas/gen-sframe.c
>>> +++ b/gas/gen-sframe.c
>>> @@ -1533,6 +1533,32 @@ sframe_xlate_do_cfi_undefined (const struct sframe_xlate_ctx *xlate_ctx ATTRIBUT
>>>     return SFRAME_XLATE_OK;
>>>   }
>>>   
>>> +/* Translate DW_CFA_same_value into SFrame context.
>>> +
>>> +   DW_CFA_undefined op implies that current value of register is the same like
>> Nit: DW_CFA_same_value and perhaps "indicates" in place of "implies".
>>
>>> +   in the previous frame, i.e. no restoration needed.  In SFrame stack trace,
>>> +   we cannot represent such a semantic.  So, we skip generating an SFrame FDE
>>> +   for this, when a register of interest is used with DW_CFA_same_value.
>> Generally I'm having a hard time seeing when "same value" would need using.
>> Aiui the absence of any information means the register is unchanged (at
>> least as long as mandated so by the ABI). And a register which had its value
>> restored would be indicated by .cfi_restore. Hence it's not clear to me
>> whether "unable to represent" is really applicable here.
> This is about what generation of SFrame should do when encountering
> .cfi_same_value for one of its tracked registers (FP and optionally RA)
> and possibly also the SP register.  If that cannot be properly
> represented in SFrame the generation of the FDE needs to be skipped.
> 
> As I have written in my separate feedback to this patch, I think that
> .cfi_same_value for FP and RA needs to be represented by clearing the
> SFrame information for the respective register.  Ignoring the CFI
> directive (as it had been done so far) or resetting the SFrame
> information for the respective register to the one from FRE[0] would be
> wrong.  This assumes that for FP and RA .cfi_same_value and .cfi_restore
> would result same.
> 
> While .cfi_same_value <SP> (value at call site) and .cfi_restore <SP>
> (value at function entry) may indicate different SP values on x86-64 due
> to the call instruction altering the SP, I think this could be ignored,
> assuming the CFA definition was updated as well.

(Incorporated Jens' feedback elsewhere on the thread.  But replying 
here, to hopefully answer Jan's question on what is meant by "unable to 
represent" with respect to SFrame).

So this is what I have now:

/* Translate DW_CFA_same_value into SFrame context.

    DW_CFA_same_value op implies that current value of register is the 
same like
    in the previous frame, i.e. no restoration needed.  In SFrame stack 
trace
    format, whether or not we can encode "no restoration needed" is case
    specific.

    For SFRAME_CFA_RA_REG, perform an ABI-sensitive check.  For AMD64, 
SFrame
    can only represent the fact that REG_RA is at a fixed offset from 
CFA; IOW,
    restoration is always necessary.  Hence, .cfi_same_value REG_RA 
cannot be
    represented in SFrame.  For AArch64, since RA-tracking is enabled, the
    manner to restore REG_RA is, in theory, encodable.  So, for
    DW_CFA_same_value, reset the SFrame FRE state for REG_RA to indicate 
that
    register does not need restoration.  P.S.: Even though resetting 
just REG_RA
    may be contradicting the AArch64 ABI (as Frame Record contains both 
FP and
    LR), sframe_xlate_do_same_value () does not detect the case and 
assumes the
    users' DW_CFA_same_value SFRAME_CFA_RA_REG has a sound reason.

    SFrame based stacktracers will implement CFA-based SP recovery for 
all ABIs:
    SP for previous frame is based on the applicable CFA-rule.  There is no
    representation in SFrame to indicate "no restoration needed" for 
REG_SP.  So
    skip generating the SFrame FDE if DW_CFA_same_value is seen for
    SFRAME_CFA_SP_REG.

    For SFRAME_CFA_FP_REG, reset the state of the current FRE to 
indicate that
    the value is the same as previous frame.

    Return SFRAME_XLATE_OK if success.  */

static int
sframe_xlate_do_same_value (const struct sframe_xlate_ctx *xlate_ctx,
			    const struct cfi_insn_data *cfi_insn)
{
   /* For most ABIs, "no restoration" semantic (indicated by 
DW_CFA_same_value)
      for SP or RA may be violating ABI rules.  More importantly, SFrame 
cannot
      encode such information.  */
   if (cfi_insn->u.r == SFRAME_CFA_SP_REG
       || (!sframe_ra_tracking_p () && cfi_insn->u.r == SFRAME_CFA_RA_REG))
     {
       as_warn (_("skipping SFrame FDE; %s reg %u in .cfi_same_value"),
	       sframe_register_name (cfi_insn->u.r), cfi_insn->u.r);
       return SFRAME_XLATE_ERR_NOTREPRESENTED; /* Not represented.  */
     }

   struct sframe_row_entry *cur_fre = xlate_ctx->cur_fre;

   if (cfi_insn->u.r == SFRAME_CFA_RA_REG)
     {
       cur_fre->ra_loc = SFRAME_FRE_ELEM_LOC_REG;
       cur_fre->ra_offset = 0;
       cur_fre->merge_candidate = false;
     }
   else if (cfi_insn->u.r == SFRAME_CFA_FP_REG)
     {
       cur_fre->bp_loc = SFRAME_FRE_ELEM_LOC_REG;
       cur_fre->bp_offset = 0;
       cur_fre->merge_candidate = false;
     }

   /* Safe to skip.  */
   return SFRAME_XLATE_OK;
}