[PATCH][GCC][Arm] Missing optimization pattern for rev16 on architectures with thumb1

[Matthieu Longo]

This patch marks a rev16 test as XFAIL for architectures having only 
Thumb1 support. The generated code is functionally correct, but the 
optimization is disabled when -mthumb is equivalent to Thumb1. Fixing 
the root issue would requires changes that are not suitable for GCC14 
stage 4.


More information at https://linaro.atlassian.net/browse/GNU-1141

gcc/testsuite/ChangeLog:

* gcc.target/arm/rev16_2.c: XFAIL when compiled with Thumb1.From 11be6a0feb1cbc0bca6e23dacf98c5817b71ba3d Mon Sep 17 00:00:00 2001
From: Matthieu Longo 
Date: Thu, 8 Feb 2024 18:13:49 +
Subject: [PATCH] [arm] Missing optimization pattern for rev16 on architectures
 with thumb1.

This patch marks a rev16 test as XFAIL for architectures having only Thumb1
support. The generated code is functionally correct, but the optimization is
disabled when -mthumb is equivalent to Thumb1. Fixing the root issue would
requires changes that are not suitable for GCC14 stage 4.
More information at https://linaro.atlassian.net/browse/GNU-1141

gcc/testsuite/ChangeLog:

* gcc.target/arm/rev16_2.c: XFAIL when compiled with Thumb1.
---
 gcc/testsuite/gcc.target/arm/rev16_2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/arm/rev16_2.c 
b/gcc/testsuite/gcc.target/arm/rev16_2.c
index c6553b38a00..dff66e10bb9 100644
--- a/gcc/testsuite/gcc.target/arm/rev16_2.c
+++ b/gcc/testsuite/gcc.target/arm/rev16_2.c
@@ -17,4 +17,4 @@ __rev16_32 (__u32 x)
  | (((__u32)(x) & (__u32)0xff00ff00UL) >> 8);
 }
 
-/* { dg-final { scan-assembler-times {rev16\tr[0-9]+, r[0-9]+} 2 } } */
+/* { dg-final { scan-assembler-times {rev16\tr[0-9]+, r[0-9]+} 2 { xfail 
arm_thumb1_ok } } } */
\ No newline at end of file
-- 
2.34.1

RE: [PATCH][GCC][Arm] Define __ARM_FEATURE_BF16 when +bf16 feature is enabled

[Matthieu Longo]

Hi Richard,

The conditions for __ARM_BF16_FORMAT_ALTERNATIVE are redundant, so I wanted to 
simplify it.
TARGET_BF16_SIMD implies TARGET_BF16_FP to be true as well.

#define TARGET_BF16_FP (TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_VFP5 \
&& arm_arch8_2 && arm_arch_bf16)
#define TARGET_BF16_SIMD (TARGET_NEON && TARGET_VFP5 \
  && arm_arch8_2 && arm_arch_bf16)
#define TARGET_NEON \
  (TARGET_32BIT && TARGET_HARD_FLOAT\
   && bitmap_bit_p (arm_active_target.isa, isa_bit_neon))

Please let me know if you agree on the simplification.

Regards,
Matthieu

-Original Message-
From: Richard Earnshaw  
Sent: Wednesday, January 10, 2024 3:26 PM
To: Matthieu Longo ; gcc-patches@gcc.gnu.org
Cc: Richard Earnshaw ; Kyrylo Tkachov 

Subject: Re: [PATCH][GCC][Arm] Define __ARM_FEATURE_BF16 when +bf16 feature is 
enabled



On 08/01/2024 17:21, Matthieu Longo wrote:
> Hi,
> 
> Arm GCC backend does not define __ARM_FEATURE_BF16 when +bf16 is 
> specified (via -march option, or target pragma) whereas it is supposed 
> to be tested before including arm_bf16.h (as specified in ACLE document:
> https://arm-software.github.io/acle/main/acle.html#arm_bf16h).
> 
> gcc/ChangeLog:
> 
>      * config/arm/arm-c.cc (arm_cpu_builtins): define
> __ARM_FEATURE_BF16
>      * config/arm/arm.h: define TARGET_BF16
> 
> Ok for master ?
> 
> Matthieu
index
2e181bf7f36bab1209d5358e65d9513541683632..21ca22ac71119eda4ff01709aa95002ca13b1813
100644
--- a/gcc/config/arm/arm-c.cc
+++ b/gcc/config/arm/arm-c.cc
@@ -425,12 +425,14 @@ arm_cpu_builtins (struct cpp_reader* pfile)
   arm_arch_cde_coproc);

def_or_undef_macro (pfile, "__ARM_FEATURE_MATMUL_INT8", TARGET_I8MM);
+
+  def_or_undef_macro (pfile, "__ARM_FEATURE_BF16", TARGET_BF16);
+  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
+ TARGET_BF16_FP);
def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_SCALAR_ARITHMETIC",
  TARGET_BF16_FP);
def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_VECTOR_ARITHMETIC",
  TARGET_BF16_SIMD);
-  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
- TARGET_BF16_FP || TARGET_BF16_SIMD);

Why is the definition of __ARM_BF16_FORMAT_ALTERNATIVE changed?  And why is 
there explanation of that change?  It doesn't seem directly related to $subject.

R.

  }

  void

[PATCH][GCC][Arm] Add pattern for bswap + rotate -> rev16 [Bug 108933]

[Matthieu Longo]

rev16 pattern was not recognised anymore as a change in the bswap tree
pass was introducing a new GIMPLE form, not recognized by the assembly
final transformation pass.

More details in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108933

gcc/ChangeLog:

PR target/108933
* config/arm/arm.md (*arm_rev16si2_alt3): new pattern to convert
  a bswap + rotate by 16 bits into rev16

gcc/testsuite/ChangeLog:

PR target/108933
* gcc.target/arm/rev16.c: Moved to...
* gcc.target/arm/rev16_1.c: ...here.
* gcc.target/arm/rev16_2.c: New test to check that rev16 is
  emitted.diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 
4a98f2d7b6251da940806b26d4c310a7f7af927b..330c0a5ce2926439760466746a68fd5f6c5b1b09
 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -12601,6 +12601,18 @@
(set_attr "type" "rev")]
 )
 
+;; Similar pattern to match (rotate (bswap) 16)
+(define_insn "*arm_rev16si2_alt3"
+  [(set (match_operand:SI 0 "register_operand" "=l,r")
+(rotate:SI (bswap:SI (match_operand:SI 1 "register_operand" "l,r"))
+ (const_int 16)))]
+  "arm_arch6"
+  "rev16\\t%0, %1"
+  [(set_attr "arch" "t,32")
+   (set_attr "length" "2,4")
+   (set_attr "type" "rev")]
+)
+
 (define_expand "arm_rev16si2"
   [(set (match_operand:SI 0 "s_register_operand")
(bswap:SI (match_operand:SI 1 "s_register_operand")))]
diff --git a/gcc/testsuite/gcc.target/arm/rev16.c 
b/gcc/testsuite/gcc.target/arm/rev16_1.c
similarity index 100%
rename from gcc/testsuite/gcc.target/arm/rev16.c
rename to gcc/testsuite/gcc.target/arm/rev16_1.c
diff --git a/gcc/testsuite/gcc.target/arm/rev16_2.c 
b/gcc/testsuite/gcc.target/arm/rev16_2.c
new file mode 100644
index 
..90213f9b49f45340ced4f29c31446971dbc88ecd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/rev16_2.c
@@ -0,0 +1,20 @@
+/* { dg-options "-O2" } */
+/* { dg-do compile } */
+
+typedef unsigned int __u32;
+
+__u32
+__rev16_32_alt (__u32 x)
+{
+  return (((__u32)(x) & (__u32)0xff00ff00UL) >> 8)
+ | (((__u32)(x) & (__u32)0x00ff00ffUL) << 8);
+}
+
+__u32
+__rev16_32 (__u32 x)
+{
+  return (((__u32)(x) & (__u32)0x00ff00ffUL) << 8)
+ | (((__u32)(x) & (__u32)0xff00ff00UL) >> 8);
+}
+
+/* { dg-final { scan-assembler-times {rev16\tr[0-9]+, r[0-9]+} 2 } } */
\ No newline at end of file

Re: [PATCH][GCC][Arm] Add pattern for bswap + rotate -> rev16 [Bug 108933]

[Matthieu Longo]

Hi Richard,

Please find below the new patch where I addressed your comments and 
updated the changelog.


rev16 pattern was not recognised anymore as a change in the bswap tree
pass was introducing a new GIMPLE form, not recognized by the assembly
final transformation pass.

More details in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108933

gcc/ChangeLog:

PR target/108933
* config/arm/arm.md (arm_rev16si2): Convert to define_insn.
Correct generated RTL.
(arm_rev16si2_alt1): Correctly handle conditional execution.
(arm_rev16si2_alt2): Likewise.

gcc/testsuite/ChangeLog:

PR target/108933
* gcc.target/arm/rev16.c: Moved to...
* gcc.target/arm/rev16_1.c: ...here.
* gcc.target/arm/rev16_2.c: New test to check that rev16 is
emitted.

On 2024-01-22 16:25, Richard Earnshaw (lists) wrote:

On 22/01/2024 12:18, Matthieu Longo wrote:

rev16 pattern was not recognised anymore as a change in the bswap tree
pass was introducing a new GIMPLE form, not recognized by the assembly
final transformation pass.

More details in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108933

gcc/ChangeLog:

     PR target/108933
     * config/arm/arm.md (*arm_rev16si2_alt3): new pattern to convert
   a bswap + rotate by 16 bits into rev16


ChangeLog entries need to be written as sentences, so start with a capital letter and end 
with a full stop; continuation lines should start in column 8 (one hard tab, don't use 
spaces).  But in this case, "New pattern." is sufficient.



gcc/testsuite/ChangeLog:

     PR target/108933
     * gcc.target/arm/rev16.c: Moved to...
     * gcc.target/arm/rev16_1.c: ...here.
     * gcc.target/arm/rev16_2.c: New test to check that rev16 is
   emitted.



+;; Similar pattern to match (rotate (bswap) 16)
+(define_insn "*arm_rev16si2_alt3"
+  [(set (match_operand:SI 0 "register_operand" "=l,r")
+(rotate:SI (bswap:SI (match_operand:SI 1 "register_operand" "l,r"))
+ (const_int 16)))]
+  "arm_arch6"
+  "rev16\\t%0, %1"
+  [(set_attr "arch" "t,32")
+   (set_attr "length" "2,4")
+   (set_attr "type" "rev")]
+)
+

Unfortunately, this is insufficient.  When generating Arm or Thumb2 code (but 
not thumb1) we also have to handle conditional execution: we need to have '%?' 
in the output template at the point where a condition code might be needed.  
That means we need separate output templates for all three alternatives (as we 
need a 16-bit variant for thumb2 that's conditional and a 16-bit for thumb1 
that isn't).  See the output of arm_rev16 for a guide of what is really needed.

I note that the arm_rev16si2_alt1, and arm_rev16si2_alt2 patterns are incorrect 
in this regard as well; that will need fixing.

I also see that arm_rev16si2 currently expands to the alt1 variant above; given 
that the preferred canonical form would now appear to use bswap + rotate, we 
should change that as well.  In fact, we can merge your new pattern with the 
expand entirely and eliminate the need to call gen_arm_rev16si2_alt1.  
Something like:

(define_insn "arm_rev16si2"
   [(set (match_operand:SI 0 "s_register_operand")
 (rotate:SI (bswap:SI (match_operand:SI 1 "s_register_operand")) 
(const_int 16))]
   "arm_arch6"
   "@
   rev16...
   ...


R.
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 
4a98f2d7b6251da940806b26d4c310a7f7af927b..5816409f86f1106b410c5e21d77e599b485f85f2
 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -12578,7 +12578,10 @@ (define_insn "arm_rev16si2_alt1"
   "arm_arch6
&& aarch_rev16_shleft_mask_imm_p (operands[3], SImode)
&& aarch_rev16_shright_mask_imm_p (operands[2], SImode)"
-  "rev16\\t%0, %1"
+  "@
+   rev16\t%0, %1
+   rev16%?\t%0, %1
+   rev16%?\t%0, %1"
   [(set_attr "arch" "t1,t2,32")
(set_attr "length" "2,2,4")
(set_attr "type" "rev")]
@@ -12595,22 +12598,28 @@ (define_insn "*arm_rev16si2_alt2"
   "arm_arch6
&& aarch_rev16_shleft_mask_imm_p (operands[3], SImode)
&& aarch_rev16_shright_mask_imm_p (operands[2], SImode)"
-  "rev16\\t%0, %1"
+  "@
+   rev16\t%0, %1
+   rev16%?\t%0, %1
+   rev16%?\t%0, %1"
   [(set_attr "arch" "t1,t2,32")
(set_attr "length" "2,2,4")
(set_attr "type" "rev")]
 )
 
-(define_expand "arm_rev16si2"
-  [(set (match_operand:SI 0 "s_register_operand")
-   (bswap:SI (match_operand:SI 1 "s_register_operand")))]
+;; Similar pattern to match (rotate (bswap) 16)
+(define_insn "arm_

[PATCH][GCC][Arm] Define __ARM_FEATURE_BF16 when +bf16 feature is enabled

[Matthieu Longo]

Hi,

Arm GCC backend does not define __ARM_FEATURE_BF16 when +bf16 is 
specified (via -march option, or target pragma) whereas it is supposed 
to be tested before including arm_bf16.h (as specified in ACLE document: 
https://arm-software.github.io/acle/main/acle.html#arm_bf16h).


gcc/ChangeLog:

* config/arm/arm-c.cc (arm_cpu_builtins): define __ARM_FEATURE_BF16
* config/arm/arm.h: define TARGET_BF16

Ok for master ?

Matthieudiff --git a/gcc/config/arm/arm-c.cc b/gcc/config/arm/arm-c.cc
index 
2e181bf7f36bab1209d5358e65d9513541683632..21ca22ac71119eda4ff01709aa95002ca13b1813
 100644
--- a/gcc/config/arm/arm-c.cc
+++ b/gcc/config/arm/arm-c.cc
@@ -425,12 +425,14 @@ arm_cpu_builtins (struct cpp_reader* pfile)
   arm_arch_cde_coproc);
 
   def_or_undef_macro (pfile, "__ARM_FEATURE_MATMUL_INT8", TARGET_I8MM);
+
+  def_or_undef_macro (pfile, "__ARM_FEATURE_BF16", TARGET_BF16);
+  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
+ TARGET_BF16_FP);
   def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_SCALAR_ARITHMETIC",
  TARGET_BF16_FP);
   def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_VECTOR_ARITHMETIC",
  TARGET_BF16_SIMD);
-  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
- TARGET_BF16_FP || TARGET_BF16_SIMD);
 }
 
 void
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 
2a2207c0ba1acef1c7082c89bf5f542b1466d033..e7a7fc47e606d2ead5f778dca2e63b2e894d0efe
 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -252,10 +252,10 @@ emission of floating point pcs attributes.  */
 #define TARGET_I8MM (TARGET_NEON && arm_arch8_2 && arm_arch_i8mm)
 
 /* FPU supports Brain half-precision floating-point (BFloat16) extension.  */
-#define TARGET_BF16_FP (TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_VFP5 \
-   && arm_arch8_2 && arm_arch_bf16)
-#define TARGET_BF16_SIMD (TARGET_NEON && TARGET_VFP5 \
- && arm_arch8_2 && arm_arch_bf16)
+#define TARGET_BF16 (TARGET_32BIT && TARGET_HARD_FLOAT && arm_arch8_2 \
+   && TARGET_VFP5 && arm_arch_bf16)
+#define TARGET_BF16_FP (TARGET_BF16)
+#define TARGET_BF16_SIMD (TARGET_BF16 && TARGET_NEON)
 
 /* Q-bit is present.  */
 #define TARGET_ARM_QBIT \

[PATCH v1 0/3][libgcc] store signing key and signing method in DWARF _Unwind_FrameState

[Matthieu Longo]

This patch series is only a refactoring of the existing implementation of PAuth 
and returned-address signing. The existing behavior is preserved.

1. aarch64: store signing key and signing method in DWARF _Unwind_FrameState

_Unwind_FrameState already contains several CIE and FDE information (see the 
attributes below the comment "The information we care about from the CIE/FDE" 
in libgcc/unwind-dw2.h).
The patch aims at moving the information from DWARF CIE (signing key stored in 
the augmentation string) and FDE (the used signing method) into 
_Unwind_FrameState along the already-stored CIE and FDE information.
Note: those information have to be saved in frame_state_reg_info instead of 
_Unwind_FrameState as they need to be savable by DW_CFA_remember_state and 
restorable by DW_CFA_restore_state, that both rely on the attribute "prev".
Those new information in _Unwind_FrameState simplifies the look-up of the 
signing key when the return address is demangled. It also allows future signing 
methods to be easily added.
_Unwind_FrameState is not a part of the public API of libunwind, so the change 
is backward compatible.

A new architecture-specific handler MD_ARCH_EXTENSION_FRAME_INIT allows to 
reset values in the frame state and unwind context if needed by the 
architecture extension before changing the frame state to the caller context.
A new architecture-specific handler MD_ARCH_EXTENSION_CIE_AUG_HANDLER isolates 
the architecture-specific augmentation strings in AArch64 backend, and allows 
others architectures to reuse augmentation strings that would have clashed with 
AArch64 DWARF extensions.
aarch64_demangle_return_addr, DW_CFA_AARCH64_negate_ra_state and 
DW_CFA_val_expression cases in libgcc/unwind-dw2-execute_cfa.h were documented 
to clarify where the value of the RA state register is stored (FS and CONTEXT 
respectively).

2. libgcc: hide CIE and FDE data for DWARF architecture extensions behind a 
handler.

This patch provides a new handler MD_ARCH_FRAME_STATE_T to hide an 
architecture-specific structure containing CIE and FDE data related to DWARF 
architecture extensions.
Hiding the architecture-specific attributes behind a handler has the following 
benefits:
1. isolating those data from the generic ones in _Unwind_FrameState
2. avoiding casts to custom types.
3. preserving typing information when debugging with GDB, and so 
facilitating their printing.

This approach required to add a new header md-unwind-def.h included at the top 
of libgcc/unwind-dw2.h, and redirecting to the corresponding architecture 
header via a symbolic link.
An obvious drawback is the increase in complexity with macros, and headers. It 
also caused a split of architecture definitions between md-unwind-def.h (types 
definitions used in unwind-dw2.h) and md-unwind.h (local types definitions and 
handlers implementations).
The naming of md-unwind.h with .h extension is a bit misleading as the file is 
only included in the middle of unwind-dw2.c. Changing this naming would require 
modification of others backends, which I prefered to abstain from.
Overall the benefits are worth the added complexity from my perspective.

3. libgcc: update configure (regenerated by autoreconf)

Regenerate the build files.


## Testing

Those changes were testing by covering the 3 following cases:
- backtracing.
- exception handling in a C++ program.
- gcc/testsuite/gcc.target/aarch64/pr104689.c: pac-ret with unusual DWARF [1]

Regression tested on aarch64-unknown-linux-gnu, and no regression found.

[1]: https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594414.html


Ok for master? I don't have commit access so I need someone to commit on my 
behalf.

Regards,
Matthieu.


Matthieu Longo (3):
  aarch64: store signing key and signing method in DWARF
_Unwind_FrameState
  libgcc: hide CIE and FDE data for DWARF architecture extensions behind
a handler.
  libgcc: update configure (regenerated by autoreconf)

 libgcc/Makefile.in |   6 +-
 libgcc/config.host |  13 +-
 libgcc/config/aarch64/aarch64-unwind-def.h |  41 ++
 libgcc/config/aarch64/aarch64-unwind.h | 150 +
 libgcc/configure   |   2 +
 libgcc/configure.ac|   1 +
 libgcc/unwind-dw2-execute_cfa.h|  34 +++--
 libgcc/unwind-dw2.c|  19 ++-
 libgcc/unwind-dw2.h|  19 ++-
 9 files changed, 233 insertions(+), 52 deletions(-)
 create mode 100644 libgcc/config/aarch64/aarch64-unwind-def.h

-- 
2.45.2

[PATCH v1 1/3] aarch64: store signing key and signing method in DWARF _Unwind_FrameState

[Matthieu Longo]

This patch is only a refactoring of the existing implementation
of PAuth and returned-address signing. The existing behavior is
preserved.

