[RFC,V2,2/5] gas: aarch64: suppport CFI directive .cfi_mte_tagged_frame

Message ID 20250411041635.1984719-3-indu.bhagat@oracle.com
State New
Headers
Series Add support for MTE stack tagging |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm warning Skipped upon request
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Skipped upon request

Commit Message

Indu Bhagat April 11, 2025, 4:16 a.m. UTC
  Process a new aarch64-specific CFI directive: .cfi_mte_tagged_frame
(LLVM uses this CFI directive already).  The CFI directive, when present
for a function, indicates that the stack frame for the function may
modify the MTE tags of the stack space it uses.  The assembler emits
char 'G' in the CIE augmentation string to indicate the same.

TBD:
  - Whats that comment style in c-aarch64.texi ?

ChangeLog:

        * gas/config/tc-aarch64.c (s_aarch64_mte_tagged_frame): New
	definition.
        * gas/config/tc-aarch64.h (tc_fde_entry_extras): Add
	memtag_frame_p.
        (tc_cie_entry_extras): Likewise.
        (tc_fde_entry_init_extra): Likewise.
        (tc_cie_fde_equivalent_extra): Likewise.
        (tc_cie_entry_init_extra): Likewise.
        * gas/doc/c-aarch64.texi: Add documentation for
	.cfi_mte_tagged_frame directive.
        * gas/testsuite/gas/aarch64/mte_tagged_stack.d: New test.
        * gas/testsuite/gas/aarch64/mte_tagged_stack.s: New test.
---
 gas/config/tc-aarch64.c                      |  9 ++++
 gas/config/tc-aarch64.h                      | 17 ++++---
 gas/doc/c-aarch64.texi                       | 10 +++++
 gas/testsuite/gas/aarch64/mte_tagged_stack.d | 47 ++++++++++++++++++++
 gas/testsuite/gas/aarch64/mte_tagged_stack.s | 24 ++++++++++
 5 files changed, 102 insertions(+), 5 deletions(-)
 create mode 100644 gas/testsuite/gas/aarch64/mte_tagged_stack.d
 create mode 100644 gas/testsuite/gas/aarch64/mte_tagged_stack.s
  

Comments

Richard Sandiford April 11, 2025, 9:49 a.m. UTC | #1
Indu Bhagat <indu.bhagat@oracle.com> writes:
> Process a new aarch64-specific CFI directive: .cfi_mte_tagged_frame
> (LLVM uses this CFI directive already).  The CFI directive, when present
> for a function, indicates that the stack frame for the function may
> modify the MTE tags of the stack space it uses.  The assembler emits
> char 'G' in the CIE augmentation string to indicate the same.
>
> TBD:
>   - Whats that comment style in c-aarch64.texi ?

I think it's just alphabetical.  The new entry would then go under:

@c CCCCCCCCCCCCCCCCCCCCCCCCCC

and before:

@cindex @code{.cpu} directive, AArch64


>  /* Extra equivalence checks required by AArch64 when selecting the correct cie
>     for some fde.  Currently only used to check for quivalence between keys used
>     to sign ther return address.  */
> -#define tc_cie_fde_equivalent_extra(cie, fde) (cie->pauth_key == fde->pauth_key)
> +#define tc_cie_fde_equivalent_extra(cie, fde) ((cie->pauth_key == fde->pauth_key) \
> +					      && (cie->memtag_frame_p = fde->memtag_frame_p))

Should be == rather than =.

It would be good to extend the testcase so that it would have caught this.

Otherwise it looks good to me.

Thanks,
Richard
  
Indu Bhagat April 11, 2025, 11:59 p.m. UTC | #2
On 4/11/25 2:49 AM, Richard Sandiford wrote:
> Indu Bhagat <indu.bhagat@oracle.com> writes:
>> Process a new aarch64-specific CFI directive: .cfi_mte_tagged_frame
>> (LLVM uses this CFI directive already).  The CFI directive, when present
>> for a function, indicates that the stack frame for the function may
>> modify the MTE tags of the stack space it uses.  The assembler emits
>> char 'G' in the CIE augmentation string to indicate the same.
>>
>> TBD:
>>    - Whats that comment style in c-aarch64.texi ?
> 
> I think it's just alphabetical.  The new entry would then go under:
> 
> @c CCCCCCCCCCCCCCCCCCCCCCCCCC
> 
> and before:
> 
> @cindex @code{.cpu} directive, AArch64
> 
> 

