release/19.x: [libunwind] Add GCS support for AArch64 (#99335) #101888

llvmbot · 2024-08-04T13:01:23Z

Backport b32aac4

Requested by: @john-brawn-arm

llvmbot · 2024-08-04T13:01:28Z

@jinge90 What do you think about merging this PR to the release branch?

llvmbot · 2024-08-04T13:01:56Z

@llvm/pr-subscribers-libunwind

Author: None (llvmbot)

Changes

Backport b32aac4

Requested by: @john-brawn-arm

Full diff: https://github.com/llvm/llvm-project/pull/101888.diff

10 Files Affected:

(modified) libunwind/CMakeLists.txt (+8)
(modified) libunwind/src/Registers.hpp (+7)
(modified) libunwind/src/UnwindCursor.hpp (+3-3)
(modified) libunwind/src/UnwindLevel1.c (+28-3)
(modified) libunwind/src/UnwindRegistersRestore.S (+1-1)
(modified) libunwind/src/cet_unwind.h (+18)
(modified) libunwind/test/CMakeLists.txt (+1)
(modified) libunwind/test/configs/llvm-libunwind-merged.cfg.in (+3)
(modified) libunwind/test/configs/llvm-libunwind-shared.cfg.in (+3)
(modified) libunwind/test/configs/llvm-libunwind-static.cfg.in (+3)

diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index b22ade0a7d71e..28d67b0fef92c 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -37,6 +37,7 @@ if (LIBUNWIND_BUILD_32_BITS)
 endif()
 
 option(LIBUNWIND_ENABLE_CET "Build libunwind with CET enabled." OFF)
+option(LIBUNWIND_ENABLE_GCS "Build libunwind with GCS enabled." OFF)
 option(LIBUNWIND_ENABLE_ASSERTIONS "Enable assertions independent of build mode." ON)
 option(LIBUNWIND_ENABLE_PEDANTIC "Compile with pedantic enabled." ON)
 option(LIBUNWIND_ENABLE_WERROR "Fail and stop if a warning is triggered." OFF)
@@ -188,6 +189,13 @@ if (LIBUNWIND_ENABLE_CET)
   endif()
 endif()
 
+if (LIBUNWIND_ENABLE_GCS)
+  add_compile_flags_if_supported(-mbranch-protection=standard)
+  if (NOT CXX_SUPPORTS_MBRANCH_PROTECTION_EQ_STANDARD_FLAG)
+    message(SEND_ERROR "Compiler doesn't support GCS -mbranch-protection option!")
+  endif()
+endif()
+
 if (WIN32)
   # The headers lack matching dllexport attributes (_LIBUNWIND_EXPORT);
   # silence the warning instead of cluttering the headers (which aren't
diff --git a/libunwind/src/Registers.hpp b/libunwind/src/Registers.hpp
index d11ddb3426d52..861e6b5f6f2c5 100644
--- a/libunwind/src/Registers.hpp
+++ b/libunwind/src/Registers.hpp
@@ -1815,6 +1815,13 @@ inline const char *Registers_ppc64::getRegisterName(int regNum) {
 /// process.
 class _LIBUNWIND_HIDDEN Registers_arm64;
 extern "C" void __libunwind_Registers_arm64_jumpto(Registers_arm64 *);
+
+#if defined(_LIBUNWIND_USE_GCS)
+extern "C" void *__libunwind_cet_get_jump_target() {
+  return reinterpret_cast<void *>(&__libunwind_Registers_arm64_jumpto);
+}
+#endif
+
 class _LIBUNWIND_HIDDEN Registers_arm64 {
 public:
   Registers_arm64();
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 758557337899e..06e654197351d 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -471,7 +471,7 @@ class _LIBUNWIND_HIDDEN AbstractUnwindCursor {
   }
 #endif
 
-#if defined(_LIBUNWIND_USE_CET)
+#if defined(_LIBUNWIND_USE_CET) || defined(_LIBUNWIND_USE_GCS)
   virtual void *get_registers() {
     _LIBUNWIND_ABORT("get_registers not implemented");
   }
@@ -954,7 +954,7 @@ class UnwindCursor : public AbstractUnwindCursor{
   virtual uintptr_t getDataRelBase();
 #endif
 
-#if defined(_LIBUNWIND_USE_CET)
+#if defined(_LIBUNWIND_USE_CET) || defined(_LIBUNWIND_USE_GCS)
   virtual void *get_registers() { return &_registers; }
 #endif
 
@@ -3005,7 +3005,7 @@ bool UnwindCursor<A, R>::isReadableAddr(const pint_t addr) const {
 }
 #endif
 
-#if defined(_LIBUNWIND_USE_CET)
+#if defined(_LIBUNWIND_USE_CET) || defined(_LIBUNWIND_USE_GCS)
 extern "C" void *__libunwind_cet_get_registers(unw_cursor_t *cursor) {
   AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
   return co->get_registers();
diff --git a/libunwind/src/UnwindLevel1.c b/libunwind/src/UnwindLevel1.c
index 48e7bc3b9e00e..7e785f4d31e71 100644
--- a/libunwind/src/UnwindLevel1.c
+++ b/libunwind/src/UnwindLevel1.c
@@ -44,7 +44,7 @@
 // _LIBUNWIND_POP_CET_SSP is used to adjust CET shadow stack pointer and we
 // directly jump to __libunwind_Registers_x86/x86_64_jumpto instead of using
 // a regular function call to avoid pushing to CET shadow stack again.
-#if !defined(_LIBUNWIND_USE_CET)
+#if !defined(_LIBUNWIND_USE_CET) && !defined(_LIBUNWIND_USE_GCS)
 #define __unw_phase2_resume(cursor, fn)                                        \
   do {                                                                         \
     (void)fn;                                                                  \
@@ -72,6 +72,19 @@
     __asm__ volatile("jmpq *%%rdx\n\t" :: "D"(cetRegContext),                  \
                      "d"(cetJumpAddress));                                     \
   } while (0)
+#elif defined(_LIBUNWIND_TARGET_AARCH64)
+#define __cet_ss_step_size 8
+#define __unw_phase2_resume(cursor, fn)                                        \
+  do {                                                                         \
+    _LIBUNWIND_POP_CET_SSP((fn));                                              \
+    void *cetRegContext = __libunwind_cet_get_registers((cursor));             \
+    void *cetJumpAddress = __libunwind_cet_get_jump_target();                  \
+    __asm__ volatile("mov x0, %0\n\t"                                          \
+                     "br %1\n\t"                                               \
+                     :                                                         \
+                     : "r"(cetRegContext), "r"(cetJumpAddress)                 \
+                     : "x0");                                                  \
+  } while (0)
 #endif
 
 static _Unwind_Reason_Code
@@ -170,6 +183,10 @@ unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, _Unwind_Exception *except
 }
 extern int __unw_step_stage2(unw_cursor_t *);
 
+#if defined(_LIBUNWIND_USE_GCS)
+// Enable the GCS target feature to permit gcspop instructions to be used.
+__attribute__((target("gcs")))
+#endif
 static _Unwind_Reason_Code
 unwind_phase2(unw_context_t *uc, unw_cursor_t *cursor, _Unwind_Exception *exception_object) {
   __unw_init_local(cursor, uc);
@@ -180,8 +197,12 @@ unwind_phase2(unw_context_t *uc, unw_cursor_t *cursor, _Unwind_Exception *except
   // uc is initialized by __unw_getcontext in the parent frame. The first stack
   // frame walked is unwind_phase2.
   unsigned framesWalked = 1;
-#ifdef _LIBUNWIND_USE_CET
+#if defined(_LIBUNWIND_USE_CET)
   unsigned long shadowStackTop = _get_ssp();
+#elif defined(_LIBUNWIND_USE_GCS)
+  unsigned long shadowStackTop = 0;
+  if (__chkfeat(_CHKFEAT_GCS))
+    shadowStackTop = (unsigned long)__gcspr();
 #endif
   // Walk each frame until we reach where search phase said to stop.
   while (true) {
@@ -238,7 +259,7 @@ unwind_phase2(unw_context_t *uc, unw_cursor_t *cursor, _Unwind_Exception *except
 // against return address stored in CET shadow stack, if the 2 addresses don't
 // match, it means return address in normal stack has been corrupted, we return
 // _URC_FATAL_PHASE2_ERROR.
-#ifdef _LIBUNWIND_USE_CET
+#if defined(_LIBUNWIND_USE_CET) || defined(_LIBUNWIND_USE_GCS)
     if (shadowStackTop != 0) {
       unw_word_t retInNormalStack;
       __unw_get_reg(cursor, UNW_REG_IP, &retInNormalStack);
@@ -306,6 +327,10 @@ unwind_phase2(unw_context_t *uc, unw_cursor_t *cursor, _Unwind_Exception *except
   return _URC_FATAL_PHASE2_ERROR;
 }
 
+#if defined(_LIBUNWIND_USE_GCS)
+// Enable the GCS target feature to permit gcspop instructions to be used.
+__attribute__((target("gcs")))
+#endif
 static _Unwind_Reason_Code
 unwind_phase2_forced(unw_context_t *uc, unw_cursor_t *cursor,
                      _Unwind_Exception *exception_object,
diff --git a/libunwind/src/UnwindRegistersRestore.S b/libunwind/src/UnwindRegistersRestore.S
index 67d9e05711898..e1d6e17549880 100644
--- a/libunwind/src/UnwindRegistersRestore.S
+++ b/libunwind/src/UnwindRegistersRestore.S
@@ -680,7 +680,7 @@ DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_arm64_jumpto)
   ldr    x16,     [x0, #0x0F8]
   ldp    x0, x1,  [x0, #0x000]  // restore x0,x1
   mov    sp,x16                 // restore sp
-  ret    x30                    // jump to pc
+  br     x30                    // jump to pc
 
 #elif defined(__arm__) && !defined(__APPLE__)
 
diff --git a/libunwind/src/cet_unwind.h b/libunwind/src/cet_unwind.h
index c364ed3e12feb..45c11973cb7fa 100644
--- a/libunwind/src/cet_unwind.h
+++ b/libunwind/src/cet_unwind.h
@@ -35,6 +35,24 @@
   } while (0)
 #endif
 
+// On AArch64 we use _LIBUNWIND_USE_GCS to indicate that GCS is supported. We
+// need to guard any use of GCS instructions with __chkfeat though, as GCS may
+// not be enabled.
+#if defined(_LIBUNWIND_TARGET_AARCH64) && defined(__ARM_FEATURE_GCS_DEFAULT)
+#define _LIBUNWIND_USE_GCS 1
+#include <arm_acle.h>
+
+#define _LIBUNWIND_POP_CET_SSP(x)                                              \
+  do {                                                                         \
+    if (__chkfeat(_CHKFEAT_GCS)) {                                             \
+      unsigned tmp = (x);                                                      \
+      while (tmp--)                                                            \
+        __gcspopm();                                                           \
+    }                                                                          \
+  } while (0)
+
+#endif
+
 extern void *__libunwind_cet_get_registers(unw_cursor_t *);
 extern void *__libunwind_cet_get_jump_target(void);
 
diff --git a/libunwind/test/CMakeLists.txt b/libunwind/test/CMakeLists.txt
index 19f055f6f93ff..c7b1b3d01d8c7 100644
--- a/libunwind/test/CMakeLists.txt
+++ b/libunwind/test/CMakeLists.txt
@@ -9,6 +9,7 @@ macro(pythonize_bool var)
 endmacro()
 
 pythonize_bool(LIBUNWIND_ENABLE_CET)
+pythonize_bool(LIBUNWIND_ENABLE_GCS)
 pythonize_bool(LIBUNWIND_ENABLE_THREADS)
 pythonize_bool(LIBUNWIND_USES_ARM_EHABI)
 
diff --git a/libunwind/test/configs/llvm-libunwind-merged.cfg.in b/libunwind/test/configs/llvm-libunwind-merged.cfg.in
index 38b79840c9fe2..fafe12962b428 100644
--- a/libunwind/test/configs/llvm-libunwind-merged.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-merged.cfg.in
@@ -11,6 +11,9 @@ link_flags = []
 if @LIBUNWIND_ENABLE_CET@:
     compile_flags.append('-fcf-protection=full')
 
+if @LIBUNWIND_ENABLE_GCS@:
+    compile_flags.append('-mbranch-protection=standard')
+
 # On ELF platforms, link tests with -Wl,--export-dynamic if supported by the linker.
 if len('@CMAKE_EXE_EXPORTS_CXX_FLAG@'):
     link_flags.append('@CMAKE_EXE_EXPORTS_CXX_FLAG@')
diff --git a/libunwind/test/configs/llvm-libunwind-shared.cfg.in b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
index 13896aeb13bc4..f3e40928b525d 100644
--- a/libunwind/test/configs/llvm-libunwind-shared.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
@@ -10,6 +10,9 @@ link_flags = []
 if @LIBUNWIND_ENABLE_CET@:
     compile_flags.append('-fcf-protection=full')
 
+if @LIBUNWIND_ENABLE_GCS@:
+    compile_flags.append('-mbranch-protection=standard')
+
 # On ELF platforms, link tests with -Wl,--export-dynamic if supported by the linker.
 if len('@CMAKE_EXE_EXPORTS_CXX_FLAG@'):
     link_flags.append('@CMAKE_EXE_EXPORTS_CXX_FLAG@')
diff --git a/libunwind/test/configs/llvm-libunwind-static.cfg.in b/libunwind/test/configs/llvm-libunwind-static.cfg.in
index 50b64dc665a5a..a3a65ae82591b 100644
--- a/libunwind/test/configs/llvm-libunwind-static.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-static.cfg.in
@@ -13,6 +13,9 @@ if @LIBUNWIND_ENABLE_THREADS@:
 if @LIBUNWIND_ENABLE_CET@:
     compile_flags.append('-fcf-protection=full')
 
+if @LIBUNWIND_ENABLE_GCS@:
+    compile_flags.append('-mbranch-protection=standard')
+
 # On ELF platforms, link tests with -Wl,--export-dynamic if supported by the linker.
 if len('@CMAKE_EXE_EXPORTS_CXX_FLAG@'):
     link_flags.append('@CMAKE_EXE_EXPORTS_CXX_FLAG@')

nico · 2024-08-04T18:57:50Z

FYI, this seems to break building on Android, see comment on original PR. Maybe worth waiting with the cherry pick until that's resolved.

tru · 2024-08-10T10:33:42Z

Has this been fixed for Android yet?

john-brawn-arm · 2024-08-12T10:31:50Z

I've added the commits that fix this on Android, and fix a problem with combining GCS and BTI.

AArch64 GCS (Guarded Control Stack) is similar enough to CET that we can re-use the existing code that is guarded by _LIBUNWIND_USE_CET, so long as we also add defines to locate the GCS stack and pop the entries from it. We also need the jumpto function to exit using br instead of ret, to prevent it from popping the GCS stack. GCS support is enabled using the LIBUNWIND_ENABLE_GCS cmake option. This enables -mbranch-protection=standard, which enables GCS. For the places we need to use GCS instructions we use the target attribute, as there's not a command-line option to enable a specific architecture extension. (cherry picked from commit b32aac4)

We need both GCS to be enabled by the compiler (which we do by checking if __ARM_FEATURE_GCS_DEFAULT is defined) and for arm_acle.h to define the GCS intrinsics. Check the latter by checking if _CHKFEAT_GCS is defined. (cherry picked from commit c649194)

The libunwind assembly files need adjustment in order to work correctly when both BTI and GCS are both enabled (which will be the case when using -mbranch-protection=standard): * __libunwind_Registers_arm64_jumpto can't use br to jump to the return location, instead we need to use gcspush then ret. * Because we indirectly call __libunwind_Registers_arm64_jumpto it needs to start with bti jc. * We need to set the GCS GNU property bit when it's enabled. --------- Co-authored-by: Daniel Kiss <daniel.kristof.kiss@gmail.com> (cherry picked from commit 3952910)

github-actions · 2024-08-20T07:53:49Z

@john-brawn-arm (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

llvmbot requested a review from a team as a code owner August 4, 2024 13:01

llvmbot added this to the LLVM 19.X Release milestone Aug 4, 2024

llvmbot requested a review from jinge90 August 4, 2024 13:01

llvmbot mentioned this pull request Aug 4, 2024

[libunwind] Add GCS support for AArch64 #99335

Merged

llvmbot added the libunwind label Aug 4, 2024

MaskRay approved these changes Aug 4, 2024

View reviewed changes

llvmbot force-pushed the issue99335 branch from 121ef30 to d28294f Compare August 12, 2024 10:21

john-brawn-arm added 3 commits August 20, 2024 09:29

tru force-pushed the issue99335 branch from d28294f to 72d2932 Compare August 20, 2024 07:29

tru merged commit 72d2932 into llvm:release/19.x Aug 20, 2024
9 of 11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

release/19.x: [libunwind] Add GCS support for AArch64 (#99335) #101888

release/19.x: [libunwind] Add GCS support for AArch64 (#99335) #101888

llvmbot commented Aug 4, 2024

llvmbot commented Aug 4, 2024

llvmbot commented Aug 4, 2024

nico commented Aug 4, 2024

tru commented Aug 10, 2024

john-brawn-arm commented Aug 12, 2024

github-actions bot commented Aug 20, 2024

release/19.x: [libunwind] Add GCS support for AArch64 (#99335) #101888

release/19.x: [libunwind] Add GCS support for AArch64 (#99335) #101888

Conversation

llvmbot commented Aug 4, 2024

llvmbot commented Aug 4, 2024

llvmbot commented Aug 4, 2024

nico commented Aug 4, 2024

tru commented Aug 10, 2024

john-brawn-arm commented Aug 12, 2024

github-actions bot commented Aug 20, 2024