Home Original page

[PATCH 3/4] gas: sframe: handle .cfi_undefined

Indu Bhagat indu.bhagat@oracle.com
Mon May 12 18:55:37 GMT 2025 
On 5/12/25 1:47 AM, Jens Remus wrote:
> Hello Indu!
> 
> On 11.05.2025 09:35, Indu Bhagat wrote:
>> Fix PR gas/32952 - sframe: incorrect handling of .cfi_undefined in gas
>>
>> In context of SFrame generation, it is incorrect to simply ignore all
>> .cfi_undefined.  We may ignore only those .cfi_undefined which are for
>> registers of no interest (similar to whats done for other CFI
>> directives).
> 
> Is this a first step towards representing .cfi_undefined <RA>
> in SFrame as either (potentially starting with SFrame V3):
> 1. empty FDE (i.e. FDE without any FREs) or
> 2. FRE without offsets?
> 

Yes.

I am still unsure about whether it should be #1 or #2 above.  I have a 
patch for #1, and so far I think we do not need the flexibility provided 
by #2: What does it mean to have an FDE with one FRE designating 
"outermost frame" and other FREs with valid stack trace information; 
there cannot be such a case IIUC..

>>
>> gas/
>>          * gen-sframe.c (sframe_xlate_do_cfi_undefined): New definition.
>>          (sframe_do_cfi_insn): Handle .cfi_undefined.
>> gas/testsuite/
>>          * gas/cfi-sframe/cfi-sframe.exp: Add new tests.
>>          * gas/cfi-sframe/cfi-sframe-common-10.d: New test.
>>          * gas/cfi-sframe/cfi-sframe-common-10.s: New test.
>> 	* gas/cfi-sframe/cfi-sframe-x86_64-empty-4.d: New test.
>> 	* gas/cfi-sframe/cfi-sframe-x86_64-empty-4.s: New test.
>> ---
>>   gas/gen-sframe.c                              | 28 +++++++++++++++++++
>>   .../gas/cfi-sframe/cfi-sframe-common-10.d     | 22 +++++++++++++++
>>   .../gas/cfi-sframe/cfi-sframe-common-10.s     | 12 ++++++++
>>   .../cfi-sframe/cfi-sframe-x86_64-empty-4.d    | 17 +++++++++++
>>   .../cfi-sframe/cfi-sframe-x86_64-empty-4.s    |  6 ++++
>>   gas/testsuite/gas/cfi-sframe/cfi-sframe.exp   |  2 ++
>>   6 files changed, 87 insertions(+)
>>   create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-common-10.d
>>   create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-common-10.s
>>   create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-empty-4.d
>>   create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-empty-4.s
>>
>> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
>> index da714cb1f35..a87d464fd5f 100644
>> --- a/gas/gen-sframe.c
>> +++ b/gas/gen-sframe.c
>> @@ -1507,6 +1507,32 @@ sframe_xlate_do_cfi_escape (const struct sframe_xlate_ctx *xlate_ctx,
>>     return err;
>>   }
>>   
>> +/* Translate DW_CFA_undefined into SFrame context.
>> +
>> +   DW_CFA_undefined op implies that from now on the previous value of register
>> +   can’t be restored anymore.  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_undefined.
>> +
>> +   Return SFRAME_XLATE_OK if success.  */
>> +
>> +static int
>> +sframe_xlate_do_cfi_undefined (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
>> +      || cfi_insn->u.ri.reg == SFRAME_CFA_RA_REG
>> +      || cfi_insn->u.ri.reg == SFRAME_CFA_SP_REG)
> 
> Use cfi_insn->u.r, as .cfi_undefined takes only a register.  See also
> occurrences of DW_CFA_undefined in gas/dw2gencfi.c.
> 

Done.

