[libc-commits] [libc] [libc][LIBC_ADD_NULL_CHECKS] replace volatile deref with __builtin_trap (PR #123401)
Nick Desaulniers via libc-commits
libc-commits at lists.llvm.org
Tue Jan 21 13:46:46 PST 2025
https://github.com/nickdesaulniers updated https://github.com/llvm/llvm-project/pull/123401
>From b8c0189eb559b698bff5262f70f36c3e6388af71 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Fri, 17 Jan 2025 13:07:17 -0800
Subject: [PATCH 1/2] [libc][LIBC_ADD_NULL_CHECKS] replace volatile deref with
__builtin_trap
Also, update the unit tests that were checking for SIGSEGV to check for SIGILL.
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.
---
libc/src/__support/macros/null_check.h | 9 ++-------
libc/test/src/math/smoke/nan_test.cpp | 4 ++--
libc/test/src/math/smoke/nanf128_test.cpp | 4 ++--
libc/test/src/math/smoke/nanf16_test.cpp | 4 ++--
libc/test/src/math/smoke/nanf_test.cpp | 4 ++--
libc/test/src/math/smoke/nanl_test.cpp | 4 ++--
6 files changed, 12 insertions(+), 17 deletions(-)
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 46b9e9aa9563ab..579e37c4268f7d 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); }, WITH_SIGNAL(SIGILL));
}
#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 25dd2ef1d5b1ca..bc5b4f0d9afc6d 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); }, WITH_SIGNAL(SIGILL));
}
#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 ec640a3b9eef92..66f8963c971305 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); }, WITH_SIGNAL(SIGILL));
}
#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 dd3124ee9c5112..7795803ac203ef 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); }, WITH_SIGNAL(SIGILL));
}
#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 ef3f9c15dafd9f..e07297813d6040 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); }, WITH_SIGNAL(SIGILL));
}
#endif // LIBC_HAS_ADDRESS_SANITIZER
>From a5327a8e45e13900005c1ef5f61508690a56936d Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Tue, 21 Jan 2025 13:46:33 -0800
Subject: [PATCH 2/2] remove explicit signal checks
---
libc/test/src/math/smoke/nan_test.cpp | 2 +-
libc/test/src/math/smoke/nanf128_test.cpp | 2 +-
libc/test/src/math/smoke/nanf16_test.cpp | 2 +-
libc/test/src/math/smoke/nanf_test.cpp | 2 +-
libc/test/src/math/smoke/nanl_test.cpp | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/libc/test/src/math/smoke/nan_test.cpp b/libc/test/src/math/smoke/nan_test.cpp
index 579e37c4268f7d..53c43efa2c0d94 100644
--- a/libc/test/src/math/smoke/nan_test.cpp
+++ b/libc/test/src/math/smoke/nan_test.cpp
@@ -46,6 +46,6 @@ TEST_F(LlvmLibcNanTest, RandomString) {
#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
TEST_F(LlvmLibcNanTest, InvalidInput) {
- EXPECT_DEATH([] { LIBC_NAMESPACE::nan(nullptr); }, WITH_SIGNAL(SIGILL));
+ 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 bc5b4f0d9afc6d..1dee38f92af83d 100644
--- a/libc/test/src/math/smoke/nanf128_test.cpp
+++ b/libc/test/src/math/smoke/nanf128_test.cpp
@@ -57,6 +57,6 @@ TEST_F(LlvmLibcNanf128Test, RandomString) {
#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
TEST_F(LlvmLibcNanf128Test, InvalidInput) {
- EXPECT_DEATH([] { LIBC_NAMESPACE::nanf128(nullptr); }, WITH_SIGNAL(SIGILL));
+ 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 66f8963c971305..c59381a1acd038 100644
--- a/libc/test/src/math/smoke/nanf16_test.cpp
+++ b/libc/test/src/math/smoke/nanf16_test.cpp
@@ -45,6 +45,6 @@ TEST_F(LlvmLibcNanf16Test, RandomString) {
#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
TEST_F(LlvmLibcNanf16Test, InvalidInput) {
- EXPECT_DEATH([] { LIBC_NAMESPACE::nanf16(nullptr); }, WITH_SIGNAL(SIGILL));
+ 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 7795803ac203ef..74cb48b2e3d33c 100644
--- a/libc/test/src/math/smoke/nanf_test.cpp
+++ b/libc/test/src/math/smoke/nanf_test.cpp
@@ -45,6 +45,6 @@ TEST_F(LlvmLibcNanfTest, RandomString) {
#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
TEST_F(LlvmLibcNanfTest, InvalidInput) {
- EXPECT_DEATH([] { LIBC_NAMESPACE::nanf(nullptr); }, WITH_SIGNAL(SIGILL));
+ 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 e07297813d6040..97017345d0ce35 100644
--- a/libc/test/src/math/smoke/nanl_test.cpp
+++ b/libc/test/src/math/smoke/nanl_test.cpp
@@ -73,6 +73,6 @@ TEST_F(LlvmLibcNanlTest, RandomString) {
#if defined(LIBC_ADD_NULL_CHECKS) && !defined(LIBC_HAS_SANITIZER)
TEST_F(LlvmLibcNanlTest, InvalidInput) {
- EXPECT_DEATH([] { LIBC_NAMESPACE::nanl(nullptr); }, WITH_SIGNAL(SIGILL));
+ EXPECT_DEATH([] { LIBC_NAMESPACE::nanl(nullptr); });
}
#endif // LIBC_HAS_ADDRESS_SANITIZER
More information about the libc-commits
mailing list