_Unwind_FrameState already contains several CIE and FDE information
(see the attributes below the comment "The information we care
about from the CIE/FDE" in libgcc/unwind-dw2.h).
The patch aims at moving the information from DWARF CIE (signing
key stored in the augmentation string) and FDE (the used signing
method) into _Unwind_FrameState along the already-stored CIE and
FDE information.
Note: those information have to be saved in frame_state_reg_info
instead of _Unwind_FrameState as they need to be savable by
DW_CFA_remember_state and restorable by DW_CFA_restore_state, that
both rely on the attribute "prev".

Those new information in _Unwind_FrameState simplifies the look-up
of the signing key when the return address is demangled. It also
allows future signing methods to be easily added.

_Unwind_FrameState is not a part of the public API of libunwind,
so the change is backward compatible.

A new architecture-specific handler MD_ARCH_EXTENSION_FRAME_INIT
allows to reset values (if needed) in the frame state and unwind
context before changing the frame state to the caller context.

A new architecture-specific handler MD_ARCH_EXTENSION_CIE_AUG_HANDLER
isolates the architecture-specific augmentation strings in AArch64
backend, and allows others architectures to reuse augmentation
strings that would have clashed with AArch64 DWARF extensions.

aarch64_demangle_return_addr, DW_CFA_AARCH64_negate_ra_state and
DW_CFA_val_expression cases in libgcc/unwind-dw2-execute_cfa.h
were documented to clarify where the value of the RA state register
is stored (FS and CONTEXT respectively).

libgcc/ChangeLog:

  * config/aarch64/aarch64-unwind.h
  (AARCH64_DWARF_RA_STATE_MASK): The mask for RA state register.
  (aarch64_RA_signing_method_t): The diversifiers used to sign a
  function's return address.
  (aarch64_pointer_auth_key): The key used to sign a function's
  return address.
  (aarch64_cie_signed_with_b_key): Deleted as the signing key is
  available now in _Unwind_FrameState.
  (MD_ARCH_EXTENSION_CIE_AUG_HANDLER): New CIE augmentation string
  handler for architecture extensions.
  (MD_ARCH_EXTENSION_FRAME_INIT): New architecture-extension
  initialization routine for DWARF frame state and context before
  execution of DWARF instructions.
  (aarch64_context_RA_state_get): Read RA state register from CONTEXT.
  (aarch64_RA_state_get): Read RA state register from FS.
  (aarch64_RA_state_set): Write RA state register into FS.
  (aarch64_RA_state_toggle): Toggle RA state register in FS.
  (aarch64_cie_aug_handler): Handler AArch64 augmentation strings.
  (aarch64_arch_extension_frame_init): Initialize defaults for the
  signing key (PAUTH_KEY_A), and RA state register (RA_no_signing).
  (aarch64_demangle_return_addr): Rely on the frame registers and
  the signing_key attribute in _Unwind_FrameState.
  * unwind-dw2-execute_cfa.h:
  Use the right alias DW_CFA_AARCH64_negate_ra_state for __aarch64__
  instead of DW_CFA_GNU_window_save.
  (DW_CFA_AARCH64_negate_ra_state): Save the signing method in RA
  state register. Toggle RA state register without resetting 'how'
  to REG_UNSAVED.
  * unwind-dw2.c:
  (extract_cie_info): Save the signing key in the current
  _Unwind_FrameState while parsing the augmentation data.
  (uw_frame_state_for): Reset some attributes related to architecture
  extensions in _Unwind_FrameState.
  (uw_update_context): Move authentication code to AArch64 unwinding.
  * unwind-dw2.h (enum register_rule): Give a name to the existing
  enum for the register rules, and replace 'unsigned char' by 'enum
  register_rule' to facilitate debugging in GDB.
  (_Unwind_FrameState): Add a new architecture-extension attribute
  to store the signing key.
---
 libgcc/config/aarch64/aarch64-unwind.h | 154 -
 libgcc/unwind-dw2-execute_cfa.h|  34 --
 libgcc/unwind-dw2.c|  19 ++-
 libgcc/unwind-dw2.h|  17 ++-
 4 files changed, 175 insertions(+), 49 deletions(-)

diff --git a/libgcc/config/aarch64/aarch64-unwind.h 
b/libgcc/config/aarch64/aarch64-unwind.h
index daf96624b5e..cc225a7e207 100644
--- a/libgcc/config/aarch64/aarch64-unwind.h
+++ b/libgcc/config/aarch64/aarch64-unwind.h
@@ -25,55 +25,155 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
If not, see
 #if !defined (AARCH64_UNWIND_H) && !defined (__ILP32__)
 #define AARCH64_UNWIND_H
 
-#define DWARF_REGNUM_AARCH64_RA_STATE 34
+#include "ansidecl.h"
+#include 
+
+#define AARCH64_DWARF_REGNUM_RA_STATE 34
+#define AARCH64_DWARF_RA_STATE_MASK   0x1
+
+/* The diversifiers used to sign a function's return address. */
+typedef enum
+{
+  AARCH64_RA_no_signing = 0x0,
+  AARCH64_RA_signing_SP = 0x1,
+} __attribute__((packed)) aarch64_RA_signing_method_t;
+
+/* The key used to sign a function's return address. */
+typedef enum {
+  AARCH64_PAUTH_KE

[PATCH v1 2/3] libgcc: hide CIE and FDE data for DWARF architecture extensions behind a handler.

[Matthieu Longo]

This patch provides a new handler MD_ARCH_FRAME_STATE_T to hide an
architecture-specific structure containing CIE and FDE data related
to DWARF architecture extensions.

Hiding the architecture-specific attributes behind a handler has the
following benefits:
1. isolating those data from the generic ones in _Unwind_FrameState
2. avoiding casts to custom types.
3. preserving typing information when debugging with GDB, and so
   facilitating their printing.

This approach required to add a new header md-unwind-def.h included at
the top of libgcc/unwind-dw2.h, and redirecting to the corresponding
architecture header via a symbolic link.

An obvious drawback is the increase in complexity with macros, and
headers. It also caused a split of architecture definitions between
md-unwind-def.h (types definitions used in unwind-dw2.h) and
md-unwind.h (local types definitions and handlers implementations).
The naming of md-unwind.h with .h extension is a bit misleading as
the file is only included in the middle of unwind-dw2.c. Changing
this naming would require modification of others backends, which I
prefered to abstain from. Overall the benefits are worth the added
complexity from my perspective.

libgcc/ChangeLog:

* Makefile.in: New target for symbolic link to md-unwind-def.h
* config.host: New parameter md_unwind_def_header. Set it to
aarch64/aarch64-unwind-def.h for AArch64 targets, or no-unwind.h
by default.
* config/aarch64/aarch64-unwind.h
(aarch64_pointer_auth_key): Move to aarch64-unwind-def.h
(aarch64_cie_aug_handler): Update.
(aarch64_arch_extension_frame_init): Update.
(aarch64_demangle_return_addr): Update.
* configure.ac: New substitute variable md_unwind_def_header.
* unwind-dw2.h (defined): MD_ARCH_FRAME_STATE_T.
* config/aarch64/aarch64-unwind-def.h: New file.
---
 libgcc/Makefile.in |  6 +++-
 libgcc/config.host | 13 +--
 libgcc/config/aarch64/aarch64-unwind-def.h | 41 ++
 libgcc/config/aarch64/aarch64-unwind.h | 14 +++-
 libgcc/configure.ac|  1 +
 libgcc/unwind-dw2.h|  6 ++--
 6 files changed, 67 insertions(+), 14 deletions(-)
 create mode 100644 libgcc/config/aarch64/aarch64-unwind-def.h

diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index 0e46e9ef768..ffc45f21267 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -47,6 +47,7 @@ with_aix_soname = @with_aix_soname@
 solaris_ld_v2_maps = @solaris_ld_v2_maps@
 enable_execute_stack = @enable_execute_stack@
 unwind_header = @unwind_header@
+md_unwind_def_header = @md_unwind_def_header@
 md_unwind_header = @md_unwind_header@
 sfp_machine_header = @sfp_machine_header@
 thread_header = @thread_header@
@@ -358,13 +359,16 @@ SHLIBUNWIND_INSTALL =
 
 
 # Create links to files specified in config.host.
-LIBGCC_LINKS = enable-execute-stack.c unwind.h md-unwind-support.h \
+LIBGCC_LINKS = enable-execute-stack.c \
+   unwind.h md-unwind-def.h md-unwind-support.h \
sfp-machine.h gthr-default.h
 
 enable-execute-stack.c: $(srcdir)/$(enable_execute_stack)
-$(LN_S) $< $@
 unwind.h: $(srcdir)/$(unwind_header)
-$(LN_S) $< $@
+md-unwind-def.h: $(srcdir)/config/$(md_unwind_def_header)
+   -$(LN_S) $< $@
 md-unwind-support.h: $(srcdir)/config/$(md_unwind_header)
-$(LN_S) $< $@
 sfp-machine.h: $(srcdir)/config/$(sfp_machine_header)
diff --git a/libgcc/config.host b/libgcc/config.host
index 9fae51d4ce7..61825e72fe4 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -51,8 +51,10 @@
 #  If either is set, EXTRA_PARTS and
 #  EXTRA_MULTILIB_PARTS inherited from the GCC
 #  subdirectory will be ignored.
-#  md_unwind_headerThe name of a header file defining
-#  MD_FALLBACK_FRAME_STATE_FOR.
+#  md_unwind_def_header The name of a header file defining 
architecture-specific
+#  frame information types for unwinding.
+#  md_unwind_headerThe name of a header file defining architecture-specific
+#  handlers used in the unwinder.
 #  sfp_machine_header  The name of a sfp-machine.h header file for soft-fp.
 #  Defaults to "$cpu_type/sfp-machine.h" if it exists,
 #  no-sfp-machine.h otherwise.
@@ -72,6 +74,7 @@ extra_parts=
 tmake_file=
 tm_file=
 tm_define=
+md_unwind_def_header=no-unwind.h
 md_unwind_header=no-unwind.h
 unwind_header=unwind-generic.h
 
@@ -403,6 +406,7 @@ aarch64*-*-elf | aarch64*-*-rtems*)
tmake_file="${tmake_file} ${cpu_type}/t-lse t-slibgcc-libgcc"
tmake_file="${tmake_file} ${cpu_type}/t-softfp t-softfp t-crtfm"
tmake_file="${tmake_file} t-dfprules"
+   md_unwind_def_header=aarch64/aarch64-unwind-def.h
md_unwind_header=aarch64/aarch64-unwind.h
;;
 aarch64*-*-freebsd*)
@@ -411,6 +415,7 @@ aarch64*-*-freebs

[PATCH v1 3/3] libgcc: update configure (regenerated by autoreconf)

[Matthieu Longo]

libgcc/ChangeLog:

* configure: Regenerate.
---
 libgcc/configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libgcc/configure b/libgcc/configure
index a69d314374a..15a0be23644 100755
--- a/libgcc/configure
+++ b/libgcc/configure
@@ -587,6 +587,7 @@ ac_includes_default='/* none */'
 ac_subst_vars='LTLIBOBJS
 LIBOBJS
 md_unwind_header
+md_unwind_def_header
 unwind_header
 enable_execute_stack
 asm_hidden_op
@@ -5786,6 +5787,7 @@ fi
 
 
 
+
 # We need multilib support.
 ac_config_files="$ac_config_files Makefile"
 
-- 
2.45.2

Re: [PATCH v1 1/3] aarch64: store signing key and signing method in DWARF _Unwind_FrameState

[Matthieu Longo]

On 2024-07-19 15:54, Matthieu Longo wrote:

This patch is only a refactoring of the existing implementation
of PAuth and returned-address signing. The existing behavior is
preserved.

_Unwind_FrameState already contains several CIE and FDE information
(see the attributes below the comment "The information we care
about from the CIE/FDE" in libgcc/unwind-dw2.h).
The patch aims at moving the information from DWARF CIE (signing
key stored in the augmentation string) and FDE (the used signing
method) into _Unwind_FrameState along the already-stored CIE and
FDE information.
Note: those information have to be saved in frame_state_reg_info
instead of _Unwind_FrameState as they need to be savable by
DW_CFA_remember_state and restorable by DW_CFA_restore_state, that
both rely on the attribute "prev".

Those new information in _Unwind_FrameState simplifies the look-up
of the signing key when the return address is demangled. It also
allows future signing methods to be easily added.

_Unwind_FrameState is not a part of the public API of libunwind,
so the change is backward compatible.

A new architecture-specific handler MD_ARCH_EXTENSION_FRAME_INIT
allows to reset values (if needed) in the frame state and unwind
context before changing the frame state to the caller context.

A new architecture-specific handler MD_ARCH_EXTENSION_CIE_AUG_HANDLER
isolates the architecture-specific augmentation strings in AArch64
backend, and allows others architectures to reuse augmentation
strings that would have clashed with AArch64 DWARF extensions.

aarch64_demangle_return_addr, DW_CFA_AARCH64_negate_ra_state and
DW_CFA_val_expression cases in libgcc/unwind-dw2-execute_cfa.h
were documented to clarify where the value of the RA state register
is stored (FS and CONTEXT respectively).

libgcc/ChangeLog:

   * config/aarch64/aarch64-unwind.h
   (AARCH64_DWARF_RA_STATE_MASK): The mask for RA state register.
   (aarch64_RA_signing_method_t): The diversifiers used to sign a
   function's return address.
   (aarch64_pointer_auth_key): The key used to sign a function's
   return address.
   (aarch64_cie_signed_with_b_key): Deleted as the signing key is
   available now in _Unwind_FrameState.
   (MD_ARCH_EXTENSION_CIE_AUG_HANDLER): New CIE augmentation string
   handler for architecture extensions.
   (MD_ARCH_EXTENSION_FRAME_INIT): New architecture-extension
   initialization routine for DWARF frame state and context before
   execution of DWARF instructions.
   (aarch64_context_RA_state_get): Read RA state register from CONTEXT.
   (aarch64_RA_state_get): Read RA state register from FS.
   (aarch64_RA_state_set): Write RA state register into FS.
   (aarch64_RA_state_toggle): Toggle RA state register in FS.
   (aarch64_cie_aug_handler): Handler AArch64 augmentation strings.
   (aarch64_arch_extension_frame_init): Initialize defaults for the
   signing key (PAUTH_KEY_A), and RA state register (RA_no_signing).
   (aarch64_demangle_return_addr): Rely on the frame registers and
   the signing_key attribute in _Unwind_FrameState.
   * unwind-dw2-execute_cfa.h:
   Use the right alias DW_CFA_AARCH64_negate_ra_state for __aarch64__
   instead of DW_CFA_GNU_window_save.
   (DW_CFA_AARCH64_negate_ra_state): Save the signing method in RA
   state register. Toggle RA state register without resetting 'how'
   to REG_UNSAVED.
   * unwind-dw2.c:
   (extract_cie_info): Save the signing key in the current
   _Unwind_FrameState while parsing the augmentation data.
   (uw_frame_state_for): Reset some attributes related to architecture
   extensions in _Unwind_FrameState.
   (uw_update_context): Move authentication code to AArch64 unwinding.
   * unwind-dw2.h (enum register_rule): Give a name to the existing
   enum for the register rules, and replace 'unsigned char' by 'enum
   register_rule' to facilitate debugging in GDB.
   (_Unwind_FrameState): Add a new architecture-extension attribute
   to store the signing key.
---
  libgcc/config/aarch64/aarch64-unwind.h | 154 -
  libgcc/unwind-dw2-execute_cfa.h|  34 --
  libgcc/unwind-dw2.c|  19 ++-
  libgcc/unwind-dw2.h|  17 ++-
  4 files changed, 175 insertions(+), 49 deletions(-)

diff --git a/libgcc/config/aarch64/aarch64-unwind.h 
b/libgcc/config/aarch64/aarch64-unwind.h
index daf96624b5e..cc225a7e207 100644
--- a/libgcc/config/aarch64/aarch64-unwind.h
+++ b/libgcc/config/aarch64/aarch64-unwind.h
@@ -25,55 +25,155 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
If not, see
  #if !defined (AARCH64_UNWIND_H) && !defined (__ILP32__)
  #define AARCH64_UNWIND_H
  
-#define DWARF_REGNUM_AARCH64_RA_STATE 34

+#include "ansidecl.h"
+#include 
+
+#define AARCH64_DWARF_REGNUM_RA_STATE 34
+#define AARCH64_DWARF_RA_STATE_MASK   0x1
+
+/* The diversifiers used to sign a function's return address. */
+typedef enum
+{
+  AARCH64_RA_no_signing = 0x0,
+  

Re: [PATCH v1 0/3][libgcc] store signing key and signing method in DWARF _Unwind_FrameState

[Matthieu Longo]

On 2024-07-19 15:54, Matthieu Longo wrote:

This patch series is only a refactoring of the existing implementation of PAuth 
and returned-address signing. The existing behavior is preserved.

1. aarch64: store signing key and signing method in DWARF _Unwind_FrameState

_Unwind_FrameState already contains several CIE and FDE information (see the attributes 
below the comment "The information we care about from the CIE/FDE" in 
libgcc/unwind-dw2.h).
The patch aims at moving the information from DWARF CIE (signing key stored in 
the augmentation string) and FDE (the used signing method) into 
_Unwind_FrameState along the already-stored CIE and FDE information.
Note: those information have to be saved in frame_state_reg_info instead of 
_Unwind_FrameState as they need to be savable by DW_CFA_remember_state and restorable by 
DW_CFA_restore_state, that both rely on the attribute "prev".
Those new information in _Unwind_FrameState simplifies the look-up of the 
signing key when the return address is demangled. It also allows future signing 
methods to be easily added.
_Unwind_FrameState is not a part of the public API of libunwind, so the change 
is backward compatible.

A new architecture-specific handler MD_ARCH_EXTENSION_FRAME_INIT allows to 
reset values in the frame state and unwind context if needed by the 
architecture extension before changing the frame state to the caller context.
A new architecture-specific handler MD_ARCH_EXTENSION_CIE_AUG_HANDLER isolates 
the architecture-specific augmentation strings in AArch64 backend, and allows 
others architectures to reuse augmentation strings that would have clashed with 
AArch64 DWARF extensions.
aarch64_demangle_return_addr, DW_CFA_AARCH64_negate_ra_state and 
DW_CFA_val_expression cases in libgcc/unwind-dw2-execute_cfa.h were documented 
to clarify where the value of the RA state register is stored (FS and CONTEXT 
respectively).

2. libgcc: hide CIE and FDE data for DWARF architecture extensions behind a 
handler.

This patch provides a new handler MD_ARCH_FRAME_STATE_T to hide an 
architecture-specific structure containing CIE and FDE data related to DWARF 
architecture extensions.
Hiding the architecture-specific attributes behind a handler has the following 
benefits:
 1. isolating those data from the generic ones in _Unwind_FrameState
 2. avoiding casts to custom types.
 3. preserving typing information when debugging with GDB, and so 
facilitating their printing.

This approach required to add a new header md-unwind-def.h included at the top 
of libgcc/unwind-dw2.h, and redirecting to the corresponding architecture 
header via a symbolic link.
An obvious drawback is the increase in complexity with macros, and headers. It 
also caused a split of architecture definitions between md-unwind-def.h (types 
definitions used in unwind-dw2.h) and md-unwind.h (local types definitions and 
handlers implementations).
The naming of md-unwind.h with .h extension is a bit misleading as the file is 
only included in the middle of unwind-dw2.c. Changing this naming would require 
modification of others backends, which I prefered to abstain from.
Overall the benefits are worth the added complexity from my perspective.

3. libgcc: update configure (regenerated by autoreconf)

Regenerate the build files.


## Testing

Those changes were testing by covering the 3 following cases:
- backtracing.
- exception handling in a C++ program.
- gcc/testsuite/gcc.target/aarch64/pr104689.c: pac-ret with unusual DWARF [1]

Regression tested on aarch64-unknown-linux-gnu, and no regression found.

[1]: https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594414.html


Ok for master? I don't have commit access so I need someone to commit on my 
behalf.

Regards,
Matthieu.


Matthieu Longo (3):
   aarch64: store signing key and signing method in DWARF
 _Unwind_FrameState
   libgcc: hide CIE and FDE data for DWARF architecture extensions behind
 a handler.
   libgcc: update configure (regenerated by autoreconf)

  libgcc/Makefile.in |   6 +-
  libgcc/config.host |  13 +-
  libgcc/config/aarch64/aarch64-unwind-def.h |  41 ++
  libgcc/config/aarch64/aarch64-unwind.h | 150 +
  libgcc/configure   |   2 +
  libgcc/configure.ac|   1 +
  libgcc/unwind-dw2-execute_cfa.h|  34 +++--
  libgcc/unwind-dw2.c|  19 ++-
  libgcc/unwind-dw2.h|  19 ++-
  9 files changed, 233 insertions(+), 52 deletions(-)
  create mode 100644 libgcc/config/aarch64/aarch64-unwind-def.h



Adding Ian in CC as he is listed as the maintainer of libgcc in 
MAINTAINERS file.

[PATCH v1 0/4] dwarf2: add hooks for architecture-specific CFIs

[Matthieu Longo]

Architecture-specific CFI directives are currently declared an processed among 
others architecture-independent CFI directives in gcc/dwarf2* files. This 
approach creates confusion, specifically in the case of DWARF instructions in 
the vendor space and using the same instruction code.
Such a clash currently happen between DW_CFA_GNU_window_save (used on SPARC) 
and DW_CFA_AARCH64_negate_ra_state (used on AArch64), and both having the same 
instruction code 0x2d. Then AArch64 compilers generates a SPARC CFI directive 
(.cfi_window_save) instead of .cfi_negate_ra_state, contrarily to what is 
expected in [1].

1. Rename REG_CFA_TOGGLE_RA_MANGLE to REG_CFA_NEGATE_RA_STATE

This patch renames:
- dwarf2out_frame_debug_cfa_toggle_ra_mangle to 
dwarf2out_frame_debug_cfa_negate_ra_state,
- REG_CFA_TOGGLE_RA_MANGLE to REG_CFA_NEGATE_RA_STATE,
as the naming was misleading.
The word "toggle" suggested a binary state, whereas this register stores the 
mangling state (that can be more than 2 states) for the return address on 
AArch64.

2. dwarf2: add hooks for architecture-specific CFIs

This refactoring does not solve completely the problem, but improve the 
situation by moving some of the processing of those directives (more 
specifically their output in the assembly) to the backend via 2 target hooks:
- DW_CFI_OPRND1_DESC: parse the first operand of the directive (if any).
- OUTPUT_CFI_DIRECTIVE: output the CFI directive as a string.
Only AArch64's and SPARC's backend are impacted.

3. aarch64 testsuite: explain expectections for pr94515*
PR94515's tests in AArch64 G++ testsuite were lacking documentation. They are 
now thoroughly documented.

4. dwarf2: store the RA state in CFI row

On AArch64, the RA state informs the unwinder whether the return address is 
mangled and how, or not. This information is encoded in a boolean in the CFI 
row. This binary approach prevents from expressing more complex configuration, 
as it is the case with PAuth_LR introduced in Armv9.5-A.
This patch addresses this limitation by replacing the boolean by an enum.


References:
[1] DWARF for the Arm 64-bit Architecture (AArch64) --> 
https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst

## Testing

Built for target aarch64-unknown-linux-gnu and ran GCC's & G++'s testsuites for 
AArch64.
Built GCC stage 1 for target sparc64-unknown-linux-gnu.


Ok for master? I don't have commit access so I need someone to commit on my 
behalf.

Regards,
Matthieu.


Matthieu Longo (4):
  Rename REG_CFA_TOGGLE_RA_MANGLE to REG_CFA_NEGATE_RA_STATE
  dwarf2: add hooks for architecture-specific CFIs
  aarch64 testsuite: explain expectections for pr94515* tests
  dwarf2: store the RA state in CFI row

 gcc/combine-stack-adj.cc |  2 +-
 gcc/config/aarch64/aarch64.cc| 36 -
 gcc/config/sparc/sparc.cc| 36 +
 gcc/coretypes.h  |  6 +++
 gcc/doc/tm.texi  | 28 ++
 gcc/doc/tm.texi.in   | 17 ++
 gcc/dwarf2cfi.cc | 57 ++--
 gcc/dwarf2out.cc | 13 +++--
 gcc/dwarf2out.h  | 10 ++--
 gcc/reg-notes.def|  8 +--
 gcc/target.def   | 20 +++
 gcc/testsuite/g++.target/aarch64/pr94515-1.C | 14 +++--
 gcc/testsuite/g++.target/aarch64/pr94515-2.C | 30 ---
 libffi/include/ffi_cfi.h |  2 +
 libgcc/config/aarch64/aarch64-asm.h  |  4 +-
 libitm/config/aarch64/sjlj.S | 10 ++--
 16 files changed, 233 insertions(+), 60 deletions(-)

-- 
2.46.0

[PATCH v1 1/4] Rename REG_CFA_TOGGLE_RA_MANGLE to REG_CFA_NEGATE_RA_STATE

[Matthieu Longo]

The current name REG_CFA_TOGGLE_RA_MANGLE is not representative of what
it really is, i.e. a register to represent several states, not only a
binary one. Same for dwarf2out_frame_debug_cfa_toggle_ra_mangle.

gcc/ChangeLog:

* combine-stack-adj.cc
(no_unhandled_cfa): Rename.
* config/aarch64/aarch64.cc
(aarch64_expand_prologue): Rename.
(aarch64_expand_epilogue): Rename.
* dwarf2cfi.cc
(dwarf2out_frame_debug_cfa_toggle_ra_mangle): Rename this...
(dwarf2out_frame_debug_cfa_negate_ra_state): To this.
(dwarf2out_frame_debug): Rename.
* reg-notes.def (REG_CFA_NOTE): Rename REG_CFA_TOGGLE_RA_MANGLE.
---
 gcc/combine-stack-adj.cc  | 2 +-
 gcc/config/aarch64/aarch64.cc | 4 ++--
 gcc/dwarf2cfi.cc  | 8 
 gcc/reg-notes.def | 8 
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/gcc/combine-stack-adj.cc b/gcc/combine-stack-adj.cc
index 2da9bf2bc1e..367d3b66b74 100644
--- a/gcc/combine-stack-adj.cc
+++ b/gcc/combine-stack-adj.cc
@@ -212,7 +212,7 @@ no_unhandled_cfa (rtx_insn *insn)
   case REG_CFA_SET_VDRAP:
   case REG_CFA_WINDOW_SAVE:
   case REG_CFA_FLUSH_QUEUE:
-  case REG_CFA_TOGGLE_RA_MANGLE:
+  case REG_CFA_NEGATE_RA_STATE:
return false;
   }
 
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index e0cf382998c..0af5d85c36f 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -9606,7 +9606,7 @@ aarch64_expand_prologue (void)
  default:
gcc_unreachable ();
}
-  add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
+  add_reg_note (insn, REG_CFA_NEGATE_RA_STATE, const0_rtx);
   RTX_FRAME_RELATED_P (insn) = 1;
 }
 
@@ -10027,7 +10027,7 @@ aarch64_expand_epilogue (rtx_call_insn *sibcall)
  default:
gcc_unreachable ();
}
-  add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
+  add_reg_note (insn, REG_CFA_NEGATE_RA_STATE, const0_rtx);
   RTX_FRAME_RELATED_P (insn) = 1;
 }
 
diff --git a/gcc/dwarf2cfi.cc b/gcc/dwarf2cfi.cc
index 1231b5bb5f0..4ad9acbd6fd 100644
--- a/gcc/dwarf2cfi.cc
+++ b/gcc/dwarf2cfi.cc
@@ -1547,13 +1547,13 @@ dwarf2out_frame_debug_cfa_window_save (void)
   cur_row->window_save = true;
 }
 
-/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_TOGGLE_RA_MANGLE.
+/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_NEGATE_RA_STATE.
Note: DW_CFA_GNU_window_save dwarf opcode is reused for toggling RA mangle
state, this is a target specific operation on AArch64 and can only be used
on other targets if they don't use the window save operation otherwise.  */
 
 static void
-dwarf2out_frame_debug_cfa_toggle_ra_mangle (void)
+dwarf2out_frame_debug_cfa_negate_ra_state (void)
 {
   dw_cfi_ref cfi = new_cfi ();
 
@@ -2341,8 +2341,8 @@ dwarf2out_frame_debug (rtx_insn *insn)
handled_one = true;
break;
 
-  case REG_CFA_TOGGLE_RA_MANGLE:
-   dwarf2out_frame_debug_cfa_toggle_ra_mangle ();
+  case REG_CFA_NEGATE_RA_STATE:
+   dwarf2out_frame_debug_cfa_negate_ra_state ();
handled_one = true;
break;
 
diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def
index 5b878fb2a1c..ddcf16b68be 100644
--- a/gcc/reg-notes.def
+++ b/gcc/reg-notes.def
@@ -180,10 +180,10 @@ REG_CFA_NOTE (CFA_WINDOW_SAVE)
the rest of the compiler as a CALL_INSN.  */
 REG_CFA_NOTE (CFA_FLUSH_QUEUE)
 
-/* Attached to insns that are RTX_FRAME_RELATED_P, toggling the mangling status
-   of return address.  Currently it's only used by AArch64.  The argument is
-   ignored.  */
-REG_CFA_NOTE (CFA_TOGGLE_RA_MANGLE)
+/* Attached to insns that are RTX_FRAME_RELATED_P, indicating an authentication
+   of the return address. Currently it's only used by AArch64.
+   The argument is ignored.  */
+REG_CFA_NOTE (CFA_NEGATE_RA_STATE)
 
 /* Indicates what exception region an INSN belongs in.  This is used
to indicate what region to which a call may throw.  REGION 0
-- 
2.46.0

[PATCH v1 3/4] aarch64 testsuite: explain expectections for pr94515* tests

[Matthieu Longo]

gcc/testsuite/ChangeLog:

* g++.target/aarch64/pr94515-1.C: Improve test documentation.
* g++.target/aarch64/pr94515-2.C: Same.
---
 gcc/testsuite/g++.target/aarch64/pr94515-1.C |  8 ++
 gcc/testsuite/g++.target/aarch64/pr94515-2.C | 28 +++-
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-1.C 
b/gcc/testsuite/g++.target/aarch64/pr94515-1.C
index d5c114a83a8..359039e1753 100644
--- a/gcc/testsuite/g++.target/aarch64/pr94515-1.C
+++ b/gcc/testsuite/g++.target/aarch64/pr94515-1.C
@@ -15,12 +15,20 @@ void unwind (void)
 __attribute__((noinline, noipa, target("branch-protection=pac-ret")))
 int test (int z)
 {
+  // paciasp -> cfi_negate_ra_state: RA_no_signing -> RA_signing_SP
   if (z) {
 asm volatile ("":::"x20","x21");
 unwind ();
+// autiasp -> cfi_negate_ra_state: RA_signing_SP -> RA_no_signing
 return 1;
   } else {
+// 2nd cfi_negate_ra_state because the CFI directives are processed 
linearily.
+// At this point, the unwinder would believe that the address is not signed
+// due to the previous return. That's why the compiler has to emit second
+// cfi_negate_ra_state to mean that the return address is still signed.
+// cfi_negate_ra_state: RA_no_signing -> RA_signing_SP
 unwind ();
+// autiasp -> cfi_negate_ra_state: RA_signing_SP -> RA_no_signing
 return 2;
   }
 }
diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-2.C 
b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
index f4abeed..af6b128b8fd 100644
--- a/gcc/testsuite/g++.target/aarch64/pr94515-2.C
+++ b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
@@ -6,6 +6,7 @@
 volatile int zero = 0;
 int global = 0;
 
+/* This is a leaf function, so no .cfi_negate_ra_state directive is expected.  
*/
 __attribute__((noinline))
 int bar(void)
 {
@@ -13,29 +14,44 @@ int bar(void)
   return 0;
 }
 
+/* This function does not return normally, so the address is signed but no
+ * authentication code is emitted. It means that only one CFI directive is
+ * supposed to be emitted at signing time.  */
 __attribute__((noinline, noreturn))
 void unwind (void)
 {
   throw 42;
 }
 
+/* This function has several return instructions, and alternates different RA
+ * states. 4 .cfi_negate_ra_state and a .cfi_remember_state/.cfi_restore_state
+ * should be emitted.
+ */
 __attribute__((noinline, noipa))
 int test(int x)
 {
-  if (x==1) return 2; /* This return path may not use the stack.  */
+  // This return path may not use the stack. This means that the return address
+  // won't be signed.
+  if (x==1) return 2;
+
+  // All the return paths of the code below must have RA mangle state set, and
+  // the return address must be signed.
   int y = bar();
   if (y > global) global=y;
-  if (y==3) unwind(); /* This return path must have RA mangle state set.  */
-  return 0;
+  if (y==3) unwind(); // authentication of the return address is not required.
+  return 0; // authentication of the return address is required.
 }
 
+/* This function requires only 2 .cfi_negate_ra_state.  */
 int main ()
 {
+  // paciasp -> cfi_negate_ra_state: RA_no_signing -> RA_signing_SP
   try {
 test (zero);
-__builtin_abort ();
+__builtin_abort (); // authentication of the return address is not 
required.
   } catch (...) {
+// autiasp -> cfi_negate_ra_state: RA_signing_SP -> RA_no_signing
 return 0;
   }
-  __builtin_abort ();
-}
+  __builtin_abort (); // authentication of the return address is not required.
+}
\ No newline at end of file
-- 
2.46.0

[PATCH v1 2/4] dwarf2: add hooks for architecture-specific CFIs

[Matthieu Longo]

Architecture-specific CFI directives are currently declared an processed
among others architecture-independent CFI directives in gcc/dwarf2* files.
This approach creates confusion, specifically in the case of DWARF
instructions in the vendor space and using the same instruction code.

Such a clash currently happen between DW_CFA_GNU_window_save (used on
SPARC) and DW_CFA_AARCH64_negate_ra_state (used on AArch64), and both
having the same instruction code 0x2d.
Then AArch64 compilers generates a SPARC CFI directive (.cfi_window_save)
instead of .cfi_negate_ra_state, contrarilly to what is expected in
[DWARF for the Arm 64-bit Architecture (AArch64)](https://github.com/
ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst).

This refactoring does not solve completely the problem, but improve the
situation by moving some of the processing of those directives (more
specifically their output in the assembly) to the backend via 2 target
hooks:
- DW_CFI_OPRND1_DESC: parse the first operand of the directive (if any).
- OUTPUT_CFI_DIRECTIVE: output the CFI directive as a string.

Additionally, this patch also contains a renaming of an enum used for
return address mangling on AArch64.

gcc/ChangeLog:

* config/aarch64/aarch64.cc
(aarch64_output_cfi_directive): New hook for CFI directives.
(aarch64_dw_cfi_oprnd1_desc): Same.
(TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive.
(TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc.
* config/sparc/sparc.cc
(sparc_output_cfi_directive): New hook for CFI directives.
(sparc_dw_cfi_oprnd1_desc): Same.
(TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive.
(TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc.
* coretypes.h
(struct dw_cfi_node): Forward declaration of CFI type from
gcc/dwarf2out.h.
(enum dw_cfi_oprnd_type): Same.
* doc/tm.texi: Regenerated from doc/tm.texi.in.
* doc/tm.texi.in: Add doc for OUTPUT_CFI_DIRECTIVE and
DW_CFI_OPRND1_DESC.
* dwarf2cfi.cc
(struct dw_cfi_row): Update the description for window_save
and ra_mangled.
(cfi_equal_p): Adapt parameter of dw_cfi_oprnd1_desc.
(dwarf2out_frame_debug_cfa_negate_ra_state): Use AArch64 CFI
directive instead of the SPARC one.
(change_cfi_row): Use the right CFI directive's name for RA
mangling.
(output_cfi): Remove explicit architecture-specific CFI
directive DW_CFA_GNU_window_save that falls into default case.
(output_cfi_directive): Use target hook as default.
* dwarf2out.cc (dw_cfi_oprnd1_desc): Use target hook as default.
* dwarf2out.h (enum dw_cfi_oprnd_type): specify underlying type
of enum to allow forward declaration.
(dw_cfi_oprnd1_desc): Change type of parameter.
(output_cfi_directive): Use dw_cfi_ref instead of struct
dw_cfi_node *.
* target.def: Documentation for new hooks.

libffi/ChangeLog:

* include/ffi_cfi.h (cfi_negate_ra_state): Declare AArch64 cfi
directive.

libgcc/ChangeLog:

* config/aarch64/aarch64-asm.h (PACIASP): Replace SPARC CFI
directive by AArch64 one.
(AUTIASP): Same.

libitm/ChangeLog:

* config/aarch64/sjlj.S: Replace SPARC CFI directive by
AArch64 one.

gcc/testsuite/ChangeLog:

* g++.target/aarch64/pr94515-1.C: Replace SPARC CFI directive by
AArch64 one.
* g++.target/aarch64/pr94515-2.C: Same.
---
 gcc/config/aarch64/aarch64.cc| 32 +
 gcc/config/sparc/sparc.cc| 36 
 gcc/coretypes.h  |  6 
 gcc/doc/tm.texi  | 28 +++
 gcc/doc/tm.texi.in   | 17 +
 gcc/dwarf2cfi.cc | 29 ++--
 gcc/dwarf2out.cc | 13 ---
 gcc/dwarf2out.h  | 10 +++---
 gcc/target.def   | 20 +++
 gcc/testsuite/g++.target/aarch64/pr94515-1.C |  6 ++--
 gcc/testsuite/g++.target/aarch64/pr94515-2.C |  2 +-
 libffi/include/ffi_cfi.h |  2 ++
 libgcc/config/aarch64/aarch64-asm.h  |  4 +--
 libitm/config/aarch64/sjlj.S | 10 +++---
 14 files changed, 176 insertions(+), 39 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 0af5d85c36f..1f87779f40a 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -59,6 +59,7 @@
 #include "opts.h"
 #include "gimplify.h"
 #include "dwarf2.h"
+#include "dwarf2out.h"
 #include "gimple-iterator.h"
 #include "tree-vectorizer.h"
 #include "aarch64-cost-tables.h"
@@ -1449,6 +1450,31 @@ aarch64_dwarf_frame_reg_mode (int regno)
   return default_dwarf_frame_reg_mode (regno);
 }
 
+/* Implement TARGET_OUTPU

[PATCH v1 4/4] dwarf2: store the RA state in CFI row

[Matthieu Longo]

On AArch64, the RA state informs the unwinder whether the return address
is mangled and how, or not. This information is encoded in a boolean in
the CFI row. This binary approach prevents from expressing more complex
configuration, as it is the case with PAuth_LR introduced in Armv9.5-A.

This patch addresses this limitation by replacing the boolean by an enum.

gcc/ChangeLog:

* dwarf2cfi.cc
(struct dw_cfi_row): Declare a new enum type to replace ra_mangled.
(cfi_row_equal_p): Use ra_state instead of ra_mangled.
(dwarf2out_frame_debug_cfa_negate_ra_state): Same.
(change_cfi_row): Same.
---
 gcc/dwarf2cfi.cc | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/gcc/dwarf2cfi.cc b/gcc/dwarf2cfi.cc
index 6c80e0b17bd..023f61fb712 100644
--- a/gcc/dwarf2cfi.cc
+++ b/gcc/dwarf2cfi.cc
@@ -57,6 +57,15 @@ along with GCC; see the file COPYING3.  If not see
 #define DEFAULT_INCOMING_FRAME_SP_OFFSET INCOMING_FRAME_SP_OFFSET
 #endif
 
+
+/* Signing method used for return address authentication.
+   (AArch64 extension)*/
+typedef enum
+{
+  RA_no_signing = 0x0,
+  RA_signing_SP = 0x1,
+} RA_signing_method_t;
+
 /* A collected description of an entire row of the abstract CFI table.  */
 struct GTY(()) dw_cfi_row
 {
@@ -74,8 +83,8 @@ struct GTY(()) dw_cfi_row
   bool window_save;
 
   /* Aarch64 extension for DW_CFA_AARCH64_negate_ra_state.
- True if the return address is in a mangled state.  */
-  bool ra_mangled;
+ Enum which stores the return address state.  */
+  RA_signing_method_t ra_state;
 };
 
 /* The caller's ORIG_REG is saved in SAVED_IN_REG.  */
@@ -859,7 +868,7 @@ cfi_row_equal_p (dw_cfi_row *a, dw_cfi_row *b)
   if (a->window_save != b->window_save)
 return false;
 
-  if (a->ra_mangled != b->ra_mangled)
+  if (a->ra_state != b->ra_state)
 return false;
 
   return true;
@@ -1556,8 +1565,11 @@ dwarf2out_frame_debug_cfa_negate_ra_state (void)
 {
   dw_cfi_ref cfi = new_cfi ();
   cfi->dw_cfi_opc = DW_CFA_AARCH64_negate_ra_state;
+  cur_row->ra_state =
+ (cur_row->ra_state == RA_no_signing)
+ ? RA_signing_SP
+ : RA_no_signing;
   add_cfi (cfi);
-  cur_row->ra_mangled = !cur_row->ra_mangled;
 }
 
 /* Record call frame debugging information for an expression EXPR,
@@ -2414,12 +2426,12 @@ change_cfi_row (dw_cfi_row *old_row, dw_cfi_row 
*new_row)
 {
   dw_cfi_ref cfi = new_cfi ();
 
-  gcc_assert (!old_row->ra_mangled && !new_row->ra_mangled);
+  gcc_assert (!old_row->ra_state && !new_row->ra_state);
   cfi->dw_cfi_opc = DW_CFA_GNU_window_save;
   add_cfi (cfi);
 }
 
-  if (old_row->ra_mangled != new_row->ra_mangled)
+  if (old_row->ra_state != new_row->ra_state)
 {
   dw_cfi_ref cfi = new_cfi ();
 
-- 
2.46.0

Re: [PATCH v1 3/4] aarch64: improve assembly debug comments for build attributes

[Matthieu Longo]

On 2024-10-08 18:45, Richard Sandiford wrote:

Matthieu Longo  writes:

The previous implementation to emit build attributes did not support
string values (asciz) in aeabi_subsection, and was not emitting values
associated to tags in the assembly comments.

This new approach provides a more user-friendly interface relying on
typing, and improves the emitted assembly comments:
   * aeabi_attribute:
 ** Adds the interpreted value next to the tag in the assembly
 comment.
 ** Supports asciz values.
   * aeabi_subsection:
 ** Adds debug information for its parameters.
 ** Auto-detects the attribute types when declaring the subsection.

Additionally, it is also interesting to note that the code was moved
to a separate file to improve modularity and "releave" the 1000-lines
long aarch64.cc file from a few lines. Finally, it introduces a new
namespace "aarch64::" for AArch64 backend which reduce the length of
function names by not prepending 'aarch64_' to each of them.

gcc/ChangeLog:

* config/aarch64/aarch64.cc
(aarch64_emit_aeabi_attribute): Delete.
(aarch64_emit_aeabi_subsection): Delete.
(aarch64_start_file): Use aeabi_subsection.
* config/aarch64/aarch64-dwarf-metadata.h: New file.

gcc/testsuite/ChangeLog:

* gcc.target/aarch64/build-attributes/build-attribute-gcs.c:
  Improve test to match debugging comments in assembly.
* gcc.target/aarch64/build-attributes/build-attribute-standard.c:
  Likewise.


Generally this looks really nice!  Just some minor comments below.
Most of them are formatting nits; sorry that we don't have automatic
formatting to care of this stuff.


All the formatting comments are addressed in the next revision.


---
  gcc/config/aarch64/aarch64-dwarf-metadata.h   | 247 ++
  gcc/config/aarch64/aarch64.cc |  43 +--
  .../build-attributes/build-attribute-gcs.c|   4 +-
  .../build-attribute-standard.c|   4 +-
  4 files changed, 261 insertions(+), 37 deletions(-)
  create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.h

diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.h 
b/gcc/config/aarch64/aarch64-dwarf-metadata.h
new file mode 100644
index 000..9638bc7702f
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-dwarf-metadata.h
@@ -0,0 +1,247 @@
+/* Machine description for AArch64 architecture.
+   Copyright (C) 2009-2024 Free Software Foundation, Inc.
+   Contributed by ARM Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_AARCH64_DWARF_METADATA_H
+#define GCC_AARCH64_DWARF_METADATA_H
+
+#include 
+#include 


As mentioned in the part 1 review, we need to include system headers
via system.h.  These two should be included by default, though,
so I think they can just be removed.



Fixed.


+#include 


"vec.h"



Fixed.


+
+namespace aarch64 {
+
+enum attr_val_type: uint8_t
+{
+  uleb128 = 0x0,
+  asciz = 0x1,
+};
+
+enum BA_TagFeature_t: uint8_t
+{
+  Tag_Feature_BTI = 1,
+  Tag_Feature_PAC = 2,
+  Tag_Feature_GCS = 3,
+};
+
+template 
+struct aeabi_attribute
+{
+  T_tag tag;
+  T_val value;
+};
+
+template 
+aeabi_attribute
+make_aeabi_attribute (T_tag tag, T_val val)
+{
+  return aeabi_attribute{tag, val};
+}
+
+namespace details {
+
+constexpr const char*


Formatting nit, but: space before "*", even here.  Same below.



Fixed.


+to_c_str (bool b)
+{
+  return b ? "true" : "false";
+}
+
+constexpr const char*
+to_c_str (const char *s)
+{
+  return s;
+}
+
+constexpr const char*
+to_c_str (attr_val_type t)
+{
+  const char *s = nullptr;
+  switch (t) {
+case uleb128:
+  s = "ULEB128";
+  break;
+case asciz:
+  s = "asciz";
+  break;
+  }


Formatting nit: the opening brace should be on its own line, indented like so:

   switch (t)
 {
 case uleb128:
   s = "ULEB128";
   break;
 case asciz:
   s = "asciz";
   break;
 }

However...


+  return s;


...we are unfortunately limited to C++11 constexprs, so I think this needs
to be:

   return (t == uleb128 ? "ULEB128"
   : t == asciz ? "asciz"
   : nullptr);

if we want it to be treated as a constant for all builds.



Fixed in the next revision.
FYI we might have C

Re: [PATCH v1 4/4] aarch64: encapsulate note.gnu.property emission into a class

[Matthieu Longo]

On 2024-10-08 18:51, Richard Sandiford wrote:

Matthieu Longo  writes:

gcc/ChangeLog:

 * config.gcc: Add aarch64-dwarf-metadata.o to extra_objs.
 * config/aarch64/aarch64-dwarf-metadata.h (class 
section_note_gnu_property):
 Encapsulate GNU properties code into a class.
 * config/aarch64/aarch64.cc
 (GNU_PROPERTY_AARCH64_FEATURE_1_AND): Define.
 (GNU_PROPERTY_AARCH64_FEATURE_1_BTI): Likewise.
 (GNU_PROPERTY_AARCH64_FEATURE_1_PAC): Likewise.
 (GNU_PROPERTY_AARCH64_FEATURE_1_GCS): Likewise.
 (aarch64_file_end_indicate_exec_stack): Move GNU properties code to
 aarch64-dwarf-metadata.cc
 * config/aarch64/t-aarch64: Declare target aarch64-dwarf-metadata.o
 * config/aarch64/aarch64-dwarf-metadata.cc: New file.
---
  gcc/config.gcc   |   2 +-
  gcc/config/aarch64/aarch64-dwarf-metadata.cc | 120 +++
  gcc/config/aarch64/aarch64-dwarf-metadata.h  |  19 +++
  gcc/config/aarch64/aarch64.cc|  87 +-
  gcc/config/aarch64/t-aarch64 |   7 ++
  5 files changed, 153 insertions(+), 82 deletions(-)
  create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.cc

diff --git a/gcc/config.gcc b/gcc/config.gcc
index f09ce9f63a0..b448c2a91d1 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -351,7 +351,7 @@ aarch64*-*-*)
c_target_objs="aarch64-c.o"
cxx_target_objs="aarch64-c.o"
d_target_objs="aarch64-d.o"
-   extra_objs="aarch64-builtins.o aarch-common.o aarch64-sve-builtins.o 
aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o aarch64-sve-builtins-sve2.o 
aarch64-sve-builtins-sme.o cortex-a57-fma-steering.o aarch64-speculation.o 
falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o 
aarch64-early-ra.o aarch64-ldp-fusion.o"
+   extra_objs="aarch64-builtins.o aarch-common.o aarch64-dwarf-metadata.o 
aarch64-sve-builtins.o aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o 
aarch64-sve-builtins-sve2.o aarch64-sve-builtins-sme.o cortex-a57-fma-steering.o 
aarch64-speculation.o falkor-tag-collision-avoidance.o aarch-bti-insert.o 
aarch64-cc-fusion.o aarch64-early-ra.o aarch64-ldp-fusion.o"
target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.h 
\$(srcdir)/config/aarch64/aarch64-builtins.cc 
\$(srcdir)/config/aarch64/aarch64-sve-builtins.h 
\$(srcdir)/config/aarch64/aarch64-sve-builtins.cc"
target_has_targetm_common=yes
;;
diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.cc 
b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
new file mode 100644
index 000..36659862b59
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
@@ -0,0 +1,120 @@


Missing copyright & licence.



Oups :/ Fixed.


+#define INCLUDE_STRING
+#define INCLUDE_ALGORITHM
+#define INCLUDE_MEMORY
+#define INCLUDE_VECTOR
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "target.h"
+#include "rtl.h"
+#include "output.h"
+
+#include "aarch64-dwarf-metadata.h"
+
[...]
diff --git a/gcc/config/aarch64/t-aarch64 b/gcc/config/aarch64/t-aarch64
index c2a0715e9ab..194e3a4ac99 100644
--- a/gcc/config/aarch64/t-aarch64
+++ b/gcc/config/aarch64/t-aarch64
@@ -139,6 +139,13 @@ aarch-common.o: $(srcdir)/config/arm/aarch-common.cc 
$(CONFIG_H) $(SYSTEM_H) \
$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \
$(srcdir)/config/arm/aarch-common.cc
  
+aarch64-dwarf-metadata.o: $(srcdir)/config/aarch64/aarch64-dwarf-metadata.cc \

+$(CONFIG_H) \
+$(SYSTEM_H) \
+$(TARGET_H)


Looks like this also needs: $(RTL_H) output.h aarch64-dwarf-metadata.h


Fixed.


The comments for patch 1 apply to this refactored version too, but otherwise
it looks good.


Fixed in both patches 1/4 and 4/4 in the next revision.


Thanks,
Richard



+   $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_SPPFLAGS) $(INCLUDES) \
+ $(srcdir)/config/aarch64/aarch64-dwarf-metadata.cc
+
  aarch64-c.o: $(srcdir)/config/aarch64/aarch64-c.cc $(CONFIG_H) $(SYSTEM_H) \
  coretypes.h $(TM_H) $(TREE_H) output.h $(C_COMMON_H) $(TARGET_H)
$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) \

Re: [PATCH v1 1/4] aarch64: add debug comments to feature properties in .note.gnu.property

[Matthieu Longo]

On 2024-10-08 18:39, Richard Sandiford wrote:

Sorry for the slow review.

Matthieu Longo  writes:

GNU properties are emitted to provide some information about the features
used in the generated code like PAC, BTI, or GCS. However, no debug
comment are emitted in the generated assembly even if -dA is provided.
This makes understanding the information stored in the .note.gnu.property
section more difficult than needed.

This patch adds assembly comments (if -dA is provided) next to the GNU
properties. For instance, if PAC and BTI are enabled, it will emit:
   .word  3  // GNU_PROPERTY_AARCH64_FEATURE_1_AND (BTI, PAC)

gcc/ChangeLog:

 * config/aarch64/aarch64.cc
 (aarch64_file_end_indicate_exec_stack): Emit assembly comments.

gcc/testsuite/ChangeLog:

 * gcc.target/aarch64/bti-1.c: Emit assembly comments, and update
   test assertion.
---
  gcc/config/aarch64/aarch64.cc| 41 +++-
  gcc/testsuite/gcc.target/aarch64/bti-1.c |  5 +--
  2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 4b2e7a690c6..6d9075011ec 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -98,6 +98,8 @@
  #include "ipa-fnsummary.h"
  #include "hash-map.h"
  
+#include 

+


If we do keep this, it needs to be done via a new INCLUDE_NUMERIC
in system.h, which aarch64.cc would then define before including
any files.  This avoids clashes between GCC and system headers
on some hosts.  But see below.


I removed it as I removed std::accumulate.




  /* This file should be included last.  */
  #include "target-def.h"
  
@@ -29129,8 +29131,45 @@ aarch64_file_end_indicate_exec_stack ()

 data   = feature_1_and.  */
assemble_integer (GEN_INT (GNU_PROPERTY_AARCH64_FEATURE_1_AND), 4, 32, 
1);
assemble_integer (GEN_INT (4), 4, 32, 1);
-  assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
  
+  if (!flag_debug_asm)

+   assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
+  else
+   {
+ asm_fprintf (asm_out_file, "\t.word\t%u", feature_1_and);


It's probably better to use integer_asm_op (4, 1) rather than
hard-coding .word.


Done.


+
+ auto join_s = [] (std::string s1,
+   const std::string &s2,
+   const std::string &separator = ", ") -> std::string
+ {
+   return std::move (s1)
+ .append (separator)
+ .append (s2);
+ };
+
+ auto features_to_string
+   = [&join_s] (unsigned feature_1_and) -> std::string
+ {
+   std::vector feature_bits;
+   if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)
+ feature_bits.push_back ("BTI");
+   if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_PAC)
+ feature_bits.push_back ("PAC");
+   if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_GCS)
+ feature_bits.push_back ("GCS");
+
+   if (feature_bits.empty ())
+ return {};
+   return std::accumulate(std::next(feature_bits.cbegin()),
+  feature_bits.cend(),
+  feature_bits[0],
+  join_s);
+ };
+ auto const& s_features = features_to_string (feature_1_and);


I do like this!  But I wonder whether it would be simpler to go for
the more prosaic:

  struct flag_name { unsigned int mask; const char *name; };
  static const flag_name flags[] =
  {
{ GNU_PROPERTY_AARCH64_FEATURE_1_BTI, "BTI" },
{ GNU_PROPERTY_AARCH64_FEATURE_1_PAC, "PAC" }
  };

  const char *separator = "";
  std::string s_features;
  for (auto &flag : flags)
if (feature_1_and & flag.mask)
  {
s_features.append (separator).append (flag.name);
separator = ", ";
  }

It's slightly shorter, but also means that there's a bit less
cut-&-paste for each flag.  (In principle, the table could even
be generated from the same source as the definitions of the
GNU_PROPERTY_*s, but that's probaby overkill.)


Replaced by your proposition.


+ asm_fprintf (asm_out_file,
+   "\t%s GNU_PROPERTY_AARCH64_FEATURE_1_AND (%s)\n",
+   ASM_COMMENT_START, s_features.c_str ());
+   }


Formatting:

  asm_fprintf (asm_out_file,
   "\t%s GNU_PROPERTY_AARCH64_FEATURE_1_AND (%s)\n",
   ASM_COMMENT_START, s_features.c_str ());

Thanks,
Richard


/* Pad the size of the note to the required alignment.  */
assemble_align (POINTER_SIZE);
  }
