[libc-commits] [libc] [libc][test] Condition out tests that can't work on bare metal (PR #215830)

[via libc-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Simon Tatham (statham-arm)

<details>
<summary>Changes</summary>

In bare-metal builds of libc, the `EXPECT_DEATH` macro may not be defined. Also, `signal-macros.h` may not define the values needed for the rest of `<signal.h>` to work. So tests that rely on either of those things will fail to compile.

I've conditioned out the `EXPECT_DEATH` tests completely if `EXPECT_DEATH` isn't defined. There's inherently no reliable way to define it: you can't rely on finding out about segmentation faults by a signal, because accessing memory outside valid C objects might silently succeed (valid unused memory), or generate a CPU fault that no kernel traps for you, or overwrite something important outside your program.

The check for signals in `FPExceptMatcher.cpp` can be more lenient, and just condition out the signal-handling code, leaving the test of cumulative FP exception flags in place, so that the checker simply returns "no signal was caught" unconditionally.

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


4 Files Affected:

- (modified) libc/test/UnitTest/FPExceptMatcher.cpp (+20-1) 
- (modified) libc/test/src/compiler/stack_chk_guard_test.cpp (+7) 
- (modified) libc/test/src/time/localtime_r_test.cpp (+2) 
- (modified) libc/test/src/time/localtime_test.cpp (+2) 


``````````diff
diff --git a/libc/test/UnitTest/FPExceptMatcher.cpp b/libc/test/UnitTest/FPExceptMatcher.cpp
index 10b6ff9f91da1..863811841124f 100644
--- a/libc/test/UnitTest/FPExceptMatcher.cpp
+++ b/libc/test/UnitTest/FPExceptMatcher.cpp
@@ -20,7 +20,15 @@
 #include "hdr/types/fenv_t.h"
 #include "src/__support/FPUtil/FEnvImpl.h"
 #include <setjmp.h>
+
+// To make this test work on bare-metal targets without working signal.h, find
+// out if signal-macros.h did anything, before including the full signal.h. It
+// doesn't define a specific macro of the form HAVE_SIGNALS, so we just test
+// for one of the macros it _does_ define.
+#include "llvm-libc-macros/signal-macros.h"
+#ifdef __NSIGSET_WORDS
 #include <signal.h>
+#endif
 
 #if LIBC_TEST_HAS_MATCHERS()
 
@@ -37,27 +45,38 @@ namespace testing {
 using sighandler_t = __sighandler_t *;
 #endif
 
-static thread_local sigjmp_buf jumpBuffer;
 static thread_local bool caughtExcept;
 
+#ifdef __NSIGSET_WORDS
+
+static thread_local sigjmp_buf jumpBuffer;
+
 static void sigfpeHandler([[maybe_unused]] int sig) {
   caughtExcept = true;
   siglongjmp(jumpBuffer, -1);
 }
 
+#endif // __NSIGSET_WORDS
+
 FPExceptMatcher::FPExceptMatcher(FunctionCaller *func) {
+#ifdef __NSIGSET_WORDS
   auto *oldSIGFPEHandler = signal(SIGFPE, &sigfpeHandler);
+#endif
 
   caughtExcept = false;
   fenv_t oldEnv;
   fputil::get_env(&oldEnv);
+#ifdef __NSIGSET_WORDS
   if (sigsetjmp(jumpBuffer, 1) == 0)
+#endif
     func->call();
   delete func;
   // We restore the previous floating point environment after
   // the call to the function which can potentially raise SIGFPE.
   fputil::set_env(&oldEnv);
+#ifdef __NSIGSET_WORDS
   signal(SIGFPE, oldSIGFPEHandler);
+#endif
   exceptionRaised = caughtExcept;
 }
 
diff --git a/libc/test/src/compiler/stack_chk_guard_test.cpp b/libc/test/src/compiler/stack_chk_guard_test.cpp
index 301031ff47bd5..978e889263405 100644
--- a/libc/test/src/compiler/stack_chk_guard_test.cpp
+++ b/libc/test/src/compiler/stack_chk_guard_test.cpp
@@ -10,6 +10,13 @@
 #include "src/compiler/__stack_chk_fail.h"
 #include "test/UnitTest/Test.h"
 
+#ifdef EXPECT_DEATH
 TEST(LlvmLibcStackChkFail, Death) {
   EXPECT_DEATH([] { __stack_chk_fail(); }, WITH_SIGNAL(SIGABRT));
 }
+#else
+TEST(LlvmLibcStackChkFail, Dummy) {
+  // Need at least one test, because a completely empty test file
+  // counts as failure
+}
+#endif // EXPECT_DEATH
diff --git a/libc/test/src/time/localtime_r_test.cpp b/libc/test/src/time/localtime_r_test.cpp
index bc71419d68c64..e3c3b96278b89 100644
--- a/libc/test/src/time/localtime_r_test.cpp
+++ b/libc/test/src/time/localtime_r_test.cpp
@@ -27,6 +27,7 @@ TEST(LlvmLibcLocaltimeR, ValidUnixTimestamp0) {
   ASSERT_EQ(0, result->tm_isdst);
 }
 
+#ifdef EXPECT_DEATH
 TEST(LlvmLibcLocaltimeR, NullPtr) {
   struct tm input;
   time_t timer = 0;
@@ -37,6 +38,7 @@ TEST(LlvmLibcLocaltimeR, NullPtr) {
   EXPECT_DEATH([&] { LIBC_NAMESPACE::localtime_r(&timer, nullptr); },
                WITH_SIGNAL(-1));
 }
+#endif // EXPECT_DEATH
 
 // TODO(zimirza): These tests does not expect the correct output of localtime as
 // per specification. This is due to timezone functions removed from
diff --git a/libc/test/src/time/localtime_test.cpp b/libc/test/src/time/localtime_test.cpp
index 37974d27771dc..6e1116b98eeb1 100644
--- a/libc/test/src/time/localtime_test.cpp
+++ b/libc/test/src/time/localtime_test.cpp
@@ -25,9 +25,11 @@ TEST(LlvmLibcLocaltime, ValidUnixTimestamp0) {
   ASSERT_EQ(0, result->tm_isdst);
 }
 
+#ifdef EXPECT_DEATH
 TEST(LlvmLibcLocaltime, NullPtr) {
   EXPECT_DEATH([] { LIBC_NAMESPACE::localtime(nullptr); }, WITH_SIGNAL(-1));
 }
+#endif // EXPECT_DEATH
 
 // TODO(zimirza): These tests does not expect the correct output of localtime as
 // per specification. This is due to timezone functions removed from

``````````

</details>


https://github.com/llvm/llvm-project/pull/215830