[libc-commits] [libc] 8e79ade - [libc][LIBC_ADD_NULL_CHECKS] replace volatile deref with __builtin_trap (#123401)

[via libc-commits]

Author: Nick Desaulniers
Date: 2025-01-22T09:34:59-08:00
New Revision: 8e79ade49d68c49aeb8ba008b59f559b86d22765

URL: https://github.com/llvm/llvm-project/commit/8e79ade49d68c49aeb8ba008b59f559b86d22765
DIFF: https://github.com/llvm/llvm-project/commit/8e79ade49d68c49aeb8ba008b59f559b86d22765.diff

LOG: [libc][LIBC_ADD_NULL_CHECKS] replace volatile deref with __builtin_trap (#123401)

Also, update the unit tests that were checking for SIGSEGV to not check for a
specific signal.

To further improve this check, it may be worth:
- renaming the configuration option/macro/docs to be clearer about intent.
- swap __builtin_trap for __builtin_unreachable, removing the preprocessor
  variants of LIBC_CRASH_ON_NULLPTR, then unconditionally using
  `-fsanitize=unreachable -fsanitize-trap=unreachable` in cmake flags when
  LIBC_ADD_NULL_CHECKS is enabled.
- building with `-fno-delete-null-pointer-checks` when LIBC_ADD_NULL_CHECKS (or
  when some larger yet to be added hardening config) is enabled.

Link: #111546

Added: 
    

Modified: 
    libc/src/__support/macros/null_check.h
    libc/test/src/math/smoke/nan_test.cpp
    libc/test/src/math/smoke/nanf128_test.cpp
    libc/test/src/math/smoke/nanf16_test.cpp
    libc/test/src/math/smoke/nanf_test.cpp
    libc/test/src/math/smoke/nanl_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/macros/null_check.h b/libc/src/__support/macros/null_check.h
index 400f7d809db4fa..eda19f889235e4 100644
--- a/libc/src/__support/macros/null_check.h
+++ b/libc/src/__support/macros/null_check.h
@@ -14,15 +14,10 @@
 #include "src/__support/macros/sanitizer.h"
 
 #if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
-// Use volatile to prevent undefined behavior of dereferencing nullptr.
-// Intentionally crashing with SIGSEGV.
-#define LIBC_CRASH_ON_NULLPTR(PTR)                                             \
+#define LIBC_CRASH_ON_NULLPTR(ptr)                                             \
   do {                                                                         \
-    if (LIBC_UNLIKELY(PTR == nullptr)) {                                       \
-      volatile auto *crashing = PTR;                                           \
-      [[maybe_unused]] volatile auto crash = *crashing;                        \
+    if (LIBC_UNLIKELY((ptr) == nullptr))                                       \
       __builtin_trap();                                                        \
-    }                                                                          \
   } while (0)
 #else
 #define LIBC_CRASH_ON_NULLPTR(ptr)                                             \

diff  --git a/libc/test/src/math/smoke/nan_test.cpp b/libc/test/src/math/smoke/nan_test.cpp
index da6beb94c7f05d..e45e2e6d499a2b 100644
--- a/libc/test/src/math/smoke/nan_test.cpp
+++ b/libc/test/src/math/smoke/nan_test.cpp
@@ -44,8 +44,8 @@ TEST_F(LlvmLibcNanTest, RandomString) {
   run_test("123 ", 0x7ff8000000000000);
 }
 
-#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
+#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
 TEST_F(LlvmLibcNanTest, InvalidInput) {
-  EXPECT_DEATH([] { LIBC_NAMESPACE::nan(nullptr); }, WITH_SIGNAL(SIGSEGV));
+  EXPECT_DEATH([] { LIBC_NAMESPACE::nan(nullptr); });
 }
 #endif // LIBC_HAS_ADDRESS_SANITIZER

diff  --git a/libc/test/src/math/smoke/nanf128_test.cpp b/libc/test/src/math/smoke/nanf128_test.cpp
index dd1986f17b9785..aa59b79aac9d80 100644
--- a/libc/test/src/math/smoke/nanf128_test.cpp
+++ b/libc/test/src/math/smoke/nanf128_test.cpp
@@ -55,8 +55,8 @@ TEST_F(LlvmLibcNanf128Test, RandomString) {
            QUIET_NAN);
 }
 
-#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
+#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
 TEST_F(LlvmLibcNanf128Test, InvalidInput) {
-  EXPECT_DEATH([] { LIBC_NAMESPACE::nanf128(nullptr); }, WITH_SIGNAL(SIGSEGV));
+  EXPECT_DEATH([] { LIBC_NAMESPACE::nanf128(nullptr); });
 }
 #endif // LIBC_HAS_ADDRESS_SANITIZER

diff  --git a/libc/test/src/math/smoke/nanf16_test.cpp b/libc/test/src/math/smoke/nanf16_test.cpp
index 5fafb1a36e4cdc..04a8c7bb5d9338 100644
--- a/libc/test/src/math/smoke/nanf16_test.cpp
+++ b/libc/test/src/math/smoke/nanf16_test.cpp
@@ -43,8 +43,8 @@ TEST_F(LlvmLibcNanf16Test, RandomString) {
   run_test("123 ", 0x7e00);
 }
 
-#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
+#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
 TEST_F(LlvmLibcNanf16Test, InvalidInput) {
-  EXPECT_DEATH([] { LIBC_NAMESPACE::nanf16(nullptr); }, WITH_SIGNAL(SIGSEGV));
+  EXPECT_DEATH([] { LIBC_NAMESPACE::nanf16(nullptr); });
 }
 #endif // LIBC_HAS_ADDRESS_SANITIZER

diff  --git a/libc/test/src/math/smoke/nanf_test.cpp b/libc/test/src/math/smoke/nanf_test.cpp
index 19d94b40b5ffbd..40e90c48d8cda7 100644
--- a/libc/test/src/math/smoke/nanf_test.cpp
+++ b/libc/test/src/math/smoke/nanf_test.cpp
@@ -43,8 +43,8 @@ TEST_F(LlvmLibcNanfTest, RandomString) {
   run_test("123 ", 0x7fc00000);
 }
 
-#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
+#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
 TEST_F(LlvmLibcNanfTest, InvalidInput) {
-  EXPECT_DEATH([] { LIBC_NAMESPACE::nanf(nullptr); }, WITH_SIGNAL(SIGSEGV));
+  EXPECT_DEATH([] { LIBC_NAMESPACE::nanf(nullptr); });
 }
 #endif // LIBC_HAS_ADDRESS_SANITIZER

diff  --git a/libc/test/src/math/smoke/nanl_test.cpp b/libc/test/src/math/smoke/nanl_test.cpp
index c7217928e943b0..dea969fd3d2adc 100644
--- a/libc/test/src/math/smoke/nanl_test.cpp
+++ b/libc/test/src/math/smoke/nanl_test.cpp
@@ -71,8 +71,8 @@ TEST_F(LlvmLibcNanlTest, RandomString) {
   run_test("123 ", expected);
 }
 
-#if !defined(LIBC_HAS_ADDRESS_SANITIZER) && defined(LIBC_TARGET_OS_IS_LINUX)
+#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
 TEST_F(LlvmLibcNanlTest, InvalidInput) {
-  EXPECT_DEATH([] { LIBC_NAMESPACE::nanl(nullptr); }, WITH_SIGNAL(SIGSEGV));
+  EXPECT_DEATH([] { LIBC_NAMESPACE::nanl(nullptr); });
 }
 #endif // LIBC_HAS_ADDRESS_SANITIZER