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

[Jens Remus]

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.

> 
>> +   Return SFRAME_XLATE_OK if success.  */
>> +
>> +static int
>> +sframe_xlate_do_same_value (const struct sframe_xlate_ctx *xlate_ctx ATTRIBUTE_UNUSED,
>> +			    const struct cfi_insn_data *cfi_insn)
>> +{
>> +  if (cfi_insn->u.ri.reg == SFRAME_CFA_FP_REG
>> +      || (sframe_ra_tracking_p () && cfi_insn->u.ri.reg == SFRAME_CFA_RA_REG)
> 
> Patch 2 eliminated RA tracking, I thought? Or else why do you need to
> use the predicate here, but not in patch 3?

Patch 2 does not eliminate SFrame RA tracking.  It eliminates the
internal macro SFRAME_FRE_RA_TRACKING, which was used to test whether a
target uses SFrame RA tracking at compile-time.  The predicate
sframe_ra_tracking_p remains to test at run-time.

Patch 3 handles .cfi_undefined.  x86-64 does not use a RA register (and
therefore also not SFRame RA tracking) and therefore code such as Glibc
uses .cfi_undefined <IP> on x86-64 to indicate unwinding should stop.
Patch 3 therefore relies on the hack from patch 2 to define
SFRAME_CFA_RA_REG to REG_IP (16) and ignores whether a target actually
uses SFrame RA tracking to cover x86-64.

This patch contains logic that is applicable only to targets that use
RA tracking.  It therefore needs to test sframe_ra_tracking_p.

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des
Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der
Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/