OK.

>>   /* Extra equivalence checks required by AArch64 when selecting the correct cie
>>      for some fde.  Currently only used to check for quivalence between keys used
>>      to sign ther return address.  */
>> -#define tc_cie_fde_equivalent_extra(cie, fde) (cie->pauth_key == fde->pauth_key)
>> +#define tc_cie_fde_equivalent_extra(cie, fde) ((cie->pauth_key == fde->pauth_key) \
>> +					      && (cie->memtag_frame_p = fde->memtag_frame_p))
> 
> Should be == rather than =.
> 
> It would be good to extend the testcase so that it would have caught this.
> 

Swapping the order of the two functions in the testcase (with the first 
without .cfi_mte_tagged_frame and the second with .cfi_mte_tagged_frame) 
was enough to help catch this.

I have amended the current testcase now.

Thanks!

> Otherwise it looks good to me.
> 
> Thanks,
> Richard
  

Patch

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index e071ad11f46..0f21c635a68 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -2351,6 +2351,14 @@  s_aarch64_cfi_b_key_frame (int ignored ATTRIBUTE_UNUSED)
   fde->pauth_key = AARCH64_PAUTH_KEY_B;
 }
 
+static void
+s_aarch64_mte_tagged_frame (int ignored ATTRIBUTE_UNUSED)
+{
+  demand_empty_rest_of_line ();
+  struct fde_entry *fde = frchain_now->frch_cfi_data->cur_fde_data;
+  fde->memtag_frame_p = true;
+}
+
 #ifdef OBJ_ELF
 /* Emit BFD_RELOC_AARCH64_TLSDESC_ADD on the next ADD instruction.  */
 
