-
Notifications
You must be signed in to change notification settings - Fork 15k
[libc++][hardening] Implement support for assertion semantics. #148172
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
base: main
Are you sure you want to change the base?
Conversation
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions ,h,cpp -- libcxx/include/__log_hardening_failure libcxx/include/__verbose_trap libcxx/src/log_hardening_failure.cpp libcxxabi/src/log_error_and_continue.cpp libcxxabi/src/log_error_and_continue.h libcxx/include/__config libcxx/include/__configuration/availability.h libcxx/test/support/check_assertion.h libcxxabi/src/demangle/DemangleConfig.hView the diff from clang-format here.diff --git a/libcxx/include/__config b/libcxx/include/__config
index c6e90d3a5..2223d466c 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -519,7 +519,7 @@ typedef __char32_t char32_t;
# ifndef _LIBCPP_NO_ABI_TAG
# define _LIBCPP_HIDE_FROM_ABI \
_LIBCPP_HIDDEN _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION \
- __attribute__((__abi_tag__(_LIBCPP_TOSTRING(_LIBCPP_ODR_SIGNATURE))))
+ __attribute__((__abi_tag__(_LIBCPP_TOSTRING(_LIBCPP_ODR_SIGNATURE))))
# else
# define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION
# endif
diff --git a/libcxx/include/__verbose_trap b/libcxx/include/__verbose_trap
index db77b29e6..13ea72773 100644
--- a/libcxx/include/__verbose_trap
+++ b/libcxx/include/__verbose_trap
@@ -18,19 +18,18 @@
_LIBCPP_BEGIN_NAMESPACE_STD
-# if __has_builtin(__builtin_verbose_trap)
+#if __has_builtin(__builtin_verbose_trap)
// AppleClang shipped a slightly different version of __builtin_verbose_trap from the upstream
// version before upstream Clang actually got the builtin.
// TODO: Remove once AppleClang supports the two-arguments version of the builtin.
-# if defined(_LIBCPP_APPLE_CLANG_VER) && _LIBCPP_APPLE_CLANG_VER < 1700
-# define _LIBCPP_VERBOSE_TRAP(message) __builtin_verbose_trap(message)
-# else
-# define _LIBCPP_VERBOSE_TRAP(message) __builtin_verbose_trap("libc++", message)
-# endif
+# if defined(_LIBCPP_APPLE_CLANG_VER) && _LIBCPP_APPLE_CLANG_VER < 1700
+# define _LIBCPP_VERBOSE_TRAP(message) __builtin_verbose_trap(message)
# else
-# define _LIBCPP_VERBOSE_TRAP(message) ((void)message, __builtin_trap())
+# define _LIBCPP_VERBOSE_TRAP(message) __builtin_verbose_trap("libc++", message)
# endif
-
+#else
+# define _LIBCPP_VERBOSE_TRAP(message) ((void)message, __builtin_trap())
+#endif
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxxabi/src/log_error_and_continue.cpp b/libcxxabi/src/log_error_and_continue.cpp
index 51088cd11..51cee6d56 100644
--- a/libcxxabi/src/log_error_and_continue.cpp
+++ b/libcxxabi/src/log_error_and_continue.cpp
@@ -18,11 +18,10 @@ extern "C" void android_set_abort_message(const char* msg);
#if defined(__APPLE__) && __has_include(<os/reason_private.h>)
# include <os/reason_private.h>
-# define _LIBCXXABI_USE_OS_FAULT
+# define _LIBCXXABI_USE_OS_FAULT
#endif
-void __log_error_and_continue(const char* message)
-{
+void __log_error_and_continue(const char* message) {
// On Apple platforms, use the `os_fault_with_payload` OS function that simulates a crash.
#if defined(_LIBCXXABI_USE_OS_FAULT)
os_fault_with_payload(
|
3f1b215 to
0b6face
Compare
| # endif | ||
|
|
||
| // Hardening assertion semantics mirror the evaluation semantics of P3100 Contracts: | ||
| // - `ignore` does not evaluate the assertion; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is incorrect. We do evaluate the assertion, but then we do nothing with that. Let's make this documentation accurate.
That being said, let's also file a Github issue to later go back and fix this: we should mirror what the actual C++26 contracts do, and not evaluate the assertion. Since that's going to require a rework of our assertion machinery, let's not do that right now.
No description provided.