[PATCH v1 2/4] aarch64 SFrame: use preferred CFI directive for AArch64 PAC

[Richard Earnshaw (lists)]

On 25/11/2024 16:28, Matthieu Longo wrote:
> ARMv8.3 addded support for a new security feature named Pointer
> Authentication. Support for this feature in SFrame already exists,
> but is relying on the deprecated "AArch64" (actually Sparc) CFI
> directive .cfi_gnu_window_save. .cfi_negate_ra_state CFI directive
> should be used instead to convey this information.
> 
> In GCC 14 and older, the Sparc DWARF extension .cfi_gnu_window_save
> is emitted instead of .cfi_negate_ra_state.
> GCC 15 fixed this issue, but this behavior is preserved for backward
> compatibility.
> 
> The existing sframe test for AArch64 PAC was using .cfi_gnu_window_save.
> This patch replaces this CFI in the existing test by the preferred one,
> and adds a new test to check for backward compatibility when using
> .cfi_gnu_window_save.
> ---
>  .../gas/cfi-sframe/cfi-sframe-aarch64-3.d     | 20 ++++++++++++++++++
>  .../gas/cfi-sframe/cfi-sframe-aarch64-3.s     | 21 +++++++++++++++++++
>  .../cfi-sframe-aarch64-pac-ab-key-1.s         |  8 +++----
>  gas/testsuite/gas/cfi-sframe/cfi-sframe.exp   |  1 +
>  4 files changed, 46 insertions(+), 4 deletions(-)
>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-3.d
>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-3.s
> 
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-3.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-3.d
> new file mode 100644
> index 00000000000..f72b70a970a
> --- /dev/null
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-3.d
> @@ -0,0 +1,20 @@
> +#as: --gsframe
> +#objdump: --sframe=.sframe
> +#name: SFrame cfi_negate_ra_state test (using cfi_window_save)
> +#...
> +Contents of the SFrame section .sframe:
> +
> +  Header :
> +
> +    Version: SFRAME_VERSION_2
> +    Flags: NONE
> +    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 +u\[s\] +
> +
> +#pass
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-3.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-3.s
> new file mode 100644
> index 00000000000..df6ff6e8c8f
> --- /dev/null
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-3.s
> @@ -0,0 +1,21 @@
> +## ARMv8.3 addded support a new security feature named Pointer Authentication. The

You refer in the rest of the sentence to the feature in the present tense, so I think it's more natural to use the present tense at the start as well, so...

Armv8.3 adds support for a new ...

> +## main idea behind this is to use the unused bits in the pointer values.
> +## Each pointer is patched with a PAC before writing to memory, and is verified
> +## before using it.
> +## When the pointers are mangled, the stack trace generator needs to know so it
> +## can mask off the PAC from the pointer value to recover the return address,
> +## and conversely, skip doing so if the pointers are not mangled.
> +##
> +## .cfi_negate_ra_state CFI directive is usually used to convey this information.
> +## In GCC 14 and older, the Sparc DWARF extension .cfi_window_save is emitted
> +## instead of .cfi_negate_ra_state.  GCC 15 fixed this issue, but this behavior
> +## is preserved in binutils for backward compatibility.

So the key reason for this confusion is that .cfi_negate_ra_state and .cfi_window_save are in the processor-specific numbering space, but use the same code value in the dwarf tables.  It was because of this that older versions of GCC were mapping this to a single name rather than emitting the correct name on aarch64.  The consequence of this is that there is no change to the object file created when the source is assembled, but it is a portability improvement as we cannot require other assemblers to accept the SPARC directive when assembling for AArch64.  Nevertheless, in order to ensure that gas remains compatible with existing GCC releases we need to handle the SPARC name as well in gas, hence this test.


> +##
> +## SFrame has support for this. This testcase ensures that the directive
> +## is interpreted successfully.
> +	.cfi_startproc
> +	.long 0
> +	.cfi_def_cfa_offset 16
> +	.cfi_window_save

Add a comment on this line for clarity?

+	.cfi_window_save // really .cfi_negate_ra_state

> +	.long 0
> +	.cfi_endproc
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-pac-ab-key-1.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-pac-ab-key-1.s
> index d9a408c668c..84230a99e3c 100644
> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-pac-ab-key-1.s
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-aarch64-pac-ab-key-1.s
> @@ -8,12 +8,12 @@ _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
>  	.cfi_offset 30, -8
> -        ret
> +	ret
>  	.cfi_endproc
>  .LFE0:
>  	.size	_Z5foo_av, .-_Z5foo_av
> @@ -25,12 +25,12 @@ _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
>  	.cfi_offset 30, -8
>  	nop
>  	nop
> -        ret
> +	ret
>  	.cfi_endproc
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
> index cfae28d39aa..c646b109895 100644
> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
> @@ -97,5 +97,6 @@ if { [istarget "x86_64-*-*"] && [gas_sframe_check] } then {
>  if { [istarget "aarch64*-*-*"] && [gas_sframe_check] } then {
>      run_dump_test "cfi-sframe-aarch64-1"
>      run_dump_test "cfi-sframe-aarch64-2"
> +    run_dump_test "cfi-sframe-aarch64-3"
>      run_dump_test "cfi-sframe-aarch64-pac-ab-key-1"
>  }