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

Message ID 20240918135506.1534663-4-matthieu.longo@arm.com
State New
Headers
Series store signing key and signing method in DWARF _Unwind_FrameState |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Build failed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Build failed

Commit Message

Matthieu Longo Sept. 18, 2024, 1:55 p.m. UTC
  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
  

Patch

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_header	The 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_header	The 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*-*-freebsd*)
 	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/freebsd-unwind.h
 	;;
 aarch64*-*-netbsd*)
@@ -418,6 +423,7 @@  aarch64*-*-netbsd*)
 	tmake_file="${tmake_file} ${cpu_type}/t-aarch64"
 	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*-*-fuchsia*)
@@ -428,6 +434,7 @@  aarch64*-*-fuchsia*)
 	;;
 aarch64*-*-linux*)
 	extra_parts="$extra_parts crtfastmath.o"
+	md_unwind_def_header=aarch64/aarch64-unwind-def.h
 	md_unwind_header=aarch64/linux-unwind.h
 	tmake_file="${tmake_file} ${cpu_type}/t-aarch64"
 	tmake_file="${tmake_file} ${cpu_type}/t-lse t-slibgcc-libgcc"
@@ -437,6 +444,7 @@  aarch64*-*-linux*)
 	;;
 aarch64*-*-gnu*)
 	extra_parts="$extra_parts crtfastmath.o"
+	md_unwind_def_header=aarch64/aarch64-unwind-def.h
 	md_unwind_header=aarch64/gnu-unwind.h
 	tmake_file="${tmake_file} ${cpu_type}/t-aarch64"
 	tmake_file="${tmake_file} ${cpu_type}/t-lse t-slibgcc-libgcc"
@@ -446,6 +454,7 @@  aarch64*-*-gnu*)
 	;;
 aarch64*-*-vxworks7*)
 	extra_parts="$extra_parts crtfastmath.o"
+	md_unwind_def_header=aarch64/aarch64-unwind-def.h
 	md_unwind_header=aarch64/aarch64-unwind.h
 	tmake_file="${tmake_file} ${cpu_type}/t-aarch64"
 	tmake_file="${tmake_file} ${cpu_type}/t-lse"
diff --git a/libgcc/config/aarch64/aarch64-unwind-def.h b/libgcc/config/aarch64/aarch64-unwind-def.h
new file mode 100644
index 00000000000..c84ec3edb46
--- /dev/null
+++ b/libgcc/config/aarch64/aarch64-unwind-def.h
@@ -0,0 +1,41 @@ 
+/* 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+<http://www.gnu.org/licenses/>.  */
+
+#if !defined (AARCH64_UNWIND_DEF_H) && !defined (__ILP32__)
+#define AARCH64_UNWIND_DEF_H
+
+/* The key used to sign a function's return address.  */
+typedef enum {
+  AARCH64_PAUTH_KEY_A,
+  AARCH64_PAUTH_KEY_B,
+} __attribute__((packed)) aarch64_pointer_auth_key;
+
+typedef struct
+{
+  aarch64_pointer_auth_key signing_key;
+} _AArch64Ext_Unwind_FrameState;
+
+#define MD_ARCH_FRAME_STATE_T _AArch64Ext_Unwind_FrameState
+
+#endif /* defined AARCH64_UNWIND_DEF_H && defined __ILP32__ */
diff --git a/libgcc/config/aarch64/aarch64-unwind.h b/libgcc/config/aarch64/aarch64-unwind.h
index 52bfd540979..2b774eb263c 100644
--- a/libgcc/config/aarch64/aarch64-unwind.h
+++ b/libgcc/config/aarch64/aarch64-unwind.h
@@ -25,6 +25,8 @@  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 "ansidecl.h"
 #include <stdbool.h>
 
@@ -38,12 +40,6 @@  typedef enum
   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_KEY_A,
-  AARCH64_PAUTH_KEY_B,
-} __attribute__((packed)) aarch64_pointer_auth_key;
-
 #define MD_ARCH_EXTENSION_CIE_AUG_HANDLER(fs, aug) \
   aarch64_cie_aug_handler (fs, aug)
 
@@ -100,7 +96,7 @@  aarch64_cie_aug_handler (_Unwind_FrameState *fs, unsigned char aug)
   /* AArch64 B-key pointer authentication.  */
   if (aug == 'B')
     {
-      fs->regs.signing_key = AARCH64_PAUTH_KEY_B;
+      fs->regs.arch_fs.signing_key = AARCH64_PAUTH_KEY_B;
       return true;
     }
   return false;
@@ -119,7 +115,7 @@  aarch64_arch_extension_frame_init (struct _Unwind_Context *context ATTRIBUTE_UNU
   /* By default, DW_CFA_AARCH64_negate_ra_state assumes key A is being used
      for signing.  This can be overridden by adding 'B' to the augmentation
      string.  */
-  fs->regs.signing_key = AARCH64_PAUTH_KEY_A;
+  fs->regs.arch_fs.signing_key = AARCH64_PAUTH_KEY_A;
 
   /* All registers are initially in state REG_UNSAVED, which indicates that
      they inherit register values from the previous frame.  However, the
@@ -174,7 +170,7 @@  aarch64_demangle_return_addr (struct _Unwind_Context *context,
   if (signing_method == aarch64_ra_signing_sp)
     {
       _Unwind_Word salt = (_Unwind_Word) context->cfa;
-      if (fs->regs.signing_key == AARCH64_PAUTH_KEY_B)
+      if (fs->regs.arch_fs.signing_key == AARCH64_PAUTH_KEY_B)
 	return __builtin_aarch64_autib1716 (addr, salt);
       return __builtin_aarch64_autia1716 (addr, salt);
     }
diff --git a/libgcc/config/no-unwind.h b/libgcc/config/no-unwind.h
index 0ecd78a60de..7400fd4d2f2 100644
--- a/libgcc/config/no-unwind.h
+++ b/libgcc/config/no-unwind.h
@@ -1,2 +1,3 @@ 
 /* Dummy header for targets without a definition of
-   MD_FALLBACK_FRAME_STATE_FOR.  */
+   architecture-specific frame information types, or
+   handlers used in the unwinder.  */
diff --git a/libgcc/configure.ac b/libgcc/configure.ac
index c2749fe0958..ca341479177 100644
--- a/libgcc/configure.ac
+++ b/libgcc/configure.ac
@@ -727,6 +727,7 @@  AC_SUBST(extra_parts)
 AC_SUBST(asm_hidden_op)
 AC_SUBST(enable_execute_stack)
 AC_SUBST(unwind_header)
+AC_SUBST(md_unwind_def_header)
 AC_SUBST(md_unwind_header)
 AC_SUBST(sfp_machine_header)
 AC_SUBST(thread_header)
diff --git a/libgcc/unwind-dw2.h b/libgcc/unwind-dw2.h
index e6b30b7188e..8e3bdbd27ad 100644
--- a/libgcc/unwind-dw2.h
+++ b/libgcc/unwind-dw2.h
@@ -22,6 +22,8 @@ 
    see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include "md-unwind-def.h"
+
 enum register_rule
 {
   REG_UNSAVED,
@@ -71,8 +73,8 @@  typedef struct
        Note: this information has to be saved in struct frame_state_reg_info
        instead of _Unwind_FrameState as DW_CFA_restore_state has to be able to
        restore them.  */
-#if defined(__aarch64__) && !defined (__ILP32__)
-    unsigned char signing_key;
+#if defined(MD_ARCH_FRAME_STATE_T)
+    MD_ARCH_FRAME_STATE_T arch_fs;
 #endif
   } regs;