diff --

Re: [PATCH v1 2/4] aarch64: add minimal support for GCS build attributes

[Matthieu Longo]

On 2024-10-08 18:42, Richard Sandiford wrote:

Since this is an RFC, it would probably be more helpful to get review
comments about the design or the adherence to the spec.  I'll need to
look into things a bit more for that, though, so I'm afraid the below
is more implementation trivia.

Matthieu Longo  writes:

[...]
diff --git a/gcc/config.in b/gcc/config.in
index 7fcabbe5061..1309ba2b133 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -379,6 +379,12 @@
  #endif
  
  
+/* Define if your assembler supports GCS build attributes. */

+#ifndef USED_FOR_TARGET
+#undef HAVE_AS_BUILD_ATTRIBUTES_GCS
+#endif


How about making this HAVE_AS_BUILD_AEABI_ATTRIBUTE?  Or is the idea
that each feature that uses build attributes would need its own
configure check?


The binutils interface is agnostic of the tag value. binutils only 
checks whether the values in this object are matching in the other 
object, that's all.

Checking for the support of each feature in Gas does not make sense.
As you recommended, I defined and used HAVE_AS_BUILD_AEABI_ATTRIBUTE in 
the next revision.



[...]
@@ -24697,6 +24724,20 @@ aarch64_start_file (void)
 asm_fprintf (asm_out_file, "\t.arch %s\n",
aarch64_last_printed_arch_string.c_str ());
  
+/* Emit gcs build attributes only when building a native AArch64-hosted

+   compiler.  */
+#if defined(__aarch64__)


Why is this restricted to native compilers?  We should avoid that
if at all possible.


This check comes from the initial revision 
(https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662825.html).
I am not sure why there was this restriction, it does not make sense to 
me as well.

I removed it in the next revision.


+  /* Check the current assembly supports gcs build attributes, if not


"Check whether the current assembler supports..."


Fixed.


+ fallback to .note.gnu.property section.  */
+  #ifdef HAVE_AS_BUILD_ATTRIBUTES_GCS


It would be good to add:

#ifndef HAVE_AS_BUILD_ATTRIBUTES_GCS
#define HAVE_AS_BUILD_ATTRIBUTES_GCS 0
#endif

somewhere, so that we can use HAVE_AS_BUILD_ATTRIBUTES_GCS in C++
conditions as well as preprocessor conditions.


Done. I added it at the top of aarch64.cc in the next revision.


+if (aarch64_gcs_enabled ())
+  {
+   aarch64_emit_aeabi_subsection (".aeabi-feature-and-bits", 1, 0);
+   aarch64_emit_aeabi_attribute ("Tag_Feature_GCS", 3, 1);
+  }
+  #endif
+#endif
+
 default_file_start ();
  }
[...]
diff --git 
a/gcc/testsuite/gcc.target/aarch64/build-attributes/build-attributes.exp 
b/gcc/testsuite/gcc.target/aarch64/build-attributes/build-attributes.exp
new file mode 100644
index 000..ea47e209227
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/build-attributes/build-attributes.exp
@@ -0,0 +1,46 @@
+# Copyright (C) 2024 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# <http://www.gnu.org/licenses/>.
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# Exit immediately if this isn't an AArch64 target.
+if ![istarget aarch64*-*-*] then {
+  return
+}
+
+# Load support procs.
+load_lib gcc-dg.exp
+
+# Initialize `dg'.
+dg-init
+
+proc check_effective_target_gas_has_build_attributes { } {
+return [check_no_compiler_messages gas_has_build_attributes object {
+   /* Assembly */
+   .set ATTR_TYPE_uleb128,   0
+   .set ATTR_TYPE_asciz, 1
+   .set Tag_Feature_GCS, 3
+   .aeabi_subsection .aeabi-feature-and-bits, 1, ATTR_TYPE_uleb128
+   .aeabi_attribute Tag_Feature_GCS, 1
+}]
+}


This should go in lib/target-supports.exp instead, probably as
"aarch64_gas_has...".


I renamed the function to 
check_effective_target_aarch64_gas_has_build_attributes to make it clear 
that this is for AArch64 targets, and moved the code into 
lib/target-supports.exp.



+
+# Main loop.
+dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]] \
+   "" ""
+
+# All done.
+dg-finish
diff --git 
a/gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c 
b/gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c
new file mode 100644
index 000..a8712d949f0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c
@@ -0,0 +1,12 @@
+/* { dg-do compile { target { a

[PATCH v2 0/4] aarch64: add minimal support of AEABI build attributes for GCS

[Matthieu Longo]

The primary focus of this patch series is to add support for build attributes 
in the context of GCS (Guarded Control Stack, an Armv9.4-a extension) to the 
AArch64 backend.
It addresses comments from revision 1 [2] and 2 [3], and proposes a different 
approach compared to the previous implementation of the build attributes.

The series is composed of the following 4 patches:
1. Patch adding assembly debug comments (-dA) to the existing GNU properties, 
to improve testing and check the correctness of values.
2. The minimal patch adding support for build attributes in the context of GCS.
3. A refactoring of (2) to make things less error-prone and more modular, add 
support for asciz attributes and more debug information.
4. A refactoring of (1) relying partly on (3).
The targeted final state of this series would consist in squashing (2) + (3), 
and (1) + (4).

**Special note regarding (2):** If Gas has support for build attributes, both 
build attributes and GNU properties will be emitted. This behavior is still 
open for discussion. Please, let me know your thoughts regarding this behavior.

This patch series needs to be applied on top of the patch series for GCS [1].

Bootstrapped on aarch64-none-linux-gnu, and no regression found.

[1]: https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/vendors/ARM/heads/gcs
[2]: https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662825.html
[3]: https://gcc.gnu.org/pipermail/gcc-patches/2024-September/664004.html

Regards,
Matthieu

Diff with revision 1 [2]:
- update the description of (2)
- address the comments related to the tests in (2)
- add new commits (1), (3) and (4)

Diff with revision 2 [3]:
- address comments of Richard Sandiford in revision 2.
- fix several formatting mistakes.
- remove RFC tag.


Matthieu Longo (3):
  aarch64: add debug comments to feature properties in .note.gnu.property
  aarch64: improve assembly debug comments for AEABI build attributes
  aarch64: encapsulate note.gnu.property emission into a class

Srinath Parvathaneni (1):
  aarch64: add minimal support of AEABI build attributes for GCS.

 gcc/config.gcc|   2 +-
 gcc/config.in |   6 +
 gcc/config/aarch64/aarch64-dwarf-metadata.cc  | 145 +++
 gcc/config/aarch64/aarch64-dwarf-metadata.h   | 245 ++
 gcc/config/aarch64/aarch64.cc |  69 ++---
 gcc/config/aarch64/t-aarch64  |  10 +
 gcc/configure |  38 +++
 gcc/configure.ac  |  10 +
 gcc/testsuite/gcc.target/aarch64/bti-1.c  |  13 +-
 .../aarch64-build-attributes.exp  |  35 +++
 .../build-attributes/build-attribute-gcs.c|  12 +
 .../build-attribute-standard.c|  12 +
 .../build-attributes/no-build-attribute-bti.c |  12 +
 .../build-attributes/no-build-attribute-gcs.c |  12 +
 .../build-attributes/no-build-attribute-pac.c |  12 +
 .../no-build-attribute-standard.c |  12 +
 gcc/testsuite/lib/target-supports.exp |  16 ++
 17 files changed, 611 insertions(+), 50 deletions(-)
 create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.cc
 create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.h
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/aarch64-build-attributes.exp
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attribute-gcs.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attribute-standard.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-gcs.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-pac.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-standard.c

-- 
2.47.0

[PATCH v2 2/4] aarch64: add minimal support of AEABI build attributes for GCS.

[Matthieu Longo]

From: Srinath Parvathaneni 

GCS (Guarded Control Stack, an Armv9.4-a extension) requires some
caution at runtime. The runtime linker needs to reason about the
compatibility of a set of relocable object files that might not have
been compiled with the same compiler.
Up until now, GNU properties are stored in a ELF section (.note.gnu.property)
and have been used for the previously mentioned runtime checks
performed by the linker. However, GNU properties are limited in
their expressibility, and a long-term commmitment was taken in the
ABI for the Arm architecture [1] to provide build attributes.

This patch adds a first support for AArch64 GCS build attributes.
This support includes generating two new assembler directives:
.aeabi_subsection and .aeabi_attribute. These directives are
generated as per the syntax mentioned in spec "Build Attributes for
the Arm® 64-bit Architecture (AArch64)" available at [1].

gcc/configure.ac now includes a new check to test whether the
assembler being used to build the toolchain supports these new
directives.
Two behaviors can be observed when -mbranch-protection=[gcs|standard]
is passed:
- If the assembler support them, the assembly directives are emitted
along the .note.gnu.property section for backward compatibility.
- If the assembler does not support them, only .note.gnu.property
section will contain the relevant information.

This patch needs to be applied on top of GCC's GCS patch series [2].

Bootstrapped on aarch64-none-linux-gnu, and no regression found.

[1]: https://github.com/ARM-software/abi-aa/pull/230
[2]: https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/vendors/ARM/heads/gcs

gcc/ChangeLog:

* config.in: Regenerated
* config/aarch64/aarch64.cc
(HAVE_AS_AEABI_BUILD_ATTRIBUTES): New definition.
(aarch64_emit_aeabi_attribute): New function declaration.
(aarch64_emit_aeabi_subsection): Likewise.
(aarch64_start_file): Emit GCS build attributes.
(aarch64_file_end_indicate_exec_stack): Update GCS bit in
note.gnu.property section.
* configure: Regenerated.
* configure.ac: Add a configure check for gcc.

gcc/testsuite/ChangeLog:

* lib/target-supports.exp:
(check_effective_target_aarch64_gas_has_build_attributes): New.
* gcc.target/aarch64/build-attributes/build-attribute-gcs.c: New test.
* gcc.target/aarch64/build-attributes/build-attribute-standard.c: New 
test.
* gcc.target/aarch64/build-attributes/aarch64-build-attributes.exp: New 
DejaGNU file.
* gcc.target/aarch64/build-attributes/no-build-attribute-bti.c: New 
test.
* gcc.target/aarch64/build-attributes/no-build-attribute-gcs.c: New 
test.
* gcc.target/aarch64/build-attributes/no-build-attribute-pac.c: New 
test.
* gcc.target/aarch64/build-attributes/no-build-attribute-standard.c: 
New test.

Co-Authored-By: Matthieu Longo  
---
 gcc/config.in |  6 +++
 gcc/config/aarch64/aarch64.cc | 41 +++
 gcc/configure | 38 +
 gcc/configure.ac  | 10 +
 .../aarch64-build-attributes.exp  | 35 
 .../build-attributes/build-attribute-gcs.c| 12 ++
 .../build-attribute-standard.c| 12 ++
 .../build-attributes/no-build-attribute-bti.c | 12 ++
 .../build-attributes/no-build-attribute-gcs.c | 12 ++
 .../build-attributes/no-build-attribute-pac.c | 12 ++
 .../no-build-attribute-standard.c | 12 ++
 gcc/testsuite/lib/target-supports.exp | 16 
 12 files changed, 218 insertions(+)
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/aarch64-build-attributes.exp
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attribute-gcs.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attribute-standard.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-gcs.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-pac.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-standard.c

diff --git a/gcc/config.in b/gcc/config.in
index 7fcabbe5061..0d54141ce30 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -355,6 +355,12 @@
 #endif
 
 
+/* Define if your assembler supports AEABI build attributes. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_AS_AEABI_BUILD_ATTRIBUTES
+#endif
+
+
 /* Define if your assembler supports architecture modifiers. */
 #ifndef USED_FOR_TARGET
 #undef HAVE_AS_ARCHITECTURE_MODIFIERS
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 0466d6d11eb..90aba0fff88 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aar

[PATCH v2 4/4] aarch64: encapsulate note.gnu.property emission into a class

[Matthieu Longo]

gcc/ChangeLog:

* config.gcc: Add aarch64-dwarf-metadata.o to extra_objs.
* config/aarch64/aarch64-dwarf-metadata.h
(class section_note_gnu_property): Encapsulate GNU properties code
into a class.
* config/aarch64/aarch64.cc
(GNU_PROPERTY_AARCH64_FEATURE_1_AND): Removed.
(GNU_PROPERTY_AARCH64_FEATURE_1_BTI): Likewise.
(GNU_PROPERTY_AARCH64_FEATURE_1_PAC): Likewise.
(GNU_PROPERTY_AARCH64_FEATURE_1_GCS): Likewise.
(aarch64_file_end_indicate_exec_stack): Move GNU properties code to
aarch64-dwarf-metadata.cc
* config/aarch64/t-aarch64: Declare target aarch64-dwarf-metadata.o
* config/aarch64/aarch64-dwarf-metadata.cc: New file.
---
 gcc/config.gcc   |   2 +-
 gcc/config/aarch64/aarch64-dwarf-metadata.cc | 145 +++
 gcc/config/aarch64/aarch64-dwarf-metadata.h  |  19 +++
 gcc/config/aarch64/aarch64.cc|  79 +-
 gcc/config/aarch64/t-aarch64 |  10 ++
 5 files changed, 181 insertions(+), 74 deletions(-)
 create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.cc

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 71ac3badafd..8d26b8776e3 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -351,7 +351,7 @@ aarch64*-*-*)
c_target_objs="aarch64-c.o"
cxx_target_objs="aarch64-c.o"
d_target_objs="aarch64-d.o"
-   extra_objs="aarch64-builtins.o aarch-common.o aarch64-sve-builtins.o 
aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o 
aarch64-sve-builtins-sve2.o aarch64-sve-builtins-sme.o 
cortex-a57-fma-steering.o aarch64-speculation.o 
falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o 
aarch64-early-ra.o aarch64-ldp-fusion.o"
+   extra_objs="aarch64-builtins.o aarch-common.o aarch64-dwarf-metadata.o 
aarch64-sve-builtins.o aarch64-sve-builtins-shapes.o 
aarch64-sve-builtins-base.o aarch64-sve-builtins-sve2.o 
aarch64-sve-builtins-sme.o cortex-a57-fma-steering.o aarch64-speculation.o 
falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o 
aarch64-early-ra.o aarch64-ldp-fusion.o"
target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.h 
\$(srcdir)/config/aarch64/aarch64-builtins.cc 
\$(srcdir)/config/aarch64/aarch64-sve-builtins.h 
\$(srcdir)/config/aarch64/aarch64-sve-builtins.cc"
target_has_targetm_common=yes
;;
diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.cc 
b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
new file mode 100644
index 000..f272d290fed
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
@@ -0,0 +1,145 @@
+/* DWARF metadata for AArch64 architecture.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   Contributed by ARM Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   .  */
+
+#define INCLUDE_STRING
+#define INCLUDE_ALGORITHM
+#define INCLUDE_MEMORY
+#define INCLUDE_VECTOR
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "target.h"
+#include "rtl.h"
+#include "output.h"
+
+#include "aarch64-dwarf-metadata.h"
+
+/* Defined for convenience.  */
+#define POINTER_BYTES (POINTER_SIZE / BITS_PER_UNIT)
+
+namespace aarch64 {
+
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_AND = 0xc000;
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_BTI = (1U << 0);
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_PAC = (1U << 1);
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_GCS = (1U << 2);
+
+namespace {
+
+std::string
+gnu_property_features_to_string (unsigned feature_1_and)
+{
+  struct flag_name
+  {
+unsigned int mask;
+const char *name;
+  };
+
+  static const flag_name flags[] = {
+{GNU_PROPERTY_AARCH64_FEATURE_1_BTI, "BTI"},
+{GNU_PROPERTY_AARCH64_FEATURE_1_PAC, "PAC"},
+{GNU_PROPERTY_AARCH64_FEATURE_1_GCS, "GCS"},
+  };
+
+  const char *separator = "";
+  std::string s_features;
+  for (auto &flag : flags)
+if (feature_1_and & flag.mask)
+  {
+   s_features.append (separator).append (flag.name);
+   separator = ", ";
+  }
+  return s_features;
+};
+
+} // namespace anonymous
+
+section_note_gnu_property::section_note_gnu_property ()
+  : m_feature_1_and (0) {}
+
+void
+section_note_gnu_property::bti_enabled ()
+{
+  m_feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
+}

[PATCH v2 1/4] aarch64: add debug comments to feature properties in .note.gnu.property

[Matthieu Longo]

GNU properties are emitted to provide some information about the features
used in the generated code like BTI, GCS, or PAC. However, no debug
comment are emitted in the generated assembly even if -dA is provided.
It makes understanding the information stored in the .note.gnu.property
section more difficult than needed.

This patch adds assembly comments (if -dA is provided) next to the GNU
properties. For instance, if BTI and PAC are enabled, it will emit:
  .word  0x3  // GNU_PROPERTY_AARCH64_FEATURE_1_AND (BTI, PAC)

gcc/ChangeLog:

* config/aarch64/aarch64.cc
(aarch64_file_end_indicate_exec_stack): Emit assembly comments.

gcc/testsuite/ChangeLog:

* gcc.target/aarch64/bti-1.c: Emit assembly comments, and update
test assertion.
---
 gcc/config/aarch64/aarch64.cc| 35 ++--
 gcc/testsuite/gcc.target/aarch64/bti-1.c | 13 +
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 914f2902d25..0466d6d11eb 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -29259,10 +29259,41 @@ aarch64_file_end_indicate_exec_stack ()
 type   = GNU_PROPERTY_AARCH64_FEATURE_1_AND
 datasz = 4
 data   = feature_1_and.  */
-  assemble_integer (GEN_INT (GNU_PROPERTY_AARCH64_FEATURE_1_AND), 4, 32, 
1);
+  fputs (integer_asm_op (4, true), asm_out_file);
+  fprint_whex (asm_out_file, GNU_PROPERTY_AARCH64_FEATURE_1_AND);
+  putc ('\n', asm_out_file);
   assemble_integer (GEN_INT (4), 4, 32, 1);
-  assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
 
+  fputs (integer_asm_op (4, true), asm_out_file);
+  fprint_whex (asm_out_file, feature_1_and);
+  if (flag_debug_asm)
+   {
+ struct flag_name
+ {
+   unsigned int mask;
+   const char *name;
+ };
+ static const flag_name flags[] = {
+   { GNU_PROPERTY_AARCH64_FEATURE_1_BTI, "BTI" },
+   { GNU_PROPERTY_AARCH64_FEATURE_1_PAC, "PAC" },
+   { GNU_PROPERTY_AARCH64_FEATURE_1_GCS, "GCS" },
+ };
+
+ const char *separator = "";
+ std::string s_features;
+ for (auto &flag : flags)
+   if (feature_1_and & flag.mask)
+ {
+   s_features.append (separator).append (flag.name);
+   separator = ", ";
+ }
+
+ asm_fprintf (asm_out_file,
+  "\t%s GNU_PROPERTY_AARCH64_FEATURE_1_AND (%s)\n",
+  ASM_COMMENT_START, s_features.c_str ());
+   }
+  else
+   putc ('\n', asm_out_file);
   /* Pad the size of the note to the required alignment.  */
   assemble_align (POINTER_SIZE);
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/bti-1.c 
b/gcc/testsuite/gcc.target/aarch64/bti-1.c
index 5a556b08ed1..53dc2d3cd8b 100644
--- a/gcc/testsuite/gcc.target/aarch64/bti-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/bti-1.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* -Os to create jump table.  */
-/* { dg-options "-Os" } */
+/* { dg-options "-Os -dA" } */
 /* { dg-require-effective-target lp64 } */
 /* If configured with --enable-standard-branch-protection, don't use
command line option.  */
@@ -44,8 +44,8 @@ f_jump_table (int y, int n)
   return (y == 0)? y+1:4;
 }
 /* f_jump_table should have PACIASP and AUTIASP.  */
-/* { dg-final { scan-assembler-times "hint\t25" 1 } } */
-/* { dg-final { scan-assembler-times "hint\t29" 1 } } */
+/* { dg-final { scan-assembler-times "hint\t25 // paciasp" 1 } } */
+/* { dg-final { scan-assembler-times "hint\t29 // autiasp" 1 } } */
 
 int
 f_label_address ()
@@ -59,6 +59,7 @@ lab2:
   addr = &&lab1;
   return 2;
 }
-/* { dg-final { scan-assembler-times "hint\t34" 1 } } */
-/* { dg-final { scan-assembler-times "hint\t36" 12 } } */
-/* { dg-final { scan-assembler ".note.gnu.property" { target *-*-linux* } } } 
*/
+/* { dg-final { scan-assembler-times "hint\t34 // bti c" 1 } } */
+/* { dg-final { scan-assembler-times "hint\t36 // bti j" 12 } } */
+/* { dg-final { scan-assembler "\.section\t\.note\.gnu\.property" { target 
*-*-linux* } } } */
+/* { dg-final { scan-assembler "\.word\t0x7\t\/\/ 
GNU_PROPERTY_AARCH64_FEATURE_1_AND \\(BTI, PAC, GCS\\)" { target *-*-linux* } } 
} */
\ No newline at end of file
-- 
2.47.0

[PATCH v2 3/4] aarch64: improve assembly debug comments for AEABI build attributes

[Matthieu Longo]

The previous implementation to emit AEABI build attributes did not
support string values (asciz) in aeabi_subsection, and was not
emitting values associated to tags in the assembly comments.

