Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libunwind] Be more careful about enabling GCS #101973

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

john-brawn-arm
Copy link
Collaborator

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.

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.
@john-brawn-arm john-brawn-arm requested review from nico, tru and MaskRay August 5, 2024 13:18
@john-brawn-arm john-brawn-arm requested a review from a team as a code owner August 5, 2024 13:18
@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2024

@llvm/pr-subscribers-libunwind

Author: John Brawn (john-brawn-arm)

Changes

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.


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

1 Files Affected:

  • (modified) libunwind/src/cet_unwind.h (+5-1)
diff --git a/libunwind/src/cet_unwind.h b/libunwind/src/cet_unwind.h
index 45c11973cb7fa..47d7616a7322c 100644
--- a/libunwind/src/cet_unwind.h
+++ b/libunwind/src/cet_unwind.h
@@ -39,9 +39,13 @@
 // 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>
 
+// We can only use GCS if arm_acle.h defines the GCS intrinsics.
+#ifdef _CHKFEAT_GCS
+#define _LIBUNWIND_USE_GCS 1
+#endif
+
 #define _LIBUNWIND_POP_CET_SSP(x)                                              \
   do {                                                                         \
     if (__chkfeat(_CHKFEAT_GCS)) {                                             \

Copy link
Contributor

@nico nico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks!

@john-brawn-arm john-brawn-arm merged commit c649194 into llvm:main Aug 5, 2024
52 of 54 checks passed
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
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.
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 12, 2024
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)
@john-brawn-arm john-brawn-arm deleted the libunwind_gcs_android branch August 14, 2024 09:44
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Aug 15, 2024
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.
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 20, 2024
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants