[PATCH v1 1/4] aarch64: make explicit that CFI gnu_window_save is for Sparc, not AArch64

[Matthieu Longo]

On 2024-12-01 03:23, Indu Bhagat wrote:
> On 11/25/24 8:28 AM, Matthieu Longo wrote:
>> - add a detailed comment when parsing DW_CFA_GNU_window_save in SFrame to
>>    explain why we are checking whether the targeted architecture is 
>> AArch64,
>>    whereas this CFI is a Sparc extension.
>> - replace .cfi_gnu_window_save by .cfi_negate_ra_state in existing 
>> AArch64
>>    DWARF tests as this is the preferred directive since GCC 15.
>> - add a new AARch64 test to check backward compatibility with old GCC
>>    versions that emits .cfi_gnu_window_save.
>> ---
>>   gas/gen-sframe.c                              |  8 +++++-
>>   gas/testsuite/gas/aarch64/pac_ab_key.s        |  4 +--
>>   .../gas/aarch64/pac_compat_cfi_window_save.d  | 26 +++++++++++++++++++
>>   .../gas/aarch64/pac_compat_cfi_window_save.s  | 20 ++++++++++++++
>>   4 files changed, 55 insertions(+), 3 deletions(-)
>>   create mode 100644 gas/testsuite/gas/aarch64/ 
>> pac_compat_cfi_window_save.d
>>   create mode 100644 gas/testsuite/gas/aarch64/ 
>> pac_compat_cfi_window_save.s
>>
>> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
>> index 626dc33b71d..be48b339609 100644
>> --- a/gas/gen-sframe.c
>> +++ b/gas/gen-sframe.c
>> @@ -1283,7 +1283,13 @@ sframe_xlate_do_aarch64_negate_ra_state (struct 
>> sframe_xlate_ctx *xlate_ctx,
>>   }
>>   /* Translate DW_CFA_GNU_window_save into SFrame context.
>> -   DW_CFA_AARCH64_negate_ra_state is multiplexed with 
>> DW_CFA_GNU_window_save.
>> +   DW_CFA_GNU_window_save is a DWARF Sparc extension, but is 
>> multiplexed with a
>> +   directive of DWARF AArch64 extension: DW_CFA_AARCH64_negate_ra_state.
>> +   The AArch64 backend of GCC 14 and older versions was emitting 
>> mistakenly the
>> +   Sparc CFI directive (.cfi_window_save). From GCC 15, the AArch64 
>> backend only
>> +   emits .cfi_negate_ra_state. For backward compatibility, the 
>> handler for
>> +   .cfi_window_save needs to check whether the directive was used in 
>> a AArch ABI
>> +   context or not.
> 
> Nit: There are a few "dot, space, space, new sentence" issues in the 
> comment block above.
> 
> The patch looks OK to me otherwise,
> 
> Thanks
> Indu
> 

Fixed in the next revision.

Thanks,
Matthieu

>>      Return SFRAME_XLATE_OK if success.  */
>>   static int
>> diff --git a/gas/testsuite/gas/aarch64/pac_ab_key.s b/gas/testsuite/ 
>> gas/aarch64/pac_ab_key.s
>> index 4b328e72ae4..3b81919409d 100644
>> --- a/gas/testsuite/gas/aarch64/pac_ab_key.s
>> +++ b/gas/testsuite/gas/aarch64/pac_ab_key.s
>> @@ -7,7 +7,7 @@ _Z5foo_av:
>>   .LFB0:
>>       .cfi_startproc
>>       hint    25 // paciasp
>> -    .cfi_window_save
>> +    .cfi_negate_ra_state
>>       stp    x29, x30, [sp, -16]!
>>       .cfi_def_cfa_offset 16
>>       .cfi_offset 29, -16
>> @@ -23,7 +23,7 @@ _Z5foo_bv:
>>       .cfi_startproc
>>       .cfi_b_key_frame
>>       hint    27 // pacibsp
>> -    .cfi_window_save
>> +    .cfi_negate_ra_state
>>       stp    x29, x30, [sp, -16]!
>>       .cfi_def_cfa_offset 16
>>       .cfi_offset 29, -16
>> diff --git a/gas/testsuite/gas/aarch64/pac_compat_cfi_window_save.d b/ 
>> gas/testsuite/gas/aarch64/pac_compat_cfi_window_save.d
>> new file mode 100644
>> index 00000000000..f49cebcbfde
>> --- /dev/null
>> +++ b/gas/testsuite/gas/aarch64/pac_compat_cfi_window_save.d
>> @@ -0,0 +1,26 @@
>> +#objdump: --dwarf=frames
>> +# This test is only valid on ELF based ports.
>> +#notarget: *-*-*coff *-*-pe *-*-wince *-*-*aout* *-*-netbsd
>> +
>> +.+:     file .+
>> +
>> +Contents of the .eh_frame section:
>> +
>> +0+ 0+10 0+ CIE
>> +  Version:               1
>> +  Augmentation:          "zR"
>> +  Code alignment factor: 4
>> +  Data alignment factor: -8
>> +  Return address column: 30
>> +  Augmentation data:     1b
>> +  DW_CFA_def_cfa: r31 \(sp\) ofs 0
>> +
>> +0+14 0+18 0+18 FDE cie=0+ pc=0+\.\.0+8
>> +  DW_CFA_advance_loc: 4 to 0+4
>> +  DW_CFA_AARCH64_negate_ra_state
>> +  DW_CFA_advance_loc: 4 to 0+8
>> +  DW_CFA_def_cfa_offset: 16
>> +  DW_CFA_offset: r29 \(x29\) at cfa-16
>> +  DW_CFA_offset: r30 \(x30\) at cfa-8
>> +  DW_CFA_nop
>> +  DW_CFA_nop
>> diff --git a/gas/testsuite/gas/aarch64/pac_compat_cfi_window_save.s b/ 
>> gas/testsuite/gas/aarch64/pac_compat_cfi_window_save.s
>> new file mode 100644
>> index 00000000000..1bfdef71f22
>> --- /dev/null
>> +++ b/gas/testsuite/gas/aarch64/pac_compat_cfi_window_save.s
>> @@ -0,0 +1,20 @@
>> +    .arch armv8-a
>> +    .text
>> +    .align    2
>> +    .global    _Z5foo_av
>> +    .type    _Z5foo_av, %function
>> +_Z5foo_av:
>> +.LFB0:
>> +    .cfi_startproc
>> +    hint    25 // paciasp
>> +    .cfi_window_save
>> +    stp    x29, x30, [sp, -16]!
>> +    .cfi_def_cfa_offset 16
>> +    .cfi_offset 29, -16
>> +    .cfi_offset 30, -8
>> +    .cfi_endproc
>> +.LFE0:
>> +    .size    _Z5foo_av, .-_Z5foo_av
>> +    .align    2
>> +    .global    _Z5foo_bv
>> +    .type    _Z5foo_bv, %function
>