>> +    {
>> +      as_warn (_("skipping SFrame FDE; %s reg %u in .cfi_undefined"),
>> +	       sframe_register_name (cfi_insn->u.ri.reg), cfi_insn->u.ri.reg);
> 
> Dito.
> 

Done.

>> +      return SFRAME_XLATE_ERR_NOTREPRESENTED; /* Not represented.  */
>> +    }
>> +
>> +  /* Safe to skip.  */
>> +  return SFRAME_XLATE_OK;
>> +}
>> +
>>   /* Returns the DWARF call frame instruction name or fake CFI name for the
>>      specified CFI opcode, or NULL if the value is not recognized.  */
>>   
>> @@ -1610,6 +1636,8 @@ sframe_do_cfi_insn (struct sframe_xlate_ctx *xlate_ctx,
>>          These do not impact the coverage of the basic stack tracing
>>          information as conveyed in the SFrame format.  */
>>       case DW_CFA_undefined:
>> +      err = sframe_xlate_do_cfi_undefined (xlate_ctx, cfi_insn);
>> +      break;
>>       case DW_CFA_same_value:
>>         break;
>>       default:
>> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-10.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-10.d
>> new file mode 100644
>> index 00000000000..106e05d160b
>> --- /dev/null
>> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-10.d
>> @@ -0,0 +1,22 @@
>> +#as: --gsframe
>> +#objdump: --sframe=.sframe
>> +#name: SFrame cfi_undefined test
>> +#...
>> +Contents of the SFrame section .sframe:
>> +
>> +  Header :
>> +
>> +    Version: SFRAME_VERSION_2
>> +    Flags: NONE
>> +#?    CFA fixed FP offset: \-?\d+
>> +#?    CFA fixed RA offset: \-?\d+
>> +    Num FDEs: 1
>> +    Num FREs: 2
>> +
>> +  Function Index :
>> +    func idx \[0\]: pc = 0x0, size = 8 bytes
>> +    STARTPC + CFA + FP + RA +
>> +#...
>> +    0+0004 +sp\+16 +u +[uf] +
>> +
>> +#pass
>> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-10.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-10.s
>> new file mode 100644
>> index 00000000000..7761edaf2b7
>> --- /dev/null
>> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-10.s
>> @@ -0,0 +1,12 @@
>> +## cfi_undefined when used with "not interesting" registers (from the
>> +## perspective of SFrame section, non SP/FP/RA registers are not
>> +## interesting) does not affect the asynchronicity of the SFrame
>> +## stack trace information.  Such CFI directives can be skipped for SFrame
>> +## stack trace info generation.
>> +	.cfi_startproc
>> +	.long 0
>> +	.cfi_def_cfa_offset 16
>> +	.cfi_undefined 1
>> +	.cfi_undefined 2
>> +	.long 0
>> +	.cfi_endproc
>> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-empty-4.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-empty-4.d
>> new file mode 100644
>> index 00000000000..c1925bc58bd
>> --- /dev/null
>> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-empty-4.d
>> @@ -0,0 +1,17 @@
>> +#as: --gsframe
>> +#warning: skipping SFrame FDE; SP reg 7 in \.cfi\_undefined
>> +#objdump: --sframe=.sframe
>> +#name: CFI_escape with multiple DWARF expr
> 
> You missed to update the test name. :)
> 

Ah. Done.

>> +#...
>> +Contents of the SFrame section .sframe:
>> +
>> +  Header :
>> +
>> +    Version: SFRAME_VERSION_2
>> +    Flags: NONE
>> +#?    CFA fixed FP offset: \-?\d+
>> +#?    CFA fixed RA offset: \-?\d+
>> +    Num FDEs: 0
>> +    Num FREs: 0
>> +
>> +#pass
>> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-empty-4.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-empty-4.s
>> new file mode 100644
>> index 00000000000..096b147e7a4
>> --- /dev/null
>> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-empty-4.s
>> @@ -0,0 +1,6 @@
>> +	.cfi_startproc
>> +	.long 0
>> +	.cfi_def_cfa_offset 16
>> +	.cfi_undefined 0x7
> 
> Out of curiosity: Why did you specify the register number in
> hexadecimal?  Did you use the SP register number instead of the
> newly introduced RA register number, so that this test does
> not change, if SFrame would start representing .cfi_undefined <RA>?
> 

Switched to decimal as done in the rest of the tests.

Yes, I thought keeping a test which doesnt shift when we add handling of 
.cfi_undefined <RA> is useful.

Thanks for reviewing
More information about the Binutils mailing list