@@ -2476,6 +2484,7 @@  const pseudo_typeS md_pseudo_table[] = {
   {"arch_extension", s_aarch64_arch_extension, 0},
   {"inst", s_aarch64_inst, 0},
   {"cfi_b_key_frame", s_aarch64_cfi_b_key_frame, 0},
+  {"cfi_mte_tagged_frame", s_aarch64_mte_tagged_frame, 0},
 #ifdef OBJ_ELF
   {"tlsdescadd", s_tlsdescadd, 0},
   {"tlsdesccall", s_tlsdesccall, 0},
diff --git a/gas/config/tc-aarch64.h b/gas/config/tc-aarch64.h
index acf1ce4aa03..849ef3825fd 100644
--- a/gas/config/tc-aarch64.h
+++ b/gas/config/tc-aarch64.h
@@ -90,13 +90,16 @@  enum pointer_auth_key {
 
 /* The extra fields required by AArch64 in fde_entry and cie_entry.  Currently
    only used to store the key used to sign the frame's return address.  */
-#define tc_fde_entry_extras enum pointer_auth_key pauth_key;
-#define tc_cie_entry_extras enum pointer_auth_key pauth_key;
+#define tc_fde_entry_extras enum pointer_auth_key pauth_key; \
+                            bool memtag_frame_p;
+#define tc_cie_entry_extras enum pointer_auth_key pauth_key; \
+                            bool memtag_frame_p;
 
 /* The extra initialisation steps needed by AArch64 in alloc_fde_entry.
    Currently only used to initialise the key used to sign the return
    address.  */
-#define tc_fde_entry_init_extra(fde) fde->pauth_key = AARCH64_PAUTH_KEY_A;
+#define tc_fde_entry_init_extra(fde) fde->pauth_key = AARCH64_PAUTH_KEY_A; \
+				     fde->memtag_frame_p = false;
 
 /* Extra checks required by AArch64 when outputting the current cie_entry.
    Currently only used to output a 'B' if the return address is signed with the
@@ -106,18 +109,22 @@  enum pointer_auth_key {
       { \
 	if (cie->pauth_key == AARCH64_PAUTH_KEY_B) \
 	  out_one ('B'); \
+	if (cie->memtag_frame_p) \
+	  out_one ('G'); \
       } \
     while (0)
 
 /* Extra equivalence checks required by AArch64 when selecting the correct cie
    for some fde.  Currently only used to check for quivalence between keys used
    to sign ther return address.  */
-#define tc_cie_fde_equivalent_extra(cie, fde) (cie->pauth_key == fde->pauth_key)
+#define tc_cie_fde_equivalent_extra(cie, fde) ((cie->pauth_key == fde->pauth_key) \
+					      && (cie->memtag_frame_p = fde->memtag_frame_p))
 
 /* The extra initialisation steps needed by AArch64 in select_cie_for_fde.
    Currently only used to initialise the key used to sign the return
    address.  */
-#define tc_cie_entry_init_extra(cie, fde) cie->pauth_key = fde->pauth_key;
+#define tc_cie_entry_init_extra(cie, fde) cie->pauth_key = fde->pauth_key; \
+					  cie->memtag_frame_p = fde->memtag_frame_p;
 
 #define TC_FIX_TYPE struct aarch64_fix
 #define TC_INIT_FIX_DATA(FIX) { (FIX)->tc_fix_data.inst = NULL;	\
diff --git a/gas/doc/c-aarch64.texi b/gas/doc/c-aarch64.texi
index 10888d1e78f..99cdf551b8d 100644
--- a/gas/doc/c-aarch64.texi
+++ b/gas/doc/c-aarch64.texi
@@ -601,6 +601,16 @@  been signed with the B-key.  If two frames are signed with differing keys then
 they will not share the same CIE.  This information is intended to be used by
 the stack unwinder in order to properly authenticate return addresses.
 
+@c ZZZZZZZZZZZZZZZZZZZZZZZZZ1
+@c ZZZZZZZZZZZZZZZZZZZZZZZZZ2
+
+@cindex @code{.cfi_mte_tagged_frame} directive, AArch64
+@item	@code{.cfi_mte_tagged_frame}
+The @code{.cfi_mte_tagged_frame} directive inserts a 'G' character into the CIE
+corresponding to the current frame's FDE, meaning that the associated frames
+may modify MTE tags on the stack space they use.  This information is intended
+to be used by the stack unwinder in order to properly untag stack frames.
+
 @end table
 
 @node AArch64 Opcodes
diff --git a/gas/testsuite/gas/aarch64/mte_tagged_stack.d b/gas/testsuite/gas/aarch64/mte_tagged_stack.d
new file mode 100644
index 00000000000..5e8afb818e7
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mte_tagged_stack.d
@@ -0,0 +1,47 @@ 
+#objdump: --dwarf=frames
+# This test is only valid on ELF based ports.
+#notarget: *-*-*coff *-*-pe *-*-wince *-*-*aout* *-*-netbsd
+# Test assembling a file with functions using MTE tagged stack or not
+# It must interpret .cfi_mte_tagged_frame properly and emit a
+# 'G' character into the correct CIE's augmentation string.
+
+.+:     file .+
+
+Contents of the .eh_frame section:
+
+0+ 0+14 0+ CIE
+  Version:               1
+  Augmentation:          "zRG"
+  Code alignment factor: 4
+  Data alignment factor: -8
+  Return address column: 30
+  Augmentation data:     1b
+  DW_CFA_def_cfa: r31 \(sp\) ofs 0
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+0+18 0+14 0+1c FDE cie=0+ pc=0+\.\.0+4
+  DW_CFA_advance_loc: 4 to 0+4
+  DW_CFA_def_cfa_offset: 16
+  DW_CFA_offset: r29 \(x29\) at cfa-16
+  DW_CFA_offset: r30 \(x30\) at cfa-8
+
+0+30 0+10 0+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+44 0+1(4|8) 0+18 FDE cie=0+30 pc=0+4\.\.0+8
+  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
+#?  DW_CFA_nop
+#?  DW_CFA_nop
diff --git a/gas/testsuite/gas/aarch64/mte_tagged_stack.s b/gas/testsuite/gas/aarch64/mte_tagged_stack.s
new file mode 100644
index 00000000000..64a92b488ee
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/mte_tagged_stack.s
@@ -0,0 +1,24 @@ 
+	.arch armv8-a+memtag
+	.text
+	.align	2
+	.global	foo
+	.type	foo, %function
+foo:
+	.cfi_startproc
+	.cfi_mte_tagged_frame
+	stp	x29, x30, [sp, -16]!
+	.cfi_def_cfa_offset 16
+	.cfi_offset 29, -16
+	.cfi_offset 30, -8
+	.cfi_endproc
+	.size	foo, .-foo
+	.align	2
+	.global	bar
+	.type	bar, %function
+bar:
+	.cfi_startproc
+	stp	x29, x30, [sp, -16]!
+	.cfi_def_cfa_offset 16
+	.cfi_offset 29, -16
+	.cfi_offset 30, -8
+	.cfi_endproc