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

Message ID 20241125162846.94691-3-matthieu.longo@arm.com
State New
Headers
Series aarch64: add DWARF and SFrame support for new CFI directive used for PAuth_LR |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Matthieu Longo Nov. 25, 2024, 4:28 p.m. UTC
  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
  

Comments

Indu Bhagat Dec. 1, 2024, 3:23 a.m. UTC | #1
On 11/25/24 8:28 AM, 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.

Hi Matthieu,

See one comment inlined below.

Thanks
Indu

> ---
>   .../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
> +## 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.
> +##
> +## 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
> +	.long 0
> +	.cfi_endproc

Since the purpose is to ensure that the old behavior is maintained, I am 
wondering if it is not better to keep the old test as is (which also 
tests the .cfi_window_save *together with* the .cfi_b_key_frame 
directives). WDYT ?

> 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"
>   }
  

Patch

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
+## 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.
+##
+## 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
+	.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"
 }