[libunwind] Fix problems caused by combining BTI and GCS

[john-brawn-arm]

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.

[llvmbot]

@llvm/pr-subscribers-libunwind

Author: John Brawn (john-brawn-arm)

Changes

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.

---

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

2 Files Affected:

• (modified) libunwind/src/UnwindRegistersRestore.S (+15-1)
• (modified) libunwind/src/assembly.h (+20-5)

diff --git a/libunwind/src/UnwindRegistersRestore.S b/libunwind/src/UnwindRegistersRestore.S
index b407221ca45f8..7783c94088f68 100644
--- a/libunwind/src/UnwindRegistersRestore.S
+++ b/libunwind/src/UnwindRegistersRestore.S
@@ -629,6 +629,10 @@ Lnovec:
 
 #elif defined(__aarch64__)
 
+#if defined(__ARM_FEATURE_GCS_DEFAULT)
+.arch_extension gcs
+#endif
+
 //
 // extern "C" void __libunwind_Registers_arm64_jumpto(Registers_arm64 *);
 //
@@ -680,7 +684,17 @@ DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_arm64_jumpto)
   ldr    x16,     [x0, #0x0F8]
   ldp    x0, x1,  [x0, #0x000]  // restore x0,x1
   mov    sp,x16                 // restore sp
-  br     x30                    // jump to pc
+#if defined(__ARM_FEATURE_GCS_DEFAULT)
+  // If GCS is enabled we need to push the address we're returning to onto the
+  // GCS stack. We can't just return using br, as there won't be a BTI landing
+  // pad instruction at the destination.
+  mov      x16, #1
+  chkfeat  x16
+  cbnz     x16, Lnogcs
+  gcspushm x30
+Lnogcs:
+#endif
+  ret                           // jump to pc
 
 #elif defined(__arm__) && !defined(__APPLE__)
 
diff --git a/libunwind/src/assembly.h b/libunwind/src/assembly.h
index fb07d04071af3..f8e83e138eff5 100644
--- a/libunwind/src/assembly.h
+++ b/libunwind/src/assembly.h
@@ -82,7 +82,22 @@
 #define PPC64_OPD2
 #endif
 
-#if defined(__aarch64__) && defined(__ARM_FEATURE_BTI_DEFAULT)
+#if defined(__aarch64__)
+#if defined(__ARM_FEATURE_GCS_DEFAULT) && defined(__ARM_FEATURE_BTI_DEFAULT)
+// Set BTI, PAC, and GCS gnu property bits
+#define GNU_PROPERTY 7
+// We indirectly branch to __libunwind_Registers_arm64_jumpto from
+// __unw_phase2_resume, so we need to use bti jc.
+#define AARCH64_BTI bti jc
+#elif defined(__ARM_FEATURE_GCS_DEFAULT)
+// Set GCS gnu property bit
+#define GNU_PROPERTY 4
+#elif defined(__ARM_FEATURE_BTI_DEFAULT)
+// Set BTI and PAC gnu property bits
+#define GNU_PROPERTY 3
+#define AARCH64_BTI bti c
+#endif
+#ifdef GNU_PROPERTY
   .pushsection ".note.gnu.property", "a" SEPARATOR                             \
   .balign 8 SEPARATOR                                                          \
   .long 4 SEPARATOR                                                            \
@@ -91,12 +106,12 @@
   .asciz "GNU" SEPARATOR                                                       \
   .long 0xc0000000 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_AND */          \
   .long 4 SEPARATOR                                                            \
-  .long 3 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_BTI AND */               \
-                    /* GNU_PROPERTY_AARCH64_FEATURE_1_PAC */                   \
+  .long GNU_PROPERTY SEPARATOR                                                 \
   .long 0 SEPARATOR                                                            \
   .popsection SEPARATOR
-#define AARCH64_BTI  bti c
-#else
+#endif
+#endif
+#if !defined(AARCH64_BTI)
 #define AARCH64_BTI
 #endif
 

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff d06303ffc1f3b2023532fd426734e9435f87d038 d64241402d6fc32d34fea649bb65186d259fc8e8 --extensions h -- libunwind/src/assembly.h

View the diff from clang-format here.

diff --git a/libunwind/src/assembly.h b/libunwind/src/assembly.h
index f8e83e138e..ff004fe887 100644
--- a/libunwind/src/assembly.h
+++ b/libunwind/src/assembly.h
@@ -98,17 +98,12 @@
 #define AARCH64_BTI bti c
 #endif
 #ifdef GNU_PROPERTY
-  .pushsection ".note.gnu.property", "a" SEPARATOR                             \
-  .balign 8 SEPARATOR                                                          \
-  .long 4 SEPARATOR                                                            \
-  .long 0x10 SEPARATOR                                                         \
-  .long 0x5 SEPARATOR                                                          \
-  .asciz "GNU" SEPARATOR                                                       \
-  .long 0xc0000000 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_AND */          \
-  .long 4 SEPARATOR                                                            \
-  .long GNU_PROPERTY SEPARATOR                                                 \
-  .long 0 SEPARATOR                                                            \
-  .popsection SEPARATOR
+.pushsection ".note.gnu.property",
+    "a" SEPARATOR.balign 8 SEPARATOR.long 4 SEPARATOR.long 0x10 SEPARATOR
+        .long 0x5 SEPARATOR.asciz "GNU" SEPARATOR
+        .long 0xc0000000 SEPARATOR /* GNU_PROPERTY_AARCH64_FEATURE_1_AND */
+        .long 4 SEPARATOR.long GNU_PROPERTY SEPARATOR.long 0 SEPARATOR
+        .popsection SEPARATOR
 #endif
 #endif
 #if !defined(AARCH64_BTI)
@@ -117,12 +112,13 @@
 
 #if !defined(__aarch64__)
 #ifdef __ARM_FEATURE_PAC_DEFAULT
-  .eabi_attribute Tag_PAC_extension, 2
-  .eabi_attribute Tag_PACRET_use, 1
+        .eabi_attribute Tag_PAC_extension,
+    2 .eabi_attribute Tag_PACRET_use,
+    1
 #endif
 #ifdef __ARM_FEATURE_BTI_DEFAULT
-  .eabi_attribute Tag_BTI_extension, 1
-  .eabi_attribute Tag_BTI_use, 1
+        .eabi_attribute Tag_BTI_extension,
+    1 .eabi_attribute Tag_BTI_use, 1
 #endif
 #endif
 

[DanielKristofKiss]

Thanks!
LGTM with a NIT.
Please wait for someone from the unwind group to ack it too.

[DanielKristofKiss]

__unw_getcontext will also get the bti jc so slightly weakens the BTI protection but since paciasp is also bti jc might not be a big problem.