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

[libc++][hardening] Use __builtin_verbose_trap if it's available. #84870

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

var-const
Copy link
Member

Keep falling back to __builtin_trap on older versions of Clang.

@var-const var-const requested a review from a team as a code owner March 12, 2024 04:47
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 12, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 12, 2024

@llvm/pr-subscribers-libcxx

Author: Konstantin Varlamov (var-const)

Changes

Keep falling back to __builtin_trap on older versions of Clang.


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

1 Files Affected:

  • (modified) libcxx/vendor/llvm/default_assertion_handler.in (+5-2)
diff --git a/libcxx/vendor/llvm/default_assertion_handler.in b/libcxx/vendor/llvm/default_assertion_handler.in
index 8bc0553c078b34..8b892ff7b565eb 100644
--- a/libcxx/vendor/llvm/default_assertion_handler.in
+++ b/libcxx/vendor/llvm/default_assertion_handler.in
@@ -23,8 +23,11 @@
 
 #else
 
-// TODO(hardening): use `__builtin_verbose_trap(message)` once that becomes available.
-#  define _LIBCPP_ASSERTION_HANDLER(message) ((void)message, __builtin_trap())
+#  if __has_builtin(__builtin_verbose_trap)
+#    define _LIBCPP_ASSERTION_HANDLER(message) (__builtin_verbose_trap(message))
+#  else
+#    define _LIBCPP_ASSERTION_HANDLER(message) ((void)message, __builtin_trap())
+#  endif
 
 #endif // _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG
 

@var-const var-const added the hardening Issues related to the hardening effort label Mar 12, 2024
@var-const
Copy link
Member Author

Depends on #79230.

@ldionne ldionne self-assigned this Mar 12, 2024
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

LGTM once we have a Clang that supports the builtin.

@ldionne ldionne added this to the LLVM 19.X Release milestone Jul 9, 2024
Keep falling back to `__builtin_trap` on older versions of Clang.
@ldionne ldionne changed the title [libc++][hardening] Use __builtin_verbose_trap if it's available. [libc++][hardening] Use __builtin_verbose_trap if it's available. Jul 9, 2024
@ldionne ldionne force-pushed the varconst/use-verbose-trap branch from b48f4ed to ea44fac Compare July 9, 2024 18:30
@ldionne
Copy link
Member

ldionne commented Jul 9, 2024

Rebase to trigger CI. This can be merged once CI is green.

@ldionne
Copy link
Member

ldionne commented Jul 12, 2024

The CI failure is a fluke, merging.

@ldionne ldionne merged commit 1e96b4a into llvm:main Jul 12, 2024
52 of 54 checks passed
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
…lvm#84870)

Keep falling back to `__builtin_trap` on older versions of Clang.

Co-authored-by: Louis Dionne <[email protected]>
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 16, 2024
…lvm#84870)

Keep falling back to `__builtin_trap` on older versions of Clang.

Co-authored-by: Louis Dionne <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hardening Issues related to the hardening effort libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants