[libc] Disable Death Tests While Hermetic

[michaelrj-google]

The death test infrastructure seems to depend on operator new, which
isn't currently supported in our hermetic tests. This patch just
disables the death tests in hermetic mode since they only overlap in the
nan tests.

[llvmbot]

@llvm/pr-subscribers-libc

Author: None (michaelrj-google)

Changes

The death test infrastructure seems to depend on operator new, which
isn't currently supported in our hermetic tests. This patch just
disables the death tests in hermetic mode since they only overlap in the
nan tests.

---

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

4 Files Affected:

• (modified) libc/cmake/modules/LLVMLibCTestRules.cmake (+1-1)
• (modified) libc/test/src/math/smoke/nan_test.cpp (+2-2)
• (modified) libc/test/src/math/smoke/nanf_test.cpp (+2-2)
• (modified) libc/test/src/math/smoke/nanl_test.cpp (+2-2)

diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 24f543f6e4c132..8029500f63e68b 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -605,7 +605,7 @@ function(add_integration_test test_name)
 endfunction(add_integration_test)
 
 set(LIBC_HERMETIC_TEST_COMPILE_OPTIONS ${LIBC_COMPILE_OPTIONS_DEFAULT}
-    -fpie -ffreestanding -fno-exceptions -fno-rtti)
+    -fpie -ffreestanding -fno-exceptions -fno-rtti -DLIBC_HERMETIC_TEST)
 # The GPU build requires overriding the default CMake triple and architecture.
 if(LIBC_GPU_TARGET_ARCHITECTURE_IS_AMDGPU)
   list(APPEND LIBC_HERMETIC_TEST_COMPILE_OPTIONS
diff --git a/libc/test/src/math/smoke/nan_test.cpp b/libc/test/src/math/smoke/nan_test.cpp
index 81e1400f0bb6b9..f7a05d0f73008f 100644
--- a/libc/test/src/math/smoke/nan_test.cpp
+++ b/libc/test/src/math/smoke/nan_test.cpp
@@ -42,8 +42,8 @@ TEST_F(LlvmLibcNanTest, RandomString) {
   run_test("123 ", 0x7ff8000000000000);
 }
 
-#ifndef LIBC_HAVE_ADDRESS_SANITIZER
+#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && !defined(LIBC_HERMETIC_TEST)
 TEST_F(LlvmLibcNanTest, InvalidInput) {
   EXPECT_DEATH([] { LIBC_NAMESPACE::nan(nullptr); }, WITH_SIGNAL(SIGSEGV));
 }
-#endif // LIBC_HAVE_ADDRESS_SANITIZER
+#endif // !LIBC_HAVE_ADDRESS_SANITIZER && !LIBC_HERMETIC_TEST
diff --git a/libc/test/src/math/smoke/nanf_test.cpp b/libc/test/src/math/smoke/nanf_test.cpp
index 1d337ecf1dcd08..1aa4f00f00c3a1 100644
--- a/libc/test/src/math/smoke/nanf_test.cpp
+++ b/libc/test/src/math/smoke/nanf_test.cpp
@@ -41,8 +41,8 @@ TEST_F(LlvmLibcNanfTest, RandomString) {
   run_test("123 ", 0x7fc00000);
 }
 
-#ifndef LIBC_HAVE_ADDRESS_SANITIZER
+#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && !defined(LIBC_HERMETIC_TEST)
 TEST_F(LlvmLibcNanfTest, InvalidInput) {
   EXPECT_DEATH([] { LIBC_NAMESPACE::nanf(nullptr); }, WITH_SIGNAL(SIGSEGV));
 }
-#endif // LIBC_HAVE_ADDRESS_SANITIZER
+#endif // !LIBC_HAVE_ADDRESS_SANITIZER && !LIBC_HERMETIC_TEST
diff --git a/libc/test/src/math/smoke/nanl_test.cpp b/libc/test/src/math/smoke/nanl_test.cpp
index 009710b49c832e..f3f241e8786e0d 100644
--- a/libc/test/src/math/smoke/nanl_test.cpp
+++ b/libc/test/src/math/smoke/nanl_test.cpp
@@ -67,8 +67,8 @@ TEST_F(LlvmLibcNanlTest, RandomString) {
   run_test("123 ", expected);
 }
 
-#ifndef LIBC_HAVE_ADDRESS_SANITIZER
+#if !defined(LIBC_HAVE_ADDRESS_SANITIZER) && !defined(LIBC_HERMETIC_TEST)
 TEST_F(LlvmLibcNanlTest, InvalidInput) {
   EXPECT_DEATH([] { LIBC_NAMESPACE::nanl(nullptr); }, WITH_SIGNAL(SIGSEGV));
 }
-#endif // LIBC_HAVE_ADDRESS_SANITIZER
+#endif // !LIBC_HAVE_ADDRESS_SANITIZER && !LIBC_HERMETIC_TEST

[jhuber6]

I thought we had operator new in hermetic tests? I'm remembering an old issue where we had the regular new but not the optional one that takes alignment. I could be wrong.

[michaelrj-google]

Here's the error if you want to look for yourself:

/usr/bin/ld: libc/test/src/math/smoke/CMakeFiles/libc.test.src.math.smoke.nan_test.__hermetic__.__build__.dir/nan_test.cpp.o: in function `LlvmLibcNanTest_InvalidInput::Run()':
nan_test.cpp:(.text._ZN28LlvmLibcNanTest_InvalidInput3RunEv+0xe): undefined reference to `operator new(unsigned long)'
/usr/bin/ld: nan_test.cpp:(.text._ZN28LlvmLibcNanTest_InvalidInput3RunEv+0x53): undefined reference to `__llvm_libc____::testing::Test::testProcessKilled(__llvm_libc____::testutils::FunctionCaller*, int, char const*, char const*, __llvm_libc____::testing::internal::Location)'

[nickdesaulniers]

Seems fine to disable for now, but I would prefer:

1. file an issue to see how to fix this more properly (by providing a definition of operator new that the linker is complaining about).
2. link to that issue in a comment in the cmake files.

Folks are probably going to keep hitting this issue as they write more and more death tests.

[jhuber6]

Seems fine to disable for now, but I would prefer:

1. file an issue to see how to fix this more properly (by providing a definition of `operator new` that the linker is complaining about).

2. link to that issue in a comment in the cmake files.

Folks are probably going to keep hitting this issue as they write more and more death tests.

My understanding was that death tests are always disabled when doing hermetic tests, so they should always be UNIT_TEST_ONLY. Maybe what happened is someone added those into a single test without knowing that death tests need to be separate.

[michaelrj-google]

Defining operator new doesn't fix the issue. I think @jhuber6 is right that death tests can't work in hermetic tests. I'll update this patch to reflect that.

[michaelrj-google]

New patch is up. After some discussion with @nickdesaulniers I decided to leave the hermetic operator new in, though I can move that to a separate patch if necessary.