[sanitizer] Don't TestPTrace() if SPARC; don't give up if internal_fork() fails

[thurstond]

Fixes corner cases of #151406:

• Don't run TestPTrace() on SPARC, because internal_fork() on SPARC actually calls __fork(). We can't safely __fork(), because it's possible seccomp has been configured to disallow fork() but allow clone().
• if internal_fork() fails for whatever reason, we shouldn't give up. It is strictly worse to give up early than to attempt StopTheWorld.

Also updates some comments/TODOs.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Thurston Dang (thurstond)

Changes

Fixes corner cases of #151406:

• internal_fork() on SPARC actually calls __fork(). We can't safely fork, because it's possible seccomp has been configured to disallow fork() but allow clone().
• if internal_fork() fails for whatever reason, we shouldn't give up. It is strictly worse to give up early than to attempt StopTheWorld.

Also adds some comments/TODOs.

---

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

1 Files Affected:

• (modified) compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp (+21-4)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp
index d5cf0f14dfeb8..00426de50a388 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp
@@ -409,6 +409,12 @@ struct ScopedSetTracerPID {
 // process with a shared virtual address space and shared TLS, and therefore
 // cannot use waitpid() due to the shared errno.
 static void TestPTrace() {
+#  if SANITIZER_SPARC
+  // internal_fork() on SPARC actually calls __fork(). We can't safely fork,
+  // because it's possible seccomp has been configured to disallow fork() but
+  // allow clone().
+  Report("WARNING: skipping TestPTrace() because this is SPARC\n");
+#  else
   // Heuristic: only check the first time this is called. This is not always
   // correct (e.g., user manually triggers leak detection, then updates
   // seccomp, then leak detection is triggered again).
@@ -417,26 +423,36 @@ static void TestPTrace() {
     return;
   checked = true;
 
-  // We hope that fork() is not too expensive, because of copy-on-write.
+  // Hopefully internal_fork() is not too expensive, thanks to copy-on-write.
   // Besides, this is only called the first time.
+  // Note that internal_fork() on non-SPARC Linux actually calls
+  // SYSCALL(clone); thus, it is reasonable to use it because if seccomp kills
+  // TestPTrace(), it would have killed StopTheWorld() anyway.
   int pid = internal_fork();
 
   if (pid < 0) {
     int rverrno;
-    if (internal_iserror(pid, &rverrno)) {
+    if (internal_iserror(pid, &rverrno))
       Report("WARNING: TestPTrace() failed to fork (errno %d)\n", rverrno);
-    }
-    internal__exit(-1);
+
+    // We don't abort the sanitizer - it's still worth letting the sanitizer
+    // try.
+    return;
   }
 
   if (pid == 0) {
     // Child subprocess
+
+    // TODO: consider checking return value of internal_ptrace, to handle
+    //       SCMP_ACT_ERRNO. However, be careful not to consume too many
+    //       resources performing a proper ptrace.
     internal_ptrace(PTRACE_ATTACH, 0, nullptr, nullptr);
     internal__exit(0);
   } else {
     int wstatus;
     internal_waitpid(pid, &wstatus, 0);
 
+    // Handle SCMP_ACT_KILL
     if (WIFSIGNALED(wstatus)) {
       VReport(0,
               "Warning: ptrace appears to be blocked (is seccomp enabled?). "
@@ -446,6 +462,7 @@ static void TestPTrace() {
       // try.
     }
   }
+#  endif
 }
 
 void StopTheWorld(StopTheWorldCallback callback, void *argument) {

[fmayer]

What are people supposed to do with this warning?

[thurstond]

Added details in 5a630c5