From b8c0189eb559b698bff5262f70f36c3e6388af71 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers 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 400f7d809db4f..eda19f889235e 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 46b9e9aa9563a..579e37c4268f7 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 25dd2ef1d5b1c..bc5b4f0d9afc6 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 ec640a3b9eef9..66f8963c97130 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 dd3124ee9c511..7795803ac203e 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 ef3f9c15dafd9..e07297813d604 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 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 579e37c4268f7..53c43efa2c0d9 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 bc5b4f0d9afc6..1dee38f92af83 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 66f8963c97130..c59381a1acd03 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 7795803ac203e..74cb48b2e3d33 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 e07297813d604..97017345d0ce3 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