This new approach provides a more user-friendly interface relying on
typing, and improves the emitted assembly comments:
  * aeabi_attribute:
** Adds the interpreted value next to the tag in the assembly
comment.
** Supports asciz values.
  * aeabi_subsection:
** Adds debug information for its parameters.
** Auto-detects the attribute types when declaring the subsection.

Additionally, it is also interesting to note that the code was moved
to a separate file to improve modularity and "releave" the 1000-lines
long aarch64.cc file from a few lines. Finally, it introduces a new
namespace "aarch64::" for AArch64 backend which reduce the length of
function names by not prepending 'aarch64_' to each of them.

gcc/ChangeLog:

* config/aarch64/aarch64.cc
(aarch64_emit_aeabi_attribute): Delete.
(aarch64_emit_aeabi_subsection): Delete.
(aarch64_start_file): Use aeabi_subsection.
* config/aarch64/aarch64-dwarf-metadata.h: New file.

gcc/testsuite/ChangeLog:

* gcc.target/aarch64/build-attributes/build-attribute-gcs.c:
Improve test to match debugging comments in assembly.
* gcc.target/aarch64/build-attributes/build-attribute-standard.c:
Likewise.
---
 gcc/config/aarch64/aarch64-dwarf-metadata.h   | 226 ++
 gcc/config/aarch64/aarch64.cc |  48 +---
 .../build-attributes/build-attribute-gcs.c|   4 +-
 .../build-attribute-standard.c|   4 +-
 4 files changed, 243 insertions(+), 39 deletions(-)
 create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.h

diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.h 
b/gcc/config/aarch64/aarch64-dwarf-metadata.h
new file mode 100644
index 000..01f08ad073e
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-dwarf-metadata.h
@@ -0,0 +1,226 @@
+/* DWARF metadata for AArch64 architecture.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   Contributed by ARM Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   .  */
+
+#ifndef GCC_AARCH64_DWARF_METADATA_H
+#define GCC_AARCH64_DWARF_METADATA_H
+
+#include "system.h"
+#include "vec.h"
+
+namespace aarch64 {
+
+enum attr_val_type : uint8_t
+{
+  uleb128 = 0x0,
+  asciz = 0x1,
+};
+
+enum BA_TagFeature_t : uint8_t
+{
+  Tag_Feature_BTI = 1,
+  Tag_Feature_PAC = 2,
+  Tag_Feature_GCS = 3,
+};
+
+template 
+struct aeabi_attribute
+{
+  T_tag tag;
+  T_val value;
+};
+
+template 
+aeabi_attribute
+make_aeabi_attribute (T_tag tag, T_val val)
+{
+  return aeabi_attribute{tag, val};
+}
+
+namespace details {
+
+constexpr const char *
+to_c_str (bool b)
+{
+  return b ? "true" : "false";
+}
+
+constexpr const char *
+to_c_str (const char *s)
+{
+  return s;
+}
+
+constexpr const char *
+to_c_str (attr_val_type t)
+{
+  return (t == uleb128 ? "ULEB128"
+ : t == asciz ? "asciz"
+ : nullptr);
+}
+
+constexpr const char *
+to_c_str (BA_TagFeature_t feature)
+{
+  return (feature == Tag_Feature_BTI ? "Tag_Feature_BTI"
+ : feature == Tag_Feature_PAC ? "Tag_Feature_PAC"
+ : feature == Tag_Feature_GCS ? "Tag_Feature_GCS"
+ : nullptr);
+}
+
+template <
+  typename T,
+  typename = typename std::enable_if::value, T>::type
+>
+constexpr const char *
+aeabi_attr_str_fmt (T phantom __attribute__((unused)))
+{
+  return "\t.aeabi_attribute %u, %u";
+}
+
+constexpr const char *
+aeabi_attr_str_fmt (const char *phantom __attribute__((unused)))
+{
+  return "\t.aeabi_attribute %u, \"%s\"";
+}
+
+template <
+  typename T,
+  typename = typename std::enable_if::value, T>::type
+>
+constexpr uint8_t
+aeabi_attr_val_for_fmt (T value)
+{
+  return static_cast(value);
+}
+
+constexpr const char *
+aeabi_attr_val_for_fmt (const char *s)
+{
+  return s;
+}
+
+template 
+void
+write (FILE *out_file, aeabi_attribute const &attr)
+{
+  asm_fprintf (out_file, aeabi_attr_str_fmt (T_val{}),
+  attr.tag, aeabi_attr_val_for_fmt (attr.value));
+  if (flag_debug_asm)
+asm_fprintf (out_file, "\t%s %s: %s", ASM_COMMENT_START,
+to_c_str (attr.tag), to_c_str (attr.value));
+  asm_fprintf (out_file, "\n");
+}
+
+template <
+  typename T,
+  typename = typename 

Re: [PATCH v2 0/4] aarch64: add minimal support of AEABI build attributes for GCS

[Matthieu Longo]

On 2024-10-23 17:43, Richard Sandiford wrote:

Matthieu Longo  writes:

The primary focus of this patch series is to add support for build attributes 
in the context of GCS (Guarded Control Stack, an Armv9.4-a extension) to the 
AArch64 backend.
It addresses comments from revision 1 [2] and 2 [3], and proposes a different 
approach compared to the previous implementation of the build attributes.

The series is composed of the following 4 patches:
1. Patch adding assembly debug comments (-dA) to the existing GNU properties, 
to improve testing and check the correctness of values.
2. The minimal patch adding support for build attributes in the context of GCS.
3. A refactoring of (2) to make things less error-prone and more modular, add 
support for asciz attributes and more debug information.
4. A refactoring of (1) relying partly on (3).
The targeted final state of this series would consist in squashing (2) + (3), 
and (1) + (4).

**Special note regarding (2):** If Gas has support for build attributes, both 
build attributes and GNU properties will be emitted. This behavior is still 
open for discussion. Please, let me know your thoughts regarding this behavior.


I don't have a strong opinion.  But emitting both seems like the safe
and conservatively correct behaviour, so I think the onus would be
on anyone who wants to drop the old information to make the case
for doing that.


This patch series needs to be applied on top of the patch series for GCS [1].

Bootstrapped on aarch64-none-linux-gnu, and no regression found.

[1]: https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/vendors/ARM/heads/gcs
[2]: https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662825.html
[3]: https://gcc.gnu.org/pipermail/gcc-patches/2024-September/664004.html

Regards,
Matthieu

Diff with revision 1 [2]:
- update the description of (2)
- address the comments related to the tests in (2)
- add new commits (1), (3) and (4)

Diff with revision 2 [3]:
- address comments of Richard Sandiford in revision 2.
- fix several formatting mistakes.
- remove RFC tag.


Matthieu Longo (3):
   aarch64: add debug comments to feature properties in .note.gnu.property
   aarch64: improve assembly debug comments for AEABI build attributes
   aarch64: encapsulate note.gnu.property emission into a class

Srinath Parvathaneni (1):
   aarch64: add minimal support of AEABI build attributes for GCS.


Looks good, thanks.  OK for trunk with the suggested changes for
patches 2 and 3.

Richard



Thanks Richard for the review.
I addressed all the comments as you requested.
I will synchronize with Yury Khrustalev to see when he plans to merge 
GCS patch series for GCC. I would like those changes to be merged for 
stage 1.


Matthieu

Re: [PATCH v1 1/3] aarch64: store signing key and signing method in DWARF _Unwind_FrameState

[Matthieu Longo]

On 2024-08-06 11:06, Richard Sandiford wrote:

Sorry for the slow review.

Matthieu Longo  writes:

This patch is only a refactoring of the existing implementation
of PAuth and returned-address signing. The existing behavior is
preserved.

_Unwind_FrameState already contains several CIE and FDE information
(see the attributes below the comment "The information we care
about from the CIE/FDE" in libgcc/unwind-dw2.h).
The patch aims at moving the information from DWARF CIE (signing
key stored in the augmentation string) and FDE (the used signing
method) into _Unwind_FrameState along the already-stored CIE and
FDE information.
Note: those information have to be saved in frame_state_reg_info
instead of _Unwind_FrameState as they need to be savable by
DW_CFA_remember_state and restorable by DW_CFA_restore_state, that
both rely on the attribute "prev".

Those new information in _Unwind_FrameState simplifies the look-up
of the signing key when the return address is demangled. It also
allows future signing methods to be easily added.

_Unwind_FrameState is not a part of the public API of libunwind,
so the change is backward compatible.

A new architecture-specific handler MD_ARCH_EXTENSION_FRAME_INIT
allows to reset values (if needed) in the frame state and unwind
context before changing the frame state to the caller context.

A new architecture-specific handler MD_ARCH_EXTENSION_CIE_AUG_HANDLER
isolates the architecture-specific augmentation strings in AArch64
backend, and allows others architectures to reuse augmentation
strings that would have clashed with AArch64 DWARF extensions.

aarch64_demangle_return_addr, DW_CFA_AARCH64_negate_ra_state and
DW_CFA_val_expression cases in libgcc/unwind-dw2-execute_cfa.h
were documented to clarify where the value of the RA state register
is stored (FS and CONTEXT respectively).


The abstraction generally looks good.  My main comment is that,
if we are putting the new behaviour behind target macros (which I
agree is a good idea), could we also have a target macro to abstract:


+#if defined(__aarch64__) && !defined (__ILP32__)
+unsigned char signing_key;
+#endif


?  E.g. something like:

#ifdef MD_CFI_STATE
 MD_CFI_STATE md;
#endif

with aarch64 defining:

#define MD_CFI_STATE struct { unsigned char signing_key; }

(Names and organisation are just suggestions.)

It might be good to try running the patch through

   contrig/check_GNU_style.py

since the GCC coding standards are quite picky about formatting and
stylistic issues.  The main ones I spotted were:

- In files that already use block comments, all comments should be
   block comments rather than // comments.  The comments should end
   with ".  " (full stop and two spaces).

- Block comments are formatted as:

 /* Line 1
Line 2.  */

   rather than as:

 /* Line 1
  * Line 2.  */

- Function names are generally all lowrecase (e.g. "ra" rather than "RA").

- In function calls, there should be a space between the function and
   the opening "("

- For pointer types, there should be a space before a "*" (or string of
   "*"s), but no space afterwards.

- "const" qualifiers generally go before the type that they qualify,
   rather than afterwards.

- The line width should be 80 characters or fewer.  (The patch was pretty
   good about this, but there were a couple of long lines).

- In multi-line conditions, the ||s and &&s go at the beginning of lines,
   rather than at the end.  (Same for infix operators in general.)

More detailed comments below.



libgcc/ChangeLog:

   * config/aarch64/aarch64-unwind.h
   (AARCH64_DWARF_RA_STATE_MASK): The mask for RA state register.
   (aarch64_RA_signing_method_t): The diversifiers used to sign a
   function's return address.
   (aarch64_pointer_auth_key): The key used to sign a function's
   return address.
   (aarch64_cie_signed_with_b_key): Deleted as the signing key is
   available now in _Unwind_FrameState.
   (MD_ARCH_EXTENSION_CIE_AUG_HANDLER): New CIE augmentation string
   handler for architecture extensions.
   (MD_ARCH_EXTENSION_FRAME_INIT): New architecture-extension
   initialization routine for DWARF frame state and context before
   execution of DWARF instructions.
   (aarch64_context_RA_state_get): Read RA state register from CONTEXT.
   (aarch64_RA_state_get): Read RA state register from FS.
   (aarch64_RA_state_set): Write RA state register into FS.
   (aarch64_RA_state_toggle): Toggle RA state register in FS.
   (aarch64_cie_aug_handler): Handler AArch64 augmentation strings.
   (aarch64_arch_extension_frame_init): Initialize defaults for the
   signing key (PAUTH_KEY_A), and RA state register (RA_no_signing).
   (aarch64_demangle_return_addr): Rely on the frame registers and
   the signing_key attribute in _Unwind_FrameState.
   * unwind-dw2-execute_cfa.h:
   Use the right alias DW_CFA_AARCH64_negate_ra_state for 

Re: [PATCH v1 2/3] libgcc: hide CIE and FDE data for DWARF architecture extensions behind a handler.

[Matthieu Longo]

On 2024-08-06 11:21, Richard Sandiford wrote:

Matthieu Longo  writes:

This patch provides a new handler MD_ARCH_FRAME_STATE_T to hide an
architecture-specific structure containing CIE and FDE data related
to DWARF architecture extensions.

Hiding the architecture-specific attributes behind a handler has the
following benefits:
1. isolating those data from the generic ones in _Unwind_FrameState
2. avoiding casts to custom types.
3. preserving typing information when debugging with GDB, and so
facilitating their printing.

This approach required to add a new header md-unwind-def.h included at
the top of libgcc/unwind-dw2.h, and redirecting to the corresponding
architecture header via a symbolic link.

An obvious drawback is the increase in complexity with macros, and
headers. It also caused a split of architecture definitions between
md-unwind-def.h (types definitions used in unwind-dw2.h) and
md-unwind.h (local types definitions and handlers implementations).
The naming of md-unwind.h with .h extension is a bit misleading as
the file is only included in the middle of unwind-dw2.c. Changing
this naming would require modification of others backends, which I
prefered to abstain from. Overall the benefits are worth the added
complexity from my perspective.


Sorry, I should have read 2/3 before making the suggestion in the
previous review.  I agree that it makes sense to separate this change
out, given that it involves a new header file.

It'd be good to update the comment in no-unwind.h:

/* Dummy header for targets without a definition of
MD_FALLBACK_FRAME_STATE_FOR.  */


Done



LGTM otherwise, thanks.

On patch 3: IMO it's better to post the regenerated files as part
of the same patch, so that each patch is self-contained.

>
> Richard
>

I did it this way as it makes things easier to rebase, and regenerate 
the files if there is a conflict.
I agree, this patch can be squashed with the previous when merging those 
changes into master.


Matthieu

Re: [PATCH v1 2/4] dwarf2: add hooks for architecture-specific CFIs

[Matthieu Longo]

On 2024-08-13 16:31, Richard Sandiford wrote:

Matthieu Longo  writes:

Architecture-specific CFI directives are currently declared an processed
among others architecture-independent CFI directives in gcc/dwarf2* files.
This approach creates confusion, specifically in the case of DWARF
instructions in the vendor space and using the same instruction code.

Such a clash currently happen between DW_CFA_GNU_window_save (used on
SPARC) and DW_CFA_AARCH64_negate_ra_state (used on AArch64), and both
having the same instruction code 0x2d.
Then AArch64 compilers generates a SPARC CFI directive (.cfi_window_save)
instead of .cfi_negate_ra_state, contrarilly to what is expected in
[DWARF for the Arm 64-bit Architecture (AArch64)](https://github.com/
ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst).

This refactoring does not solve completely the problem, but improve the
situation by moving some of the processing of those directives (more
specifically their output in the assembly) to the backend via 2 target
hooks:
- DW_CFI_OPRND1_DESC: parse the first operand of the directive (if any).
- OUTPUT_CFI_DIRECTIVE: output the CFI directive as a string.

Additionally, this patch also contains a renaming of an enum used for
return address mangling on AArch64.

gcc/ChangeLog:

 * config/aarch64/aarch64.cc
 (aarch64_output_cfi_directive): New hook for CFI directives.
 (aarch64_dw_cfi_oprnd1_desc): Same.
 (TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive.
 (TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc.
 * config/sparc/sparc.cc
 (sparc_output_cfi_directive): New hook for CFI directives.
 (sparc_dw_cfi_oprnd1_desc): Same.
 (TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive.
 (TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc.
 * coretypes.h
 (struct dw_cfi_node): Forward declaration of CFI type from
 gcc/dwarf2out.h.
 (enum dw_cfi_oprnd_type): Same.
 * doc/tm.texi: Regenerated from doc/tm.texi.in.
 * doc/tm.texi.in: Add doc for OUTPUT_CFI_DIRECTIVE and
 DW_CFI_OPRND1_DESC.
 * dwarf2cfi.cc
 (struct dw_cfi_row): Update the description for window_save
 and ra_mangled.
 (cfi_equal_p): Adapt parameter of dw_cfi_oprnd1_desc.
 (dwarf2out_frame_debug_cfa_negate_ra_state): Use AArch64 CFI
 directive instead of the SPARC one.
 (change_cfi_row): Use the right CFI directive's name for RA
 mangling.
 (output_cfi): Remove explicit architecture-specific CFI
 directive DW_CFA_GNU_window_save that falls into default case.
 (output_cfi_directive): Use target hook as default.
 * dwarf2out.cc (dw_cfi_oprnd1_desc): Use target hook as default.
 * dwarf2out.h (enum dw_cfi_oprnd_type): specify underlying type
 of enum to allow forward declaration.
 (dw_cfi_oprnd1_desc): Change type of parameter.
 (output_cfi_directive): Use dw_cfi_ref instead of struct
 dw_cfi_node *.
 * target.def: Documentation for new hooks.

libffi/ChangeLog:

 * include/ffi_cfi.h (cfi_negate_ra_state): Declare AArch64 cfi
 directive.

libgcc/ChangeLog:

 * config/aarch64/aarch64-asm.h (PACIASP): Replace SPARC CFI
 directive by AArch64 one.
 (AUTIASP): Same.

libitm/ChangeLog:

 * config/aarch64/sjlj.S: Replace SPARC CFI directive by
 AArch64 one.

gcc/testsuite/ChangeLog:

 * g++.target/aarch64/pr94515-1.C: Replace SPARC CFI directive by
 AArch64 one.
 * g++.target/aarch64/pr94515-2.C: Same.
---
  gcc/config/aarch64/aarch64.cc| 32 +
  gcc/config/sparc/sparc.cc| 36 
  gcc/coretypes.h  |  6 
  gcc/doc/tm.texi  | 28 +++
  gcc/doc/tm.texi.in   | 17 +
  gcc/dwarf2cfi.cc | 29 ++--
  gcc/dwarf2out.cc | 13 ---
  gcc/dwarf2out.h  | 10 +++---
  gcc/target.def   | 20 +++
  gcc/testsuite/g++.target/aarch64/pr94515-1.C |  6 ++--
  gcc/testsuite/g++.target/aarch64/pr94515-2.C |  2 +-
  libffi/include/ffi_cfi.h |  2 ++
  libgcc/config/aarch64/aarch64-asm.h  |  4 +--
  libitm/config/aarch64/sjlj.S | 10 +++---
  14 files changed, 176 insertions(+), 39 deletions(-)


Generally looks good to me.  Most of the comments below are very minor,
but there's also a question about the hook interface.



[...]
@@ -12621,6 +12630,33 @@ sparc_output_dwarf_dtprel (FILE *file, int size, rtx x)
fputs (")", file);
  }
  
+/* This is called from gcc/dwarf2cfi.cc via TARGET_OUTPUT_CFI_DIRECTIVE.

+   It outputs SPARC-specific CFI directives (if an

Re: [PATCH v1 3/4] aarch64 testsuite: explain expectections for pr94515* tests

[Matthieu Longo]

On 2024-08-13 16:53, Richard Sandiford wrote:

Matthieu Longo  writes:

gcc/testsuite/ChangeLog:

 * g++.target/aarch64/pr94515-1.C: Improve test documentation.
 * g++.target/aarch64/pr94515-2.C: Same.


The patch is OK as-is, since it's clearly a strict improvement over
the status quo.  But a suggestion below:


---
  gcc/testsuite/g++.target/aarch64/pr94515-1.C |  8 ++
  gcc/testsuite/g++.target/aarch64/pr94515-2.C | 28 +++-
  2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-2.C 
b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
index f4abeed..af6b128b8fd 100644
--- a/gcc/testsuite/g++.target/aarch64/pr94515-2.C
+++ b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
@@ -6,6 +6,7 @@
  volatile int zero = 0;
  int global = 0;
  
+/* This is a leaf function, so no .cfi_negate_ra_state directive is expected.  */

  __attribute__((noinline))
  int bar(void)
  {
@@ -13,29 +14,44 @@ int bar(void)
return 0;
  }
  
+/* This function does not return normally, so the address is signed but no

+ * authentication code is emitted. It means that only one CFI directive is
+ * supposed to be emitted at signing time.  */
  __attribute__((noinline, noreturn))
  void unwind (void)
  {
throw 42;
  }
  
+/* This function has several return instructions, and alternates different RA

+ * states. 4 .cfi_negate_ra_state and a .cfi_remember_state/.cfi_restore_state
+ * should be emitted.
+ */
  __attribute__((noinline, noipa))
  int test(int x)
  {
-  if (x==1) return 2; /* This return path may not use the stack.  */
+  // This return path may not use the stack. This means that the return address
+  // won't be signed.
+  if (x==1) return 2;
+
+  // All the return paths of the code below must have RA mangle state set, and
+  // the return address must be signed.
int y = bar();
if (y > global) global=y;
-  if (y==3) unwind(); /* This return path must have RA mangle state set.  */
-  return 0;
+  if (y==3) unwind(); // authentication of the return address is not required.
+  return 0; // authentication of the return address is required.
  }


I wasn't sure from reading this why it would give 4 .cfi_negate_ra_states,
and had to run the test to find out.  I think a key part is that the
current block layout is:

   A: path to return 0 without assignment to global
   B: global=y + branch back into A
   C: return 2
   D: unwind

so we have:

   A: sign -> authenticate [2 negate_ra_states + remember_state for B]
   B: signed [restore_state]
   C: unsigned [negate_ra_state]
   D: signed [negate_ra_state]

If the order had been ABDC then we would only need 3 negate_ra_states.
If the x==1 branch had been:

 cmp w0, 1
 bne .L10
 mov w0, 2
 ret
.L10:

followed by the rest of A, B and D, then we would only have needed
2 negate_ra_states.

So it might be helpful to give the block layout above too.
Like I say, it's just a suggestion though.

Thanks,
Richard


Done in the next revision.

Thanks,
Matthieu




+/* This function requires only 2 .cfi_negate_ra_state.  */
  int main ()
  {
+  // paciasp -> cfi_negate_ra_state: RA_no_signing -> RA_signing_SP
try {
  test (zero);
-__builtin_abort ();
+__builtin_abort (); // authentication of the return address is not 
required.
} catch (...) {
+// autiasp -> cfi_negate_ra_state: RA_signing_SP -> RA_no_signing
  return 0;
}
-  __builtin_abort ();
-}
+  __builtin_abort (); // authentication of the return address is not required.
+}
\ No newline at end of file

Re: [PATCH v1 4/4] dwarf2: store the RA state in CFI row

[Matthieu Longo]

On 2024-08-06 15:28, Jakub Jelinek wrote:

On Tue, Aug 06, 2024 at 03:07:44PM +0100, Matthieu Longo wrote:

On AArch64, the RA state informs the unwinder whether the return address
is mangled and how, or not. This information is encoded in a boolean in
the CFI row. This binary approach prevents from expressing more complex
configuration, as it is the case with PAuth_LR introduced in Armv9.5-A.

This patch addresses this limitation by replacing the boolean by an enum.


Formatting nits.


gcc/ChangeLog:

 * dwarf2cfi.cc
 (struct dw_cfi_row): Declare a new enum type to replace ra_mangled.
 (cfi_row_equal_p): Use ra_state instead of ra_mangled.
 (dwarf2out_frame_debug_cfa_negate_ra_state): Same.
 (change_cfi_row): Same.
---
  gcc/dwarf2cfi.cc | 24 ++--
  1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/gcc/dwarf2cfi.cc b/gcc/dwarf2cfi.cc
index 6c80e0b17bd..023f61fb712 100644
--- a/gcc/dwarf2cfi.cc
+++ b/gcc/dwarf2cfi.cc
@@ -57,6 +57,15 @@ along with GCC; see the file COPYING3.  If not see
  #define DEFAULT_INCOMING_FRAME_SP_OFFSET INCOMING_FRAME_SP_OFFSET
  #endif
  
+
+/* Signing method used for return address authentication.
+   (AArch64 extension)*/


Dot and two spaces missing before */


+typedef enum
+{
+  RA_no_signing = 0x0,
+  RA_signing_SP = 0x1,
+} RA_signing_method_t;


I think it should be ra_* and *_sp

@@ -1556,8 +1565,11 @@ dwarf2out_frame_debug_cfa_negate_ra_state (void)
  {
dw_cfi_ref cfi = new_cfi ();
cfi->dw_cfi_opc = DW_CFA_AARCH64_negate_ra_state;
+  cur_row->ra_state =
+ (cur_row->ra_state == RA_no_signing)
+ ? RA_signing_SP
+ : RA_no_signing;


This is wrongly formatted.  = shouldn't be at the end of line.
Better
   cur_row->ra_state
 = (cur_row->ra_state == ra_no_signing
? ra_signing_sp : ra_no_signing);

Jakub



The comments are addressed in the next revision.

Thanks,
Matthieu.

[PATCH] aarch64: fix build failure on aarch64-none-elf

[Matthieu Longo]

A previous patch ([1]) introduced a build regression on aarch64-none-elf
target. The changes were primarilly tested on aarch64-unknown-linux-gnu,
so the issue was missed during development.
The includes are slighly different between the two targets, and due to some
include rules ([2]), "aarch64-unwind-def.h" was not found.

[1]: 
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=bdf41d627c13bc5f0dc676991f4513daa9d9ae36

[2]: https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html
> include "file"
> ...  It searches for a file named file first in the directory
> containing the current file, ...

libgcc/ChangeLog:

* config/aarch64/aarch64-unwind.h: fix header path.
---
 libgcc/config/aarch64/aarch64-unwind.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgcc/config/aarch64/aarch64-unwind.h 
b/libgcc/config/aarch64/aarch64-unwind.h
index 2b774eb263c..4d36f0b26f7 100644
--- a/libgcc/config/aarch64/aarch64-unwind.h
+++ b/libgcc/config/aarch64/aarch64-unwind.h
@@ -25,7 +25,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
 #if !defined (AARCH64_UNWIND_H) && !defined (__ILP32__)
 #define AARCH64_UNWIND_H
 
-#include "aarch64-unwind-def.h"
+#include "config/aarch64/aarch64-unwind-def.h"
 
 #include "ansidecl.h"
 #include 
-- 
2.46.1

Re: [PATCH] aarch64: fix build failure on aarch64-none-elf

[Matthieu Longo]

On 2024-09-26 18:41, Andrew Pinski wrote:

On Thu, Sep 26, 2024 at 10:28 AM Andrew Pinski  wrote:


On Thu, Sep 26, 2024 at 10:15 AM Matthieu Longo  wrote:


A previous patch ([1]) introduced a build regression on aarch64-none-elf
target. The changes were primarilly tested on aarch64-unknown-linux-gnu,
so the issue was missed during development.
The includes are slighly different between the two targets, and due to some
include rules ([2]), "aarch64-unwind-def.h" was not found.

[1]: 
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=bdf41d627c13bc5f0dc676991f4513daa9d9ae36

[2]: https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html

include "file"
...  It searches for a file named file first in the directory
containing the current file, ...


Can you provide more information on this because I am not sure this is
the right fix.
aarch64-unwind.h and aarch64-unwind-def.h are in the same directory so
an include from aarch64-unwind.h should find aarch64-unwind-def.h that
is in the same directory.
Maybe post what strace shows to see what files are being looked for.


Oh looking into the issue some more, aarch64-unwind.h gets linked to
md-unwind-support.h in the build directory and that is used.
I see there was a new makefile target added for md-unwind-def.h. Maybe
this should be including md-unwind-def.h instead of
aarch64-unwind-def.h.

Thanks,
Andrew Pinski



I attached the trace logs for the failure case (aarch64-none-elf).

In my understanding, what happens here is the following:

1. Symbolic links in //libgcc:

 a. for aarch64-none-elf:

md-unwind-def.h -> /libgcc/config/aarch64/aarch64-unwind-def.h
md-unwind-support.h -> /libgcc/config/aarch64/aarch64-unwind.h

 b. for aarch64-unknown-linux-gnu:

md-unwind-def.h -> /libgcc/config/aarch64/aarch64-unwind-def.h
md-unwind-support.h -> /libgcc/config/aarch64/linux-unwind.h

2. Includes sequence

Note: "->" means "#include "the_file""

 a. for aarch64-none-elf:

unwind-dw2.c
 * -> unwind-dw2.h -> md-unwind-def.h => symbolic link to 
config/aarch64/aarch64-unwind-def.h
 * -> md-unwind-support.h => symbolic link to 
config/aarch64/aarch-unwind.h -> aarch64-unwind-def.h


 b. for aarch64-unknown-linux-gnu:

unwind-dw2.c
 * -> unwind-dw2.h -> md-unwind-def.h => symbolic link to 
config/aarch64/aarch64-unwind-def.h
 * -> md-unwind-support.h => symbolic link to 
config/aarch64/linux-unwind.h -> config/aarch64/aarch64-unwind.h -> 
aarch64-unwind-def.h


3. My understanding

In the case 2.b, md-unwind-support.h points to 
config/aarch64/linux-unwind.h, which then includes 
config/aarch64/aarch64-unwind.h

According to [1]
> include "file"
> ...  It searches for a file named file first in the directory
> containing the current file, ...
This means that once config/aarch64/aarch64-unwind.h is found, the 
compiler searches for aarch64-unwind-def.h in the same directory as 
aarch64-unwind.h (i.e. config/aarch64/), hence it finds the file.


In the case 2.a, md-unwind-support.h points to 
config/aarch64/aarch64-unwind.h, which then includes aarch64-unwind-def.h.
However, in this case, the current file directory is not 
config/aarch64/, but //libgcc (the directory of the 
symlink). That's why aarch64-unwind-def.h cannot be found.


4. The fix

The fix I proposed provides the full path to aarch64-unwind-def.h, as it 
is done in config/aarch64/linux-unwind.h for 
config/aarch64/aarch64-unwind.h.
Including md-unwind-def.h instead of aarch64-unwind-def.h in 
config/aarch64/aarch64-unwind.h should work, but it makes more sense 
from my point of view to include the target file instead.


Thanks,
Matthieu.

[1]: https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html



libgcc/ChangeLog:

 * config/aarch64/aarch64-unwind.h: fix header path.
---
  libgcc/config/aarch64/aarch64-unwind.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgcc/config/aarch64/aarch64-unwind.h 
b/libgcc/config/aarch64/aarch64-unwind.h
index 2b774eb263c..4d36f0b26f7 100644
--- a/libgcc/config/aarch64/aarch64-unwind.h
+++ b/libgcc/config/aarch64/aarch64-unwind.h
@@ -25,7 +25,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
  #if !defined (AARCH64_UNWIND_H) && !defined (__ILP32__)
  #define AARCH64_UNWIND_H

-#include "aarch64-unwind-def.h"
+#include "config/aarch64/aarch64-unwind-def.h"

  #include "ansidecl.h"
  #include 
--
2.46.1

5170  execve("/data/build-dir/gcc-aarch64-none-elf/./gcc/xgcc", 
["/data/build-dir/gcc-aarch64-none"..., "-B/data/build-dir/gcc-aarch64-no"..., 
"-B/data/common-sysroot/aarch64-n"..., "-B/data/common-sysroot/aarch64-n"..., 
"-isystem", "/data/common-sysroot/aarch64-non"..., "-isystem", 
"/data/common-sysroot/aarch64-non"..., "-g", "-O2", "-O2", 

[PATCH v1 2/4] aarch64: add minimal support for GCS build attributes

[Matthieu Longo]

From: Srinath Parvathaneni 

GCS (Guarded Control Stack, an Armv9.4-a extension) requires some
caution at runtime. The runtime linker needs to reason about the
compatibility of a set of relocable object files that might not have
been compiled with the same compiler.
Up until now, GNU properties are stored in a ELF section (.note.gnu.property)
and have been used for the previously mentioned runtime checks
performed by the linker. However, GNU properties are limited in
their expressibility, and a long-term commmitment was taken in the
ABI for the Arm architecture [1] to provide build attributes.

This patch adds a first support for AArch64 GCS build attributes.
This support includes generating two new assembler directives:
.aeabi_subsection and .aeabi_attribute. These directives are
generated as per the syntax mentioned in spec "Build Attributes for
the Arm® 64-bit Architecture (AArch64)" available at [1].

gcc/configure.ac now includes a new check to test whether the
assembler being used to build the toolchain supports these new
directives.
Two behaviors can be observed when -mbranch-protection=[gcs|standard]
is passed:
- If the assembler support them, the assembly directives are emitted
along the .note.gnu.property section for backward compatibility.
- If the assembler does not support them, only .note.gnu.property
section will emit the relevant information.

This patch needs to be applied on top of GCC gcs patch series [2].

Bootstrapped on aarch64-none-linux-gnu, and no regression found.

[1]: https://github.com/ARM-software/abi-aa/pull/230
[2]: https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/vendors/ARM/heads/gcs

gcc/ChangeLog:

* config.in: Regenerated
* config/aarch64/aarch64.cc (aarch64_emit_aeabi_attribute): New
function declaration.
(aarch64_emit_aeabi_subsection): Likewise.
(aarch64_start_file): Emit gcs build attributes.
(aarch64_file_end_indicate_exec_stack): Update gcs bit in
note.gnu.property section.
* configure: Regenerated.
* configure.ac: Add gcc configure check.

gcc/testsuite/ChangeLog:

* gcc.target/aarch64/build-attributes/build-attribute-gcs.c: New test.
* gcc.target/aarch64/build-attributes/build-attribute-standard.c: New test.
* gcc.target/aarch64/build-attributes/build-attributes.exp: New DejaGNU 
file.
* gcc.target/aarch64/build-attributes/no-build-attribute-bti.c: New test.
* gcc.target/aarch64/build-attributes/no-build-attribute-gcs.c: New test.
* gcc.target/aarch64/build-attributes/no-build-attribute-pac.c: New test.
* gcc.target/aarch64/build-attributes/no-build-attribute-standard.c: New 
test.
---
 gcc/config.in |  6 +++
 gcc/config/aarch64/aarch64.cc | 41 +
 gcc/configure | 38 +++
 gcc/configure.ac  | 10 
 .../build-attributes/build-attribute-gcs.c| 12 +
 .../build-attribute-standard.c| 12 +
 .../build-attributes/build-attributes.exp | 46 +++
 .../build-attributes/no-build-attribute-bti.c | 12 +
 .../build-attributes/no-build-attribute-gcs.c | 12 +
 .../build-attributes/no-build-attribute-pac.c | 12 +
 .../no-build-attribute-standard.c | 12 +
 11 files changed, 213 insertions(+)
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attribute-gcs.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attribute-standard.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attributes.exp
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-gcs.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-pac.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-standard.c

diff --git a/gcc/config.in b/gcc/config.in
index 7fcabbe5061..1309ba2b133 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -379,6 +379,12 @@
 #endif
 
 
+/* Define if your assembler supports GCS build attributes. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_AS_BUILD_ATTRIBUTES_GCS
+#endif
+
+
 /* Define to the level of your assembler's compressed debug section support.
*/
 #ifndef USED_FOR_TARGET
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 6d9075011ec..61e0248817f 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -24677,6 +24677,33 @@ aarch64_post_cfi_startproc (FILE *f, tree ignored 
ATTRIBUTE_UNUSED)
asm_fprintf (f, "\t.cfi_b_key_frame\n");
 }
 
+/* This function is used to emit an AEABI attribute with tag and its associated
+   value.  We emit the numerical value of the tag and the textual tags as
+   comment so that anyone reading the assembler output will know which tag is
+   b

[PATCH v1 0/4][RFC] aarch64: add minimal support for GCS build attributes

[Matthieu Longo]

The primary focus of this patch series is to add support for build attributes 
in the context of GCS (Guarded Control Stack, an Armv9.4-a extension) to the 
AArch64 backend.
It addresses comments from a previous review [1], and proposes a different 
approach compared to the previous implementation of the build attributes.

The series is for now at the RFC stage, and is composed of the following 4 
patches:
1. Patch adding assembly debug comments (-dA) to the existing GNU properties, 
to improve testing and check the correctness of values.
2. The minimal patch adding support for build attributes in the context of GCS.
3. A refactoring of (2) to make things less error-prone and more modular, add 
support for asciz attributes and more debug information.
4. A refactoring of (1) relying partly on (3).
The targeted final state of this series would consist in squashing (2) + (3), 
and (1) + (4).

**Special note regarding (2):** If Gas has support for build attributes, both 
build attributes and GNU properties will be emitted. This behavior is still 
open for discussion. Please, let me know your thoughts regarding this behavior.

Diff with previous revision [1]:
- update the description of (2)
- address the comments related to the tests in (2)
- add new commits (1), (3) and (4)

This patch series needs to be applied on top of the patch series for GCS [2].

Bootstrapped on aarch64-none-linux-gnu, and no regression found.

[1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662825.html
[2]: https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/vendors/ARM/heads/gcs

Regards,
Matthieu


Matthieu Longo (3):
  aarch64: add debug comments to feature properties in .note.gnu.property
  aarch64: improve assembly debug comments for build attributes
  aarch64: encapsulate note.gnu.property emission into a class

Srinath Parvathaneni (1):
  aarch64: add minimal support for GCS build attributes

 gcc/config.gcc|   2 +-
 gcc/config.in |   6 +
 gcc/config/aarch64/aarch64-dwarf-metadata.cc  | 120 
 gcc/config/aarch64/aarch64-dwarf-metadata.h   | 266 ++
 gcc/config/aarch64/aarch64.cc |  68 ++---
 gcc/config/aarch64/t-aarch64  |   7 +
 gcc/configure |  38 +++
 gcc/configure.ac  |  10 +
 gcc/testsuite/gcc.target/aarch64/bti-1.c  |   5 +-
 .../build-attributes/build-attribute-gcs.c|  12 +
 .../build-attribute-standard.c|  12 +
 .../build-attributes/build-attributes.exp |  46 +++
 .../build-attributes/no-build-attribute-bti.c |  12 +
 .../build-attributes/no-build-attribute-gcs.c |  12 +
 .../build-attributes/no-build-attribute-pac.c |  12 +
 .../no-build-attribute-standard.c |  12 +
 16 files changed, 594 insertions(+), 46 deletions(-)
 create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.cc
 create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.h
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attribute-gcs.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attribute-standard.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/build-attributes.exp
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-bti.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-gcs.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-pac.c
 create mode 100644 
gcc/testsuite/gcc.target/aarch64/build-attributes/no-build-attribute-standard.c

-- 
2.46.1

[PATCH v1 4/4] aarch64: encapsulate note.gnu.property emission into a class

[Matthieu Longo]

gcc/ChangeLog:

* config.gcc: Add aarch64-dwarf-metadata.o to extra_objs.
* config/aarch64/aarch64-dwarf-metadata.h (class section_note_gnu_property):
Encapsulate GNU properties code into a class.
* config/aarch64/aarch64.cc
(GNU_PROPERTY_AARCH64_FEATURE_1_AND): Define.
(GNU_PROPERTY_AARCH64_FEATURE_1_BTI): Likewise.
(GNU_PROPERTY_AARCH64_FEATURE_1_PAC): Likewise.
(GNU_PROPERTY_AARCH64_FEATURE_1_GCS): Likewise.
(aarch64_file_end_indicate_exec_stack): Move GNU properties code to
aarch64-dwarf-metadata.cc
* config/aarch64/t-aarch64: Declare target aarch64-dwarf-metadata.o
* config/aarch64/aarch64-dwarf-metadata.cc: New file.
---
 gcc/config.gcc   |   2 +-
 gcc/config/aarch64/aarch64-dwarf-metadata.cc | 120 +++
 gcc/config/aarch64/aarch64-dwarf-metadata.h  |  19 +++
 gcc/config/aarch64/aarch64.cc|  87 +-
 gcc/config/aarch64/t-aarch64 |   7 ++
 5 files changed, 153 insertions(+), 82 deletions(-)
 create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.cc

diff --git a/gcc/config.gcc b/gcc/config.gcc
index f09ce9f63a0..b448c2a91d1 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -351,7 +351,7 @@ aarch64*-*-*)
c_target_objs="aarch64-c.o"
cxx_target_objs="aarch64-c.o"
d_target_objs="aarch64-d.o"
-   extra_objs="aarch64-builtins.o aarch-common.o aarch64-sve-builtins.o 
aarch64-sve-builtins-shapes.o aarch64-sve-builtins-base.o 
aarch64-sve-builtins-sve2.o aarch64-sve-builtins-sme.o 
cortex-a57-fma-steering.o aarch64-speculation.o 
falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o 
aarch64-early-ra.o aarch64-ldp-fusion.o"
+   extra_objs="aarch64-builtins.o aarch-common.o aarch64-dwarf-metadata.o 
aarch64-sve-builtins.o aarch64-sve-builtins-shapes.o 
aarch64-sve-builtins-base.o aarch64-sve-builtins-sve2.o 
aarch64-sve-builtins-sme.o cortex-a57-fma-steering.o aarch64-speculation.o 
falkor-tag-collision-avoidance.o aarch-bti-insert.o aarch64-cc-fusion.o 
aarch64-early-ra.o aarch64-ldp-fusion.o"
target_gtfiles="\$(srcdir)/config/aarch64/aarch64-builtins.h 
\$(srcdir)/config/aarch64/aarch64-builtins.cc 
\$(srcdir)/config/aarch64/aarch64-sve-builtins.h 
\$(srcdir)/config/aarch64/aarch64-sve-builtins.cc"
target_has_targetm_common=yes
;;
diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.cc 
b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
new file mode 100644
index 000..36659862b59
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-dwarf-metadata.cc
@@ -0,0 +1,120 @@
+#define INCLUDE_STRING
+#define INCLUDE_ALGORITHM
+#define INCLUDE_MEMORY
+#define INCLUDE_VECTOR
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "target.h"
+#include "rtl.h"
+#include "output.h"
+
+#include "aarch64-dwarf-metadata.h"
+
+#include 
+
+/* Defined for convenience.  */
+#define POINTER_BYTES (POINTER_SIZE / BITS_PER_UNIT)
+
+namespace aarch64 {
+
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_AND = 0xc000;
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_BTI = (1U << 0);
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_PAC = (1U << 1);
+constexpr unsigned GNU_PROPERTY_AARCH64_FEATURE_1_GCS = (1U << 2);
+
+namespace {
+
+std::string join_s (std::string s1, const std::string &s2)
+{
+  constexpr const char* separator = ", ";
+  return std::move (s1)
+   .append (separator)
+   .append (s2);
+};
+
+std::string gnu_property_features_to_string (unsigned feature_1_and)
+{
+  std::vector feature_bits;
+
+  if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)
+feature_bits.push_back ("BTI");
+  if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_PAC)
+feature_bits.push_back ("PAC");
+  if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_GCS)
+feature_bits.push_back ("GCS");
+
+  if (feature_bits.empty ())
+return {};
+
+  return std::accumulate(std::next(feature_bits.cbegin()), feature_bits.cend(),
+feature_bits[0], join_s);
+};
+
+} // namespace anonymous
+
+section_note_gnu_property::section_note_gnu_property():
+  feature_1_and(0)
+{}
+
+void section_note_gnu_property::bti_enabled()
+{
+  feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
+}
+
+void section_note_gnu_property::pac_enabled()
+{
+  feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
+}
+
+void section_note_gnu_property::gcs_enabled()
+{
+  feature_1_and |= GNU_PROPERTY_AARCH64_FEATURE_1_GCS;
+}
+
+void section_note_gnu_property::write () const
+{
+  if (feature_1_and)
+{
+  /* Generate .note.gnu.property section.  */
+  switch_to_section (get_section (".note.gnu.property",
+ SECTION_NOTYPE, NULL));
+
+  /* PT_NOTE header: namesz, descsz, type.
+namesz = 4 ("GNU\0")
+descsz = 16 (Size of the program property array)
+ [(12 + padding) * Number of array elements]
+type   = 

[PATCH v1 1/4] aarch64: add debug comments to feature properties in .note.gnu.property

[Matthieu Longo]

GNU properties are emitted to provide some information about the features
used in the generated code like PAC, BTI, or GCS. However, no debug
comment are emitted in the generated assembly even if -dA is provided.
This makes understanding the information stored in the .note.gnu.property
section more difficult than needed.

This patch adds assembly comments (if -dA is provided) next to the GNU
properties. For instance, if PAC and BTI are enabled, it will emit:
  .word  3  // GNU_PROPERTY_AARCH64_FEATURE_1_AND (BTI, PAC)

gcc/ChangeLog:

* config/aarch64/aarch64.cc
(aarch64_file_end_indicate_exec_stack): Emit assembly comments.

gcc/testsuite/ChangeLog:

* gcc.target/aarch64/bti-1.c: Emit assembly comments, and update
  test assertion.
---
 gcc/config/aarch64/aarch64.cc| 41 +++-
 gcc/testsuite/gcc.target/aarch64/bti-1.c |  5 +--
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 4b2e7a690c6..6d9075011ec 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -98,6 +98,8 @@
 #include "ipa-fnsummary.h"
 #include "hash-map.h"
 
+#include 
+
 /* This file should be included last.  */
 #include "target-def.h"
 
@@ -29129,8 +29131,45 @@ aarch64_file_end_indicate_exec_stack ()
 data   = feature_1_and.  */
   assemble_integer (GEN_INT (GNU_PROPERTY_AARCH64_FEATURE_1_AND), 4, 32, 
1);
   assemble_integer (GEN_INT (4), 4, 32, 1);
-  assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
 
+  if (!flag_debug_asm)
+   assemble_integer (GEN_INT (feature_1_and), 4, 32, 1);
+  else
+   {
+ asm_fprintf (asm_out_file, "\t.word\t%u", feature_1_and);
+
+ auto join_s = [] (std::string s1,
+   const std::string &s2,
+   const std::string &separator = ", ") -> std::string
+ {
+   return std::move (s1)
+ .append (separator)
+ .append (s2);
+ };
+
+ auto features_to_string
+   = [&join_s] (unsigned feature_1_and) -> std::string
+ {
+   std::vector feature_bits;
+   if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)
+ feature_bits.push_back ("BTI");
+   if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_PAC)
+ feature_bits.push_back ("PAC");
+   if (feature_1_and & GNU_PROPERTY_AARCH64_FEATURE_1_GCS)
+ feature_bits.push_back ("GCS");
+
+   if (feature_bits.empty ())
+ return {};
+   return std::accumulate(std::next(feature_bits.cbegin()),
+  feature_bits.cend(),
+  feature_bits[0],
+  join_s);
+ };
+ auto const& s_features = features_to_string (feature_1_and);
+ asm_fprintf (asm_out_file,
+   "\t%s GNU_PROPERTY_AARCH64_FEATURE_1_AND (%s)\n",
+   ASM_COMMENT_START, s_features.c_str ());
+   }
   /* Pad the size of the note to the required alignment.  */
   assemble_align (POINTER_SIZE);
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/bti-1.c 
b/gcc/testsuite/gcc.target/aarch64/bti-1.c
index 5a556b08ed1..e48017abc35 100644
--- a/gcc/testsuite/gcc.target/aarch64/bti-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/bti-1.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* -Os to create jump table.  */
-/* { dg-options "-Os" } */
+/* { dg-options "-Os -dA" } */
 /* { dg-require-effective-target lp64 } */
 /* If configured with --enable-standard-branch-protection, don't use
command line option.  */
@@ -61,4 +61,5 @@ lab2:
 }
 /* { dg-final { scan-assembler-times "hint\t34" 1 } } */
 /* { dg-final { scan-assembler-times "hint\t36" 12 } } */
-/* { dg-final { scan-assembler ".note.gnu.property" { target *-*-linux* } } } 
*/
+/* { dg-final { scan-assembler "\.section\t\.note\.gnu\.property" { target 
*-*-linux* } } } */
+/* { dg-final { scan-assembler "\.word\t7\t\/\/ 
GNU_PROPERTY_AARCH64_FEATURE_1_AND \\(BTI, PAC, GCS\\)" { target *-*-linux* } } 
} */
\ No newline at end of file
-- 
2.46.1

[PATCH v1 3/4] aarch64: improve assembly debug comments for build attributes

[Matthieu Longo]

The previous implementation to emit build attributes did not support
string values (asciz) in aeabi_subsection, and was not emitting values
associated to tags in the assembly comments.

This new approach provides a more user-friendly interface relying on
typing, and improves the emitted assembly comments:
  * aeabi_attribute:
** Adds the interpreted value next to the tag in the assembly
comment.
** Supports asciz values.
  * aeabi_subsection:
** Adds debug information for its parameters.
** Auto-detects the attribute types when declaring the subsection.

Additionally, it is also interesting to note that the code was moved
to a separate file to improve modularity and "releave" the 1000-lines
long aarch64.cc file from a few lines. Finally, it introduces a new
namespace "aarch64::" for AArch64 backend which reduce the length of
function names by not prepending 'aarch64_' to each of them.

gcc/ChangeLog:

   * config/aarch64/aarch64.cc
   (aarch64_emit_aeabi_attribute): Delete.
   (aarch64_emit_aeabi_subsection): Delete.
   (aarch64_start_file): Use aeabi_subsection.
   * config/aarch64/aarch64-dwarf-metadata.h: New file.

gcc/testsuite/ChangeLog:

   * gcc.target/aarch64/build-attributes/build-attribute-gcs.c:
 Improve test to match debugging comments in assembly.
   * gcc.target/aarch64/build-attributes/build-attribute-standard.c:
 Likewise.
---
 gcc/config/aarch64/aarch64-dwarf-metadata.h   | 247 ++
 gcc/config/aarch64/aarch64.cc |  43 +--
 .../build-attributes/build-attribute-gcs.c|   4 +-
 .../build-attribute-standard.c|   4 +-
 4 files changed, 261 insertions(+), 37 deletions(-)
 create mode 100644 gcc/config/aarch64/aarch64-dwarf-metadata.h

diff --git a/gcc/config/aarch64/aarch64-dwarf-metadata.h 
b/gcc/config/aarch64/aarch64-dwarf-metadata.h
new file mode 100644
index 000..9638bc7702f
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-dwarf-metadata.h
@@ -0,0 +1,247 @@
+/* Machine description for AArch64 architecture.
+   Copyright (C) 2009-2024 Free Software Foundation, Inc.
+   Contributed by ARM Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   .  */
+
+#ifndef GCC_AARCH64_DWARF_METADATA_H
+#define GCC_AARCH64_DWARF_METADATA_H
+
+#include 
+#include 
+
+#include 
+
+namespace aarch64 {
+
+enum attr_val_type: uint8_t
+{
+  uleb128 = 0x0,
+  asciz = 0x1,
+};
+
+enum BA_TagFeature_t: uint8_t
+{
+  Tag_Feature_BTI = 1,
+  Tag_Feature_PAC = 2,
+  Tag_Feature_GCS = 3,
+};
+
+template 
+struct aeabi_attribute
+{
+  T_tag tag;
+  T_val value;
+};
+
+template 
+aeabi_attribute
+make_aeabi_attribute (T_tag tag, T_val val)
+{
+  return aeabi_attribute{tag, val};
+}
+
+namespace details {
+
+constexpr const char*
+to_c_str (bool b)
+{
+  return b ? "true" : "false";
+}
+
+constexpr const char*
+to_c_str (const char *s)
+{
+  return s;
+}
+
+constexpr const char*
+to_c_str (attr_val_type t)
+{
+  const char *s = nullptr;
+  switch (t) {
+case uleb128:
+  s = "ULEB128";
+  break;
+case asciz:
+  s = "asciz";
+  break;
+  }
+  return s;
+}
+
+constexpr const char*
+to_c_str (BA_TagFeature_t feature)
+{
+  const char *s = nullptr;
+  switch (feature) {
+case Tag_Feature_BTI:
+  s = "Tag_Feature_BTI";
+  break;
+case Tag_Feature_PAC:
+  s = "Tag_Feature_PAC";
+  break;
+case Tag_Feature_GCS:
+  s = "Tag_Feature_GCS";
+  break;
+  }
+  return s;
+}
+
+template <
+  typename T,
+  typename = typename std::enable_if::value, T>::type
+>
+constexpr const char*
+aeabi_attr_str_fmt (T phantom __attribute__((unused)))
+{
+return "\t.aeabi_attribute %u, %u";
+}
+
+constexpr const char*
+aeabi_attr_str_fmt (const char *phantom __attribute__((unused)))
+{
+return "\t.aeabi_attribute %u, \"%s\"";
+}
+
+template <
+  typename T,
+  typename = typename std::enable_if::value, T>::type
+>
+constexpr uint8_t
+aeabi_attr_val_for_fmt (T value)
+{
+  return static_cast(value);
+}
+
+constexpr const char*
+aeabi_attr_val_for_fmt (const char *s)
+{
+  return s;
+}
+
+template 
+void write (FILE *out_file, aeabi_attribute const &attr)
+{
+  asm_fprintf (out_file, aeabi_attr_str_fmt(T_val{}),
+  attr.tag, aeabi_attr_val_for_fmt(attr.value));
+  if (flag_debug_asm)
+asm_fprintf (out_file, "\t%s %s: %s", ASM_COMMENT_START,
+to_c_str(attr.tag), to_

[PATCH v2 1/4] aarch64: store signing key and signing method in DWARF _Unwind_FrameState

[Matthieu Longo]

This patch is only a refactoring of the existing implementation
of PAuth and returned-address signing. The existing behavior is
preserved.

_Unwind_FrameState already contains several CIE and FDE information
(see the attributes below the comment "The information we care
about from the CIE/FDE" in libgcc/unwind-dw2.h).
The patch aims at moving the information from DWARF CIE (signing
key stored in the augmentation string) and FDE (the used signing
method) into _Unwind_FrameState along the already-stored CIE and
FDE information.
Note: those information have to be saved in frame_state_reg_info
instead of _Unwind_FrameState as they need to be savable by
DW_CFA_remember_state and restorable by DW_CFA_restore_state, that
both rely on the attribute "prev".

Those new information in _Unwind_FrameState simplifies the look-up
of the signing key when the return address is demangled. It also
allows future signing methods to be easily added.

_Unwind_FrameState is not a part of the public API of libunwind,
so the change is backward compatible.

A new architecture-specific handler MD_ARCH_EXTENSION_FRAME_INIT
allows to reset values (if needed) in the frame state and unwind
context before changing the frame state to the caller context.

A new architecture-specific handler MD_ARCH_EXTENSION_CIE_AUG_HANDLER
isolates the architecture-specific augmentation strings in AArch64
backend, and allows others architectures to reuse augmentation
strings that would have clashed with AArch64 DWARF extensions.

aarch64_demangle_return_addr, DW_CFA_AARCH64_negate_ra_state and
DW_CFA_val_expression cases in libgcc/unwind-dw2-execute_cfa.h
were documented to clarify where the value of the RA state register
is stored (FS and CONTEXT respectively).

libgcc/ChangeLog:

  * config/aarch64/aarch64-unwind.h
  (AARCH64_DWARF_RA_STATE_MASK): The mask for RA state register.
  (aarch64_ra_signing_method_t): The diversifiers used to sign a
  function's return address.
  (aarch64_pointer_auth_key): The key used to sign a function's
  return address.
  (aarch64_cie_signed_with_b_key): Deleted as the signing key is
  available now in _Unwind_FrameState.
  (MD_ARCH_EXTENSION_CIE_AUG_HANDLER): New CIE augmentation string
  handler for architecture extensions.
  (MD_ARCH_EXTENSION_FRAME_INIT): New architecture-extension
  initialization routine for DWARF frame state and context before
  execution of DWARF instructions.
  (aarch64_context_ra_state_get): Read RA state register from CONTEXT.
  (aarch64_ra_state_get): Read RA state register from FS.
  (aarch64_ra_state_set): Write RA state register into FS.
  (aarch64_ra_state_toggle): Toggle RA state register in FS.
  (aarch64_cie_aug_handler): Handler AArch64 augmentation strings.
  (aarch64_arch_extension_frame_init): Initialize defaults for the
  signing key (PAUTH_KEY_A), and RA state register (RA_no_signing).
  (aarch64_demangle_return_addr): Rely on the frame registers and
  the signing_key attribute in _Unwind_FrameState.
  * unwind-dw2-execute_cfa.h:
  Use the right alias DW_CFA_AARCH64_negate_ra_state for __aarch64__
  instead of DW_CFA_GNU_window_save.
  (DW_CFA_AARCH64_negate_ra_state): Save the signing method in RA
  state register. Toggle RA state register without resetting 'how'
  to REG_UNSAVED.
  * unwind-dw2.c:
  (extract_cie_info): Save the signing key in the current
  _Unwind_FrameState while parsing the augmentation data.
  (uw_frame_state_for): Reset some attributes related to architecture
  extensions in _Unwind_FrameState.
  (uw_update_context): Move authentication code to AArch64 unwinding.
  * unwind-dw2.h (enum register_rule): Give a name to the existing
  enum for the register rules, and replace 'unsigned char' by 'enum
  register_rule' to facilitate debugging in GDB.
  (_Unwind_FrameState): Add a new architecture-extension attribute
  to store the signing key.
---
 libgcc/config/aarch64/aarch64-unwind.h | 145 -
 libgcc/unwind-dw2-execute_cfa.h|  26 +++--
 libgcc/unwind-dw2.c|  19 +++-
 libgcc/unwind-dw2.h|  17 ++-
 4 files changed, 159 insertions(+), 48 deletions(-)

diff --git a/libgcc/config/aarch64/aarch64-unwind.h 
b/libgcc/config/aarch64/aarch64-unwind.h
index daf96624b5e..94ea5891b4e 100644
--- a/libgcc/config/aarch64/aarch64-unwind.h
+++ b/libgcc/config/aarch64/aarch64-unwind.h
@@ -25,52 +25,145 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
If not, see
 #if !defined (AARCH64_UNWIND_H) && !defined (__ILP32__)
 #define AARCH64_UNWIND_H
 
-#define DWARF_REGNUM_AARCH64_RA_STATE 34
+#include "ansidecl.h"
+#include 
+
+#define AARCH64_DWARF_REGNUM_RA_STATE 34
+#define AARCH64_DWARF_RA_STATE_MASK   0x1
+
+/* The diversifiers used to sign a function's return address.  */
+typedef enum
+{
+  aarch64_ra_no_signing = 0x0,
+  aarch64_ra_signing_sp = 0x1,
+} __attribute__((packed)) aarch64_ra_signing_method_t;
+
+/* The key used to sign a function's return address.  */
+typedef enum {
+  AARCH64_PAUTH_

[PATCH v2 0/4][libgcc] store signing key and signing method in DWARF _Unwind_FrameState

[Matthieu Longo]

This patch series is only a refactoring of the existing implementation of PAuth 
and returned-address signing. The existing behavior is preserved.

1. aarch64: store signing key and signing method in DWARF _Unwind_FrameState

_Unwind_FrameState already contains several CIE and FDE information (see the 
attributes below the comment "The information we care about from the CIE/FDE" 
in libgcc/unwind-dw2.h).
The patch aims at moving the information from DWARF CIE (signing key stored in 
the augmentation string) and FDE (the used signing method) into 
_Unwind_FrameState along the already-stored CIE and FDE information.
Note: those information have to be saved in frame_state_reg_info instead of 
_Unwind_FrameState as they need to be savable by DW_CFA_remember_state and 
restorable by DW_CFA_restore_state, that both rely on the attribute "prev".
Those new information in _Unwind_FrameState simplifies the look-up of the 
signing key when the return address is demangled. It also allows future signing 
methods to be easily added.
_Unwind_FrameState is not a part of the public API of libunwind, so the change 
is backward compatible.

A new architecture-specific handler MD_ARCH_EXTENSION_FRAME_INIT allows to 
reset values in the frame state and unwind context if needed by the 
architecture extension before changing the frame state to the caller context.
A new architecture-specific handler MD_ARCH_EXTENSION_CIE_AUG_HANDLER isolates 
the architecture-specific augmentation strings in AArch64 backend, and allows 
others architectures to reuse augmentation strings that would have clashed with 
AArch64 DWARF extensions.
aarch64_demangle_return_addr, DW_CFA_AARCH64_negate_ra_state and 
DW_CFA_val_expression cases in libgcc/unwind-dw2-execute_cfa.h were documented 
to clarify where the value of the RA state register is stored (FS and CONTEXT 
respectively).

2. aarch64: skip copy of RA state register into target context

The RA state register is local to a frame, so it should not be copied to the 
target frame during the context installation.
This patch adds a new backend handler that check whether a register needs to be 
skipped or not before its installation.

3. libgcc: hide CIE and FDE data for DWARF architecture extensions behind a 
handler.

This patch provides a new handler MD_ARCH_FRAME_STATE_T to hide an 
architecture-specific structure containing CIE and FDE data related to DWARF 
architecture extensions.
Hiding the architecture-specific attributes behind a handler has the following 
benefits:
1. isolating those data from the generic ones in _Unwind_FrameState
2. avoiding casts to custom types.
3. preserving typing information when debugging with GDB, and so 
facilitating their printing.

This approach required to add a new header md-unwind-def.h included at the top 
of libgcc/unwind-dw2.h, and redirecting to the corresponding architecture 
header via a symbolic link.
An obvious drawback is the increase in complexity with macros, and headers. It 
also caused a split of architecture definitions between md-unwind-def.h (types 
definitions used in unwind-dw2.h) and md-unwind.h (local types definitions and 
handlers implementations).
The naming of md-unwind.h with .h extension is a bit misleading as the file is 
only included in the middle of unwind-dw2.c. Changing this naming would require 
modification of others backends, which I prefered to abstain from.
Overall the benefits are worth the added complexity from my perspective.

4. libgcc: update configure (regenerated by autoreconf)

Regenerate the build files.
Note: This patch should be squashed with the previous one before merging.

### Diff between revisions 1 & 2

1: code style formatting + remove resetting of the local frame register in the 
context as recommended by Richard Sandiford.
2: add a new handler as recommended by Richard Sandiford to skip the local 
frame registers before installation in the target context.
3: code style formatting.
4: no change.

## Testing

Those changes were testing by covering the 3 following cases:
- backtracing.
- exception handling in a C++ program.
- gcc/testsuite/gcc.target/aarch64/pr104689.c: pac-ret with unusual DWARF [1]

Regression tested on aarch64-unknown-linux-gnu, and no regression found.

[1]: https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594414.html

Ok for master? I don't have commit access so I need someone to commit on my 
behalf.

Regards,
Matthieu.

Matthieu Longo (4):
  aarch64: store signing key and signing method in DWARF
_Unwind_FrameState
  aarch64: skip copy of RA state register into target context
  libgcc: hide CIE and FDE data for DWARF architecture extensions behind
a handler.
  libgcc: update configure (regenerated by autoreconf)

 libgcc/Makefile.in |   6 +-
 libgcc/config.host |  13 +-
 libgcc/config/aarch64/aarch64-unwind-def.h |  41 ++
 libgcc/config/aarch64/aarch64-unwind.h | 152 +++

[PATCH v2 2/4] aarch64: skip copy of RA state register into target context

[Matthieu Longo]

The RA state register is local to a frame, so it should not be copied to
the target frame during the context installation.

This patch adds a new backend handler that check whether a register
needs to be skipped or not before its installation.

libgcc/ChangeLog:

   * config/aarch64/aarch64-unwind.h
   (MD_FRAME_LOCAL_REGISTER_P): new handler checking whether a register
   from the current context needs to be skipped before installation into
   the target context.
   (aarch64_frame_local_register):
   * unwind-dw2.c (uw_install_context_1): use MD_FRAME_LOCAL_REGISTER_P.
---
 libgcc/config/aarch64/aarch64-unwind.h | 11 +++
 libgcc/unwind-dw2.c|  5 +
 2 files changed, 16 insertions(+)

diff --git a/libgcc/config/aarch64/aarch64-unwind.h 
b/libgcc/config/aarch64/aarch64-unwind.h
index 94ea5891b4e..52bfd540979 100644
--- a/libgcc/config/aarch64/aarch64-unwind.h
+++ b/libgcc/config/aarch64/aarch64-unwind.h
@@ -53,6 +53,9 @@ typedef enum {
 #define MD_DEMANGLE_RETURN_ADDR(context, fs, addr) \
   aarch64_demangle_return_addr (context, fs, addr)
 
+#define MD_FRAME_LOCAL_REGISTER_P(reg) \
+  aarch64_frame_local_register (reg)
+
 static inline aarch64_ra_signing_method_t
 aarch64_context_ra_state_get (struct _Unwind_Context *context)
 {
@@ -127,6 +130,14 @@ aarch64_arch_extension_frame_init (struct _Unwind_Context 
*context ATTRIBUTE_UNU
   aarch64_fs_ra_state_set (fs, aarch64_ra_no_signing);
 }
 
+/* Before copying the current context to the target context, check whether
+   the register is local to this context and should not be forwarded.  */
+static inline bool
+aarch64_frame_local_register(long reg)
+{
+  return (reg == AARCH64_DWARF_REGNUM_RA_STATE);
+}
+
 /* Do AArch64 private extraction on ADDR_WORD based on context info CONTEXT and
unwind frame info FS.  If ADDR_WORD is signed, we do address authentication
on it using CFA of current frame.
diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c
index 40d64c0c0a3..5f33f80670a 100644
--- a/libgcc/unwind-dw2.c
+++ b/libgcc/unwind-dw2.c
@@ -1423,6 +1423,11 @@ uw_install_context_1 (struct _Unwind_Context *current,
   void *c = (void *) (_Unwind_Internal_Ptr) current->reg[i];
   void *t = (void *) (_Unwind_Internal_Ptr)target->reg[i];
 
+#ifdef MD_FRAME_LOCAL_REGISTER_P
+  if (MD_FRAME_LOCAL_REGISTER_P (i))
+   continue;
+#endif
+
   gcc_assert (current->by_value[i] == 0);
   if (target->by_value[i] && c)
{
-- 
2.46.0

[PATCH v2 4/4] libgcc: update configure (regenerated by autoreconf)

[Matthieu Longo]

libgcc/ChangeLog:

* configure: Regenerate.
---
 libgcc/configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libgcc/configure b/libgcc/configure
index a69d314374a..15a0be23644 100755
--- a/libgcc/configure
+++ b/libgcc/configure
@@ -587,6 +587,7 @@ ac_includes_default='/* none */'
 ac_subst_vars='LTLIBOBJS
 LIBOBJS
 md_unwind_header
+md_unwind_def_header
 unwind_header
 enable_execute_stack
 asm_hidden_op
@@ -5786,6 +5787,7 @@ fi
 
 
 
+
 # We need multilib support.
 ac_config_files="$ac_config_files Makefile"
 
-- 
2.46.0

[PATCH v2 1/4] Rename REG_CFA_TOGGLE_RA_MANGLE to REG_CFA_NEGATE_RA_STATE

[Matthieu Longo]

The current name REG_CFA_TOGGLE_RA_MANGLE is not representative of what
it really is, i.e. a register to represent several states, not only a
binary one. Same for dwarf2out_frame_debug_cfa_toggle_ra_mangle.

gcc/ChangeLog:

* combine-stack-adj.cc
(no_unhandled_cfa): Rename.
* config/aarch64/aarch64.cc
(aarch64_expand_prologue): Rename.
(aarch64_expand_epilogue): Rename.
* dwarf2cfi.cc
(dwarf2out_frame_debug_cfa_toggle_ra_mangle): Rename this...
(dwarf2out_frame_debug_cfa_negate_ra_state): To this.
(dwarf2out_frame_debug): Rename.
* reg-notes.def (REG_CFA_NOTE): Rename REG_CFA_TOGGLE_RA_MANGLE.
---
 gcc/combine-stack-adj.cc  | 2 +-
 gcc/config/aarch64/aarch64.cc | 4 ++--
 gcc/dwarf2cfi.cc  | 8 
 gcc/reg-notes.def | 8 
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/gcc/combine-stack-adj.cc b/gcc/combine-stack-adj.cc
index 2da9bf2bc1e..367d3b66b74 100644
--- a/gcc/combine-stack-adj.cc
+++ b/gcc/combine-stack-adj.cc
@@ -212,7 +212,7 @@ no_unhandled_cfa (rtx_insn *insn)
   case REG_CFA_SET_VDRAP:
   case REG_CFA_WINDOW_SAVE:
   case REG_CFA_FLUSH_QUEUE:
-  case REG_CFA_TOGGLE_RA_MANGLE:
+  case REG_CFA_NEGATE_RA_STATE:
return false;
   }
 
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 92763d403c7..acb2ddb9170 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -9612,7 +9612,7 @@ aarch64_expand_prologue (void)
  default:
gcc_unreachable ();
}
-  add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
+  add_reg_note (insn, REG_CFA_NEGATE_RA_STATE, const0_rtx);
   RTX_FRAME_RELATED_P (insn) = 1;
 }
 
@@ -10033,7 +10033,7 @@ aarch64_expand_epilogue (rtx_call_insn *sibcall)
  default:
gcc_unreachable ();
}
-  add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
+  add_reg_note (insn, REG_CFA_NEGATE_RA_STATE, const0_rtx);
   RTX_FRAME_RELATED_P (insn) = 1;
 }
 
diff --git a/gcc/dwarf2cfi.cc b/gcc/dwarf2cfi.cc
index 1231b5bb5f0..4ad9acbd6fd 100644
--- a/gcc/dwarf2cfi.cc
+++ b/gcc/dwarf2cfi.cc
@@ -1547,13 +1547,13 @@ dwarf2out_frame_debug_cfa_window_save (void)
   cur_row->window_save = true;
 }
 
-/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_TOGGLE_RA_MANGLE.
+/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_NEGATE_RA_STATE.
Note: DW_CFA_GNU_window_save dwarf opcode is reused for toggling RA mangle
state, this is a target specific operation on AArch64 and can only be used
on other targets if they don't use the window save operation otherwise.  */
 
 static void
-dwarf2out_frame_debug_cfa_toggle_ra_mangle (void)
+dwarf2out_frame_debug_cfa_negate_ra_state (void)
 {
   dw_cfi_ref cfi = new_cfi ();
 
@@ -2341,8 +2341,8 @@ dwarf2out_frame_debug (rtx_insn *insn)
handled_one = true;
break;
 
-  case REG_CFA_TOGGLE_RA_MANGLE:
-   dwarf2out_frame_debug_cfa_toggle_ra_mangle ();
+  case REG_CFA_NEGATE_RA_STATE:
+   dwarf2out_frame_debug_cfa_negate_ra_state ();
handled_one = true;
break;
 
diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def
index 5b878fb2a1c..ddcf16b68be 100644
--- a/gcc/reg-notes.def
+++ b/gcc/reg-notes.def
@@ -180,10 +180,10 @@ REG_CFA_NOTE (CFA_WINDOW_SAVE)
the rest of the compiler as a CALL_INSN.  */
 REG_CFA_NOTE (CFA_FLUSH_QUEUE)
 
-/* Attached to insns that are RTX_FRAME_RELATED_P, toggling the mangling status
-   of return address.  Currently it's only used by AArch64.  The argument is
-   ignored.  */
-REG_CFA_NOTE (CFA_TOGGLE_RA_MANGLE)
+/* Attached to insns that are RTX_FRAME_RELATED_P, indicating an authentication
+   of the return address. Currently it's only used by AArch64.
+   The argument is ignored.  */
+REG_CFA_NOTE (CFA_NEGATE_RA_STATE)
 
 /* Indicates what exception region an INSN belongs in.  This is used
to indicate what region to which a call may throw.  REGION 0
-- 
2.46.0

[PATCH v2 3/4] libgcc: hide CIE and FDE data for DWARF architecture extensions behind a handler.

[Matthieu Longo]

This patch provides a new handler MD_ARCH_FRAME_STATE_T to hide an
architecture-specific structure containing CIE and FDE data related
to DWARF architecture extensions.

Hiding the architecture-specific attributes behind a handler has the
following benefits:
1. isolating those data from the generic ones in _Unwind_FrameState
2. avoiding casts to custom types.
3. preserving typing information when debugging with GDB, and so
   facilitating their printing.

This approach required to add a new header md-unwind-def.h included at
the top of libgcc/unwind-dw2.h, and redirecting to the corresponding
architecture header via a symbolic link.

An obvious drawback is the increase in complexity with macros, and
headers. It also caused a split of architecture definitions between
md-unwind-def.h (types definitions used in unwind-dw2.h) and
md-unwind.h (local types definitions and handlers implementations).
The naming of md-unwind.h with .h extension is a bit misleading as
the file is only included in the middle of unwind-dw2.c. Changing
this naming would require modification of others backends, which I
prefered to abstain from. Overall the benefits are worth the added
complexity from my perspective.

libgcc/ChangeLog:

* Makefile.in: New target for symbolic link to md-unwind-def.h
* config.host: New parameter md_unwind_def_header. Set it to
aarch64/aarch64-unwind-def.h for AArch64 targets, or no-unwind.h
by default.
* config/aarch64/aarch64-unwind.h
(aarch64_pointer_auth_key): Move to aarch64-unwind-def.h
(aarch64_cie_aug_handler): Update.
(aarch64_arch_extension_frame_init): Update.
(aarch64_demangle_return_addr): Update.
* configure.ac: New substitute variable md_unwind_def_header.
* unwind-dw2.h (defined): MD_ARCH_FRAME_STATE_T.
* config/aarch64/aarch64-unwind-def.h: New file.
---
 libgcc/Makefile.in |  6 +++-
 libgcc/config.host | 13 +--
 libgcc/config/aarch64/aarch64-unwind-def.h | 41 ++
 libgcc/config/aarch64/aarch64-unwind.h | 14 +++-
 libgcc/config/no-unwind.h  |  3 +-
 libgcc/configure.ac|  1 +
 libgcc/unwind-dw2.h|  6 ++--
 7 files changed, 69 insertions(+), 15 deletions(-)
 create mode 100644 libgcc/config/aarch64/aarch64-unwind-def.h

diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index 0e46e9ef768..ffc45f21267 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -47,6 +47,7 @@ with_aix_soname = @with_aix_soname@
 solaris_ld_v2_maps = @solaris_ld_v2_maps@
 enable_execute_stack = @enable_execute_stack@
 unwind_header = @unwind_header@
+md_unwind_def_header = @md_unwind_def_header@
 md_unwind_header = @md_unwind_header@
 sfp_machine_header = @sfp_machine_header@
 thread_header = @thread_header@
@@ -358,13 +359,16 @@ SHLIBUNWIND_INSTALL =
 
 
 # Create links to files specified in config.host.
-LIBGCC_LINKS = enable-execute-stack.c unwind.h md-unwind-support.h \
+LIBGCC_LINKS = enable-execute-stack.c \
+   unwind.h md-unwind-def.h md-unwind-support.h \
sfp-machine.h gthr-default.h
 
 enable-execute-stack.c: $(srcdir)/$(enable_execute_stack)
-$(LN_S) $< $@
 unwind.h: $(srcdir)/$(unwind_header)
-$(LN_S) $< $@
+md-unwind-def.h: $(srcdir)/config/$(md_unwind_def_header)
+   -$(LN_S) $< $@
 md-unwind-support.h: $(srcdir)/config/$(md_unwind_header)
-$(LN_S) $< $@
 sfp-machine.h: $(srcdir)/config/$(sfp_machine_header)
diff --git a/libgcc/config.host b/libgcc/config.host
index 9fae51d4ce7..7790b824f38 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -51,8 +51,10 @@
 #  If either is set, EXTRA_PARTS and
 #  EXTRA_MULTILIB_PARTS inherited from the GCC
 #  subdirectory will be ignored.
-#  md_unwind_headerThe name of a header file defining
-#  MD_FALLBACK_FRAME_STATE_FOR.
+#  md_unwind_def_header The name of a header file defining architecture
+#  -specific frame information types for unwinding.
+#  md_unwind_headerThe name of a header file defining architecture
+#  -specific handlers used in the unwinder.
 #  sfp_machine_header  The name of a sfp-machine.h header file for soft-fp.
 #  Defaults to "$cpu_type/sfp-machine.h" if it exists,
 #  no-sfp-machine.h otherwise.
@@ -72,6 +74,7 @@ extra_parts=
 tmake_file=
 tm_file=
 tm_define=
+md_unwind_def_header=no-unwind.h
 md_unwind_header=no-unwind.h
 unwind_header=unwind-generic.h
 
@@ -403,6 +406,7 @@ aarch64*-*-elf | aarch64*-*-rtems*)
tmake_file="${tmake_file} ${cpu_type}/t-lse t-slibgcc-libgcc"
tmake_file="${tmake_file} ${cpu_type}/t-softfp t-softfp t-crtfm"
tmake_file="${tmake_file} t-dfprules"
+   md_unwind_def_header=aarch64/aarch64-unwind-def.h
md_unwind_header=aarch64/aarch64-unwind.h
;;
 aarch

[PATCH v2 2/4] dwarf2: add hooks for architecture-specific CFIs

[Matthieu Longo]

Architecture-specific CFI directives are currently declared an processed
among others architecture-independent CFI directives in gcc/dwarf2* files.
This approach creates confusion, specifically in the case of DWARF
instructions in the vendor space and using the same instruction code.

Such a clash currently happen between DW_CFA_GNU_window_save (used on
SPARC) and DW_CFA_AARCH64_negate_ra_state (used on AArch64), and both
having the same instruction code 0x2d.
Then AArch64 compilers generates a SPARC CFI directive (.cfi_window_save)
instead of .cfi_negate_ra_state, contrarilly to what is expected in
[DWARF for the Arm 64-bit Architecture (AArch64)](https://github.com/
ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst).

This refactoring does not solve completely the problem, but improve the
situation by moving some of the processing of those directives (more
specifically their output in the assembly) to the backend via 2 target
hooks:
- DW_CFI_OPRND1_DESC: parse the first operand of the directive (if any).
- OUTPUT_CFI_DIRECTIVE: output the CFI directive as a string.

Additionally, this patch also contains a renaming of an enum used for
return address mangling on AArch64.

gcc/ChangeLog:

* config/aarch64/aarch64.cc
(aarch64_output_cfi_directive): New hook for CFI directives.
(aarch64_dw_cfi_oprnd1_desc): Same.
(TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive.
(TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc.
* config/sparc/sparc.cc
(sparc_output_cfi_directive): New hook for CFI directives.
(sparc_dw_cfi_oprnd1_desc): Same.
(TARGET_OUTPUT_CFI_DIRECTIVE): Hook for output_cfi_directive.
(TARGET_DW_CFI_OPRND1_DESC): Hook for dw_cfi_oprnd1_desc.
* coretypes.h
(struct dw_cfi_node): Forward declaration of CFI type from
gcc/dwarf2out.h.
(enum dw_cfi_oprnd_type): Same.
(enum dwarf_call_frame_info): Same.
* doc/tm.texi: Regenerated from doc/tm.texi.in.
* doc/tm.texi.in: Add doc for new target hooks.
* dwarf2.h (enum dwarf_call_frame_info): specify underlying
type of enum to allow forward declaration.
* dwarf2cfi.cc
(struct dw_cfi_row): Update the description for window_save
and ra_mangled.
(dwarf2out_frame_debug_cfa_negate_ra_state): Use AArch64 CFI
directive instead of the SPARC one.
(change_cfi_row): Use the right CFI directive's name for RA
mangling.
(output_cfi): Remove explicit architecture-specific CFI
directive DW_CFA_GNU_window_save that falls into default case.
(output_cfi_directive): Use target hook as default.
* dwarf2out.cc (dw_cfi_oprnd1_desc): Use target hook as default.
* dwarf2out.h (enum dw_cfi_oprnd_type): specify underlying type
of enum to allow forward declaration.
(dw_cfi_oprnd1_desc): Call target hook.
(output_cfi_directive): Use dw_cfi_ref instead of struct
dw_cfi_node *.
* hooks.cc
(hook_bool_dwcfi_dwcfioprndtyperef_false): New.
(hook_bool_FILEptr_dwcfiptr_false): New.
* hooks.h
(hook_bool_dwcfi_dwcfioprndtyperef_false): New.
(hook_bool_FILEptr_dwcfiptr_false): New.
* target.def: Documentation for new hooks.

libffi/ChangeLog:

* include/ffi_cfi.h (cfi_negate_ra_state): Declare AArch64 cfi
directive.

libgcc/ChangeLog:

* config/aarch64/aarch64-asm.h (PACIASP): Replace SPARC CFI
directive by AArch64 one.
(AUTIASP): Same.

libitm/ChangeLog:

* config/aarch64/sjlj.S: Replace SPARC CFI directive by
AArch64 one.

gcc/testsuite/ChangeLog:

* g++.target/aarch64/pr94515-1.C: Replace SPARC CFI directive by
AArch64 one.
* g++.target/aarch64/pr94515-2.C: Same.
---
 gcc/config/aarch64/aarch64.cc| 33 ++
 gcc/config/sparc/sparc.cc| 35 
 gcc/coretypes.h  |  6 
 gcc/doc/tm.texi  | 16 -
 gcc/doc/tm.texi.in   |  5 ++-
 gcc/dwarf2cfi.cc | 31 ++---
 gcc/dwarf2out.cc | 13 +---
 gcc/dwarf2out.h  | 11 +++---
 gcc/hooks.cc | 14 
 gcc/hooks.h  |  3 ++
 gcc/target.def   | 20 +++
 gcc/testsuite/g++.target/aarch64/pr94515-1.C |  6 ++--
 gcc/testsuite/g++.target/aarch64/pr94515-2.C |  2 +-
 include/dwarf2.h |  5 +++
 libffi/include/ffi_cfi.h |  2 ++
 libgcc/config/aarch64/aarch64-asm.h  |  4 +--
 libitm/config/aarch64/sjlj.S | 10 +++---
 17 files changed, 171 insertions(+), 45 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch6

[PATCH v2 0/4] dwarf2: add hooks for architecture-specific CFIs

[Matthieu Longo]

Architecture-specific CFI directives are currently declared an processed among 
others architecture-independent CFI directives in gcc/dwarf2* files. This 
approach creates confusion, specifically in the case of DWARF instructions in 
the vendor space and using the same instruction code.
Such a clash currently happen between DW_CFA_GNU_window_save (used on SPARC) 
and DW_CFA_AARCH64_negate_ra_state (used on AArch64), and both having the same 
instruction code 0x2d. Then AArch64 compilers generates a SPARC CFI directive 
(.cfi_window_save) instead of .cfi_negate_ra_state, contrarily to what is 
expected in [1].

1. Rename REG_CFA_TOGGLE_RA_MANGLE to REG_CFA_NEGATE_RA_STATE

This patch renames:
- dwarf2out_frame_debug_cfa_toggle_ra_mangle to 
dwarf2out_frame_debug_cfa_negate_ra_state,
- REG_CFA_TOGGLE_RA_MANGLE to REG_CFA_NEGATE_RA_STATE,
as the naming was misleading.
The word "toggle" suggested a binary state, whereas this register stores the 
mangling state (that can be more than 2 states) for the return address on 
AArch64.

2. dwarf2: add hooks for architecture-specific CFIs

This refactoring does not solve completely the problem, but improve the 
situation by moving some of the processing of those directives (more 
specifically their output in the assembly) to the backend via 2 target hooks:
- DW_CFI_OPRND1_DESC: parse the first operand of the directive (if any).
- OUTPUT_CFI_DIRECTIVE: output the CFI directive as a string.
Only AArch64's and SPARC's backend are impacted.

3. aarch64 testsuite: explain expectections for pr94515*
PR94515's tests in AArch64 G++ testsuite were lacking documentation. They are 
now thoroughly documented.

4. dwarf2: store the RA state in CFI row

On AArch64, the RA state informs the unwinder whether the return address is 
mangled and how, or not. This information is encoded in a boolean in the CFI 
row. This binary approach prevents from expressing more complex configuration, 
as it is the case with PAuth_LR introduced in Armv9.5-A.
This patch addresses this limitation by replacing the boolean by an enum.


References:
[1] DWARF for the Arm 64-bit Architecture (AArch64) --> 
https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst

### Diff between revisions 1 & 2

patch 1: no change.
patch 2:
  - fix issue with default hook signature for new hooks.
  - code style formatting.
  - change targetm.dw_cfi_oprnd1_desc signature to match the one of 
dw_cfi_oprnd2_desc.
patch 3:
  - add comments suggested by Richard Sandiford in test case 
gcc/testsuite/g++.target
/aarch64/pr94515-2.C.
patch 4:
  - code style formatting.

## Testing

Built for target aarch64-unknown-linux-gnu and ran GCC's & G++'s testsuites for 
AArch64.
Built GCC stage 1 for target sparc64-unknown-linux-gnu.


Ok for master? I don't have commit access so I need someone to commit on my 
behalf.

Regards,
Matthieu.

Matthieu Longo (4):
  Rename REG_CFA_TOGGLE_RA_MANGLE to REG_CFA_NEGATE_RA_STATE
  dwarf2: add hooks for architecture-specific CFIs
  aarch64 testsuite: explain expectections for pr94515* tests
  dwarf2: store the RA state in CFI row

 gcc/combine-stack-adj.cc |  2 +-
 gcc/config/aarch64/aarch64.cc| 37 +++-
 gcc/config/sparc/sparc.cc| 35 
 gcc/coretypes.h  |  6 ++
 gcc/doc/tm.texi  | 16 +-
 gcc/doc/tm.texi.in   |  5 +-
 gcc/dwarf2cfi.cc | 59 ++--
 gcc/dwarf2out.cc | 13 +++--
 gcc/dwarf2out.h  | 11 ++--
 gcc/hooks.cc | 14 +
 gcc/hooks.h  |  3 +
 gcc/reg-notes.def|  8 +--
 gcc/target.def   | 20 +++
 gcc/testsuite/g++.target/aarch64/pr94515-1.C | 14 -
 gcc/testsuite/g++.target/aarch64/pr94515-2.C | 41 +++---
 include/dwarf2.h |  5 ++
 libffi/include/ffi_cfi.h |  2 +
 libgcc/config/aarch64/aarch64-asm.h  |  4 +-
 libitm/config/aarch64/sjlj.S | 10 ++--
 19 files changed, 239 insertions(+), 66 deletions(-)

-- 
2.46.0

[PATCH v2 3/4] aarch64 testsuite: explain expectections for pr94515* tests

[Matthieu Longo]

gcc/testsuite/ChangeLog:

* g++.target/aarch64/pr94515-1.C: Improve test documentation.
* g++.target/aarch64/pr94515-2.C: Same.
---
 gcc/testsuite/g++.target/aarch64/pr94515-1.C |  8 
 gcc/testsuite/g++.target/aarch64/pr94515-2.C | 39 +---
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-1.C 
b/gcc/testsuite/g++.target/aarch64/pr94515-1.C
index d5c114a83a8..359039e1753 100644
--- a/gcc/testsuite/g++.target/aarch64/pr94515-1.C
+++ b/gcc/testsuite/g++.target/aarch64/pr94515-1.C
@@ -15,12 +15,20 @@ void unwind (void)
 __attribute__((noinline, noipa, target("branch-protection=pac-ret")))
 int test (int z)
 {
+  // paciasp -> cfi_negate_ra_state: RA_no_signing -> RA_signing_SP
   if (z) {
 asm volatile ("":::"x20","x21");
 unwind ();
+// autiasp -> cfi_negate_ra_state: RA_signing_SP -> RA_no_signing
 return 1;
   } else {
+// 2nd cfi_negate_ra_state because the CFI directives are processed 
linearily.
+// At this point, the unwinder would believe that the address is not signed
+// due to the previous return. That's why the compiler has to emit second
+// cfi_negate_ra_state to mean that the return address is still signed.
+// cfi_negate_ra_state: RA_no_signing -> RA_signing_SP
 unwind ();
+// autiasp -> cfi_negate_ra_state: RA_signing_SP -> RA_no_signing
 return 2;
   }
 }
diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-2.C 
b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
index f4abeed..bdb65411a08 100644
--- a/gcc/testsuite/g++.target/aarch64/pr94515-2.C
+++ b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
@@ -6,6 +6,7 @@
 volatile int zero = 0;
 int global = 0;
 
+/* This is a leaf function, so no .cfi_negate_ra_state directive is expected.  
*/
 __attribute__((noinline))
 int bar(void)
 {
@@ -13,29 +14,55 @@ int bar(void)
   return 0;
 }
 
+/* This function does not return normally, so the address is signed but no
+ * authentication code is emitted. It means that only one CFI directive is
+ * supposed to be emitted at signing time.  */
 __attribute__((noinline, noreturn))
 void unwind (void)
 {
   throw 42;
 }
 
+/* This function has several return instructions, and alternates different RA
+ * states. 4 .cfi_negate_ra_state and a .cfi_remember_state/.cfi_restore_state
+ * should be emitted.
+ *
+ * Expected layout:
+ *   A: path to return 0 without assignment to global
+ *   B: global=y + branch back into A
+ *   C: return 2
+ *   D: unwind
+ * Which gives with return pointer authentication:
+ *   A: sign -> authenticate [2 negate_ra_states + remember_state for B]
+ *   B: signed [restore_state]
+ *   C: unsigned [negate_ra_state]
+ *   D: signed [negate_ra_state]
+ */
 __attribute__((noinline, noipa))
 int test(int x)
 {
-  if (x==1) return 2; /* This return path may not use the stack.  */
+  // This return path may not use the stack. This means that the return address
+  // won't be signed.
+  if (x==1) return 2;
+
+  // All the return paths of the code below must have RA mangle state set, and
+  // the return address must be signed.
   int y = bar();
   if (y > global) global=y;
-  if (y==3) unwind(); /* This return path must have RA mangle state set.  */
-  return 0;
+  if (y==3) unwind(); // authentication of the return address is not required.
+  return 0; // authentication of the return address is required.
 }
 
+/* This function requires only 2 .cfi_negate_ra_state.  */
 int main ()
 {
+  // paciasp -> cfi_negate_ra_state: RA_no_signing -> RA_signing_SP
   try {
 test (zero);
-__builtin_abort ();
+__builtin_abort (); // authentication of the return address is not 
required.
   } catch (...) {
+// autiasp -> cfi_negate_ra_state: RA_signing_SP -> RA_no_signing
 return 0;
   }
-  __builtin_abort ();
-}
+  __builtin_abort (); // authentication of the return address is not required.
+}
\ No newline at end of file
-- 
2.46.0

[PATCH v2 4/4] dwarf2: store the RA state in CFI row

[Matthieu Longo]

On AArch64, the RA state informs the unwinder whether the return address
is mangled and how, or not. This information is encoded in a boolean in
the CFI row. This binary approach prevents from expressing more complex
configuration, as it is the case with PAuth_LR introduced in Armv9.5-A.

This patch addresses this limitation by replacing the boolean by an enum.

gcc/ChangeLog:

* dwarf2cfi.cc
(struct dw_cfi_row): Declare a new enum type to replace ra_mangled.
(cfi_row_equal_p): Use ra_state instead of ra_mangled.
(dwarf2out_frame_debug_cfa_negate_ra_state): Same.
(change_cfi_row): Same.
---
 gcc/dwarf2cfi.cc | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/gcc/dwarf2cfi.cc b/gcc/dwarf2cfi.cc
index f8d19d52429..1b94185a496 100644
--- a/gcc/dwarf2cfi.cc
+++ b/gcc/dwarf2cfi.cc
@@ -57,6 +57,15 @@ along with GCC; see the file COPYING3.  If not see
 #define DEFAULT_INCOMING_FRAME_SP_OFFSET INCOMING_FRAME_SP_OFFSET
 #endif
 
+
+/* Signing method used for return address authentication.
+   (AArch64 extension)  */
+typedef enum
+{
+  ra_no_signing = 0x0,
+  ra_signing_sp = 0x1,
+} ra_signing_method_t;
+
 /* A collected description of an entire row of the abstract CFI table.  */
 struct GTY(()) dw_cfi_row
 {
@@ -74,8 +83,8 @@ struct GTY(()) dw_cfi_row
   bool window_save;
 
   /* AArch64 extension for DW_CFA_AARCH64_negate_ra_state.
- True if the return address is in a mangled state.  */
-  bool ra_mangled;
+ Enum which stores the return address state.  */
+  ra_signing_method_t ra_state;
 };
 
 /* The caller's ORIG_REG is saved in SAVED_IN_REG.  */
@@ -857,7 +866,7 @@ cfi_row_equal_p (dw_cfi_row *a, dw_cfi_row *b)
   if (a->window_save != b->window_save)
 return false;
 
-  if (a->ra_mangled != b->ra_mangled)
+  if (a->ra_state != b->ra_state)
 return false;
 
   return true;
@@ -1554,8 +1563,11 @@ dwarf2out_frame_debug_cfa_negate_ra_state (void)
 {
   dw_cfi_ref cfi = new_cfi ();
   cfi->dw_cfi_opc = DW_CFA_AARCH64_negate_ra_state;
+  cur_row->ra_state
+= (cur_row->ra_state == ra_no_signing
+  ? ra_signing_sp
+  : ra_no_signing);
   add_cfi (cfi);
-  cur_row->ra_mangled = !cur_row->ra_mangled;
 }
 
 /* Record call frame debugging information for an expression EXPR,
@@ -2412,12 +2424,12 @@ change_cfi_row (dw_cfi_row *old_row, dw_cfi_row 
*new_row)
 {
   dw_cfi_ref cfi = new_cfi ();
 
-  gcc_assert (!old_row->ra_mangled && !new_row->ra_mangled);
+  gcc_assert (!old_row->ra_state && !new_row->ra_state);
   cfi->dw_cfi_opc = DW_CFA_GNU_window_save;
   add_cfi (cfi);
 }
 
-  if (old_row->ra_mangled != new_row->ra_mangled)
+  if (old_row->ra_state != new_row->ra_state)
 {
   dw_cfi_ref cfi = new_cfi ();
 
-- 
2.46.0

[PATCH v1] autoupdate: replace obsolete macros in libiberty

[Matthieu Longo]

Autoreconf-2.72 warns about obsolete macros. This patch aims at removing
the noise from a future upgrade to autoreconf-2.72 or later. This is in
no a way a complete patch allowing the upgrade to autoreconf-2.72.

- AC_GNU_SOURCE by AC_USE_SYSTEM_EXTENSIONS
  https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.72/
  autoconf.html#index-AC_005fGNU_005fSOURCE-1
- AC_CONFIG_HEADER by AC_CONFIG_HEADERS
  https://www.gnu.org/software/automake/manual/1.12.2/html_node/Obsolete-
  Macros.html#index-AM_005fCONFIG_005fHEADER

Those fixes were originally submitted in a patch series in binutils.
https://inbox.sourceware.org/binutils/878qthm6a0@gentoo.org/

libiberty/ChangeLog:

* configure: Regenerate.
* configure.ac: Fix autoupdate warnings.
---
 libiberty/configure| 1 -
 libiberty/configure.ac | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/libiberty/configure b/libiberty/configure
index 5c69fee56c1..38856a07e5f 100755
--- a/libiberty/configure
+++ b/libiberty/configure
@@ -4413,7 +4413,6 @@ $as_echo "$ac_cv_safe_to_define___extensions__" >&6; }
   $as_echo "#define _TANDEM_SOURCE 1" >>confdefs.h
 
 
-
 # Check whether --enable-largefile was given.
 if test "${enable_largefile+set}" = set; then :
   enableval=$enable_largefile;
diff --git a/libiberty/configure.ac b/libiberty/configure.ac
index 0888e638896..c27e08e1428 100644
--- a/libiberty/configure.ac
+++ b/libiberty/configure.ac
@@ -172,7 +172,7 @@ AC_MSG_NOTICE([target_header_dir = $target_header_dir])
 
 GCC_NO_EXECUTABLES
 AC_PROG_CC
-AC_GNU_SOURCE
+AC_USE_SYSTEM_EXTENSIONS
 AC_SYS_LARGEFILE
 AC_PROG_CPP_WERROR
 
@@ -205,7 +205,7 @@ dnl AM_PROG_LIBTOOL
 
 dnl When we start using automake:
 dnl AM_CONFIG_HEADER(config.h:config.in)
-AC_CONFIG_HEADER(config.h:config.in)
+AC_CONFIG_HEADERS([config.h:config.in])
 
 dnl When we start using automake:
 dnl AM_MAINTAINER_MODE
-- 
2.47.0

[PATCH] MAINTAINERS: add myself to write after approval

[Matthieu Longo]

ChangeLog:

* MAINTAINERS: Add myself to write after approval.
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7d65ed64bdd..44153a7a51e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -630,6 +630,7 @@ Sa Liu  saliu   

 Ralph Loaderralph   
 Sheldon Lobosmlobo  
 Gabor Loki  loki
+Matthieu Longo  mlongo  
 Sandra Loosemoresandra  
 Manuel López-Ibáñez manu
 Carl Love   carll   
-- 
2.47.1