[PATCH 2/4] gas: sframe: i386: have the backend specify the RA too

[Indu Bhagat]

On 5/13/25 7:54 AM, Jens Remus wrote:
> Hello Jan,
> 
> I agree with your feedback.  This is just to add some context.
> 
> On 13.05.2025 14:57, Jan Beulich wrote:
>> On 11.05.2025 09:35, Indu Bhagat wrote:
>>> To process some CFI directives like .cfi_undefined and .cfi_same_value,
>>> it is necessary for correctness to detect all cases when the register
>>> used is one of SP, FP or RA.
>>>
>>> Currently, the backends needed to specify the RA only in the case of
>>> those ABIs where RA tracking was necessary, e.g. AArch64.  For AMD64,
>>> since the return address is always at a fixed offset from the CFA, RA
>>> tracking was disabled.  That now needs to change as we should detect the
>>> cases when RA is used like so:
>>>     .cfi_undefined <RA>
>>> or,
>>>     .cfi_same_value <RA>
>>
>> I can't really make sense of this: As you say, the return address on x86
>> is on the stack, not in a register. Yet both directives want a register
>> specified. See also below. (In fact it's unclear to me what
>>
>> 	.cfi_same_value rip
>>
>> would actually mean: %rip is neither preserved nor can it remain
>> unmodified.)
>>
>>> @@ -3656,6 +3657,7 @@ md_begin (void)
>>>   #ifdef OBJ_ELF
>>>         x86_sframe_cfa_sp_reg = REG_SP;
>>>         x86_sframe_cfa_fp_reg = REG_FP;
>>> +      x86_sframe_cfa_ra_reg = 16;
>>
>> Along the lines of the earlier comment asking to have a #define for this
>> number - what would the identifier be? Not REG_RA, I would say, as that's
>> not appropriate on x86, where there's conceptually no such register. Yet
>> REG_IP also doesn't look right, as you're after the return address here.
> 
> The background is to represent .cfi_undefined <RA> (for x86-64, due to
> the lack of a RA register, potentially .cfi_undefined <IP> instead) in
> SFrame, which is used in e.g. Glibc to identify the outermost frame,
> which cannot be further unwound.  See instances of .cfi_undefined in
> Glibc start.S:
> 
> sysdeps/aarch64/start.S: cfi_undefined (x30): LR (=RA) is undefined
> sysdeps/s390/s390-64/start.S: cfi_undefined (r14): RA is undefined
> sysdeps/x86_64/start.S: cfi_undefined (rip): IP is undefined
> 

(Adding more context to what Jens mentions)

Its a provision in the DWARF standard:
"If a Return Address register is defined in the virtual unwind table, 
and its rule is
undefined (for example, by DW_CFA_undefined), then there is no return 
address
and no call address, and the virtual unwind of stack activations is 
complete."

The SysV ABI AMD64 doc does mention 16 to be the return address register 
in the "DWARF Register Number Mapping" table:

Return Address RA 16

So, REG_RA can be mentioned as a pseudo register in the comments. 
Something like:

/* DWARF register number of the (pseudo) return-address register in 64-bit
    mode.  */
#define REG_RA 16


>>
>>> --- a/gas/config/tc-i386.h
>>> +++ b/gas/config/tc-i386.h
>>> @@ -462,6 +462,12 @@ extern unsigned int x86_sframe_cfa_sp_reg;
>>>   extern unsigned int x86_sframe_cfa_fp_reg;
>>>   #define SFRAME_CFA_FP_REG x86_sframe_cfa_fp_reg
>>>   
>>> +/* The return address DWARF register number for SFrame purposes.  Although for
>>> +   AMD64, RA tracking is disabled, specific constructs, like for indicating
>>> +   the _start function, may use it.  */
>>> +extern unsigned int x86_sframe_cfa_ra_reg;
>>> +#define SFRAME_CFA_RA_REG x86_sframe_cfa_ra_reg
>>
>> Why is it, btw, that this needs a variable which never changes? IOW why
>> not simply
>>
>> #define SFRAME_CFA_RA_REG REG_<whatever>
>>
>> ? Same actually goes for x86_sframe_cfa_{fp,sp}_reg as well.
> 
> I do have an experimental patch on top of my S390 series that
> temporarily changes sframe_cfa_ra_reg (defined to s390_sframe_cfa_ra_reg
> on S390) to represent .cfi_return_column in SFrame on S390.
> 
> Either all targets need to define sframe_cfa_ra_reg so that it can serve
> as a lvalue or I need to guard this S390-specific code with TC_S390.  So
> far I had been doing the latter, as x86-64 did not define
> sframe_cfa_ra_reg at all, which this series would change.
> 
> Regards,
> Jens