[safestack] Various Solaris fixes

[rorth]

Even with the -u __safestack_init link order fixed on Solaris, there are still several safestack test issues left:

• While 540fd42 enabled safestack on Solaris in the driver unconditionally, it ignored that Solaris also exists on SPARC and forgot to enable SPARC support for the runtime lib. This patch fixes that.

• The tests fail to link with undefined references to __sanitizer_internal_memset etc in safestack.cpp.o and interception_linux.cpp.o. These are from indirectly including sanitizer_redefine_builtins.h. Instead of using the implementations from sanitizer_common as was done in [safestack] Various Solaris fixes, this patch disables the interception as discussed in Revert "[safestack] Various Solaris fixes". A similar issue affects 32-bit Linux/sparc where compiling safestack.cpp with -ftrivial-auto-var-init=pattern causes the compiler to generate calls to memset to initialize a pthread_attr_t which is larger than can be handled inline. This is avoided by defining SANITIZER_COMMON_NO_REDEFINE_BUILTINS in safestack.cpp and also adding definitions of the interceptors that just forward to libc for the benefit of interception_linux.cpp.

• The pthread*.c tests FAIL with

  safestack CHECK failed: /vol/llvm/src/llvm-project/local/compiler-rt/lib/safestack/safestack.cpp:227 size

  The problem is that pthread_attr_init initializes the stacksize attribute to 0, signifying the default. Unless explicitly overridded, it stays that way. I think this is allowed by XPG7. Since safestack cannot deal with this, I set size to the defaults documented in pthread_create(3C). Unfortunately, there's no macro for those values outside of private libc headers.

• The Solaris syscall interface isn't stable. This is not just a theoretical concern, but the syscalls have changed incompatibly several times in the past. Therefore this patch switches the implementations of TgKill (where SYS_lwp_kill doesn't exist on Solaris 11.4 anyway), Mmap, Munmap, and Mprotect to the same _REAL* solution already used in sanitizer_solaris.cpp.

With those changes, safestack compiles and all tests PASS, so the tests are re-enabled for good.

Tested on amd64-pc-solaris2.11, sparcv9-sun-solaris2.11, x86_64-pc-linux-gnu, and sparc64-unknown-linux-gnu.

[llvmbot]

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

Author: Rainer Orth (rorth)

Changes

Even with the -u __safestack_init link order fixed on Solaris, there are still several safestack test issues left:

• While 540fd42 enabled safestack on Solaris in the driver unconditionally, it ignored that Solaris also exists on SPARC and forgot to enable SPARC support for the runtime lib. This patch fixes that.

• The tests fail to link with undefined references to __sanitizer_internal_memset etc in safestack.cpp.o and interception_linux.cpp.o. These are from indirectly including sanitizer_redefine_builtins.h. Instead of using the implementations from sanitizer_common as was done in [safestack] Various Solaris fixes, this patch disables the interception as discussed in Revert "[safestack] Various Solaris fixes". A similar issue affects 32-bit Linux/sparc where compiling safestack.cpp with -ftrivial-auto-var-init=pattern causes the compiler to generate calls to memset to initialize a pthread_attr_t which is larger than can be handled inline. Both cases are avoided by defining SANITIZER_COMMON_NO_REDEFINE_BUILTINS in the affected sources.

• The pthread*.c tests FAIL with

  safestack CHECK failed: /vol/llvm/src/llvm-project/local/compiler-rt/lib/safestack/safestack.cpp:227 size

  The problem is that pthread_attr_init initializes the stacksize attribute to 0, signifying the default. Unless explicitly overridded, it stays that way. I think this is allowed by XPG7. Since safestack cannot deal with this, I set size to the defaults documented in pthread_create(3C). Unfortunately, there's no macro for those values outside of private libc headers.

• The Solaris syscall interface isn't stable. This is not just a theoretical concern, but the syscalls have changed incompatibly several times in the past. Therefore this patch switches the implementations of TgKill (where SYS_lwp_kill doesn't exist on Solaris 11.4 anyway), Mmap, Munmap, and Mprotect to the same _REAL* solution already used in sanitizer_solaris.cpp.

With those changes, safestack compiles and all tests PASS, so the tests are re-enabled for good.

Tested on amd64-pc-solaris2.11, sparcv9-sun-solaris2.11, x86_64-pc-linux-gnu, and sparc64-unknown-linux-gnu.

---

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

5 Files Affected:

• (modified) compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake (+1-1)
• (modified) compiler-rt/lib/interception/interception_linux.cpp (+2)
• (modified) compiler-rt/lib/safestack/safestack.cpp (+13)
• (modified) compiler-rt/lib/safestack/safestack_platform.h (+28-7)
• (modified) compiler-rt/test/safestack/lit.cfg.py (+1-1)

diff --git a/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake b/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
index c8bec41db36e9..02ff92f693810 100644
--- a/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
+++ b/compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
@@ -77,7 +77,7 @@ set(ALL_UBSAN_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${RISCV64}
     ${MIPS32} ${MIPS64} ${PPC64} ${S390X} ${SPARC} ${SPARCV9} ${HEXAGON}
     ${LOONGARCH64})
 set(ALL_SAFESTACK_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM64} ${MIPS32} ${MIPS64}
-    ${HEXAGON} ${LOONGARCH64})
+    ${HEXAGON} ${LOONGARCH64} ${SPARC} ${SPARCV9})
 set(ALL_CFI_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64} ${MIPS64}
     ${HEXAGON} ${LOONGARCH64})
 set(ALL_SCUDO_STANDALONE_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64}
diff --git a/compiler-rt/lib/interception/interception_linux.cpp b/compiler-rt/lib/interception/interception_linux.cpp
index ef8136eb4fc70..f3b47fa901d67 100644
--- a/compiler-rt/lib/interception/interception_linux.cpp
+++ b/compiler-rt/lib/interception/interception_linux.cpp
@@ -11,6 +11,8 @@
 // Linux-specific interception methods.
 //===----------------------------------------------------------------------===//
 
+#define SANITIZER_COMMON_NO_REDEFINE_BUILTINS
+
 #include "interception.h"
 
 #if SANITIZER_LINUX || SANITIZER_FREEBSD || SANITIZER_NETBSD || \
diff --git a/compiler-rt/lib/safestack/safestack.cpp b/compiler-rt/lib/safestack/safestack.cpp
index 0751f3988b9c1..e662503df72ca 100644
--- a/compiler-rt/lib/safestack/safestack.cpp
+++ b/compiler-rt/lib/safestack/safestack.cpp
@@ -13,6 +13,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#define SANITIZER_COMMON_NO_REDEFINE_BUILTINS
+
 #include "safestack_platform.h"
 #include "safestack_util.h"
 
@@ -224,6 +226,17 @@ INTERCEPTOR(int, pthread_create, pthread_t *thread,
     pthread_attr_destroy(&tmpattr);
   }
 
+#if SANITIZER_SOLARIS
+  // Solaris pthread_attr_init initializes stacksize to 0 (the default), so
+  // hardcode the actual values as documented in pthread_create(3C).
+  if (size == 0)
+#  if defined(_LP64)
+    size = 2 * 1024 * 1024;
+#  else
+    size = 1024 * 1024;
+#  endif
+#endif
+
   SFS_CHECK(size);
   size = RoundUpTo(size, kStackAlign);
 
diff --git a/compiler-rt/lib/safestack/safestack_platform.h b/compiler-rt/lib/safestack/safestack_platform.h
index d4b2e2ef7391c..77eeb9cda6e15 100644
--- a/compiler-rt/lib/safestack/safestack_platform.h
+++ b/compiler-rt/lib/safestack/safestack_platform.h
@@ -17,6 +17,7 @@
 #include "sanitizer_common/sanitizer_platform.h"
 
 #include <dlfcn.h>
+#include <errno.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -68,6 +69,24 @@ static void *GetRealLibcAddress(const char *symbol) {
   SFS_CHECK(real_##func);
 #endif
 
+#if SANITIZER_SOLARIS
+#  define _REAL(func) _##func
+#  define DEFINE__REAL(ret_type, func, ...) \
+    extern "C" ret_type _REAL(func)(__VA_ARGS__)
+
+#  if !defined(_LP64) && _FILE_OFFSET_BITS == 64
+#    define _REAL64(func) _##func##64
+#  else
+#    define _REAL64(func) _REAL(func)
+#  endif
+#  define DEFINE__REAL64(ret_type, func, ...) \
+    extern "C" ret_type _REAL64(func)(__VA_ARGS__)
+
+DEFINE__REAL64(void *, mmap, void *a, size_t b, int c, int d, int e, off_t f);
+DEFINE__REAL(int, munmap, void *a, size_t b);
+DEFINE__REAL(int, mprotect, void *a, size_t b, int c);
+#endif
+
 using ThreadId = uint64_t;
 
 inline ThreadId GetTid() {
@@ -91,11 +110,10 @@ inline int TgKill(pid_t pid, ThreadId tid, int sig) {
   (void)pid;
   return _REAL(_lwp_kill, tid, sig);
 #elif SANITIZER_SOLARIS
-#  ifdef SYS_lwp_kill
-  return syscall(SYS_lwp_kill, tid, sig);
-#  else
-  return -1;
-#  endif
+  (void)pid;
+  errno = thr_kill(tid, sig);
+  // TgKill is expected to return -1 on error, not an errno.
+  return errno != 0 ? -1 : 0;
 #elif SANITIZER_FREEBSD
   return syscall(SYS_thr_kill2, pid, tid, sig);
 #else
@@ -110,8 +128,7 @@ inline void *Mmap(void *addr, size_t length, int prot, int flags, int fd,
 #elif SANITIZER_FREEBSD && (defined(__aarch64__) || defined(__x86_64__))
   return (void *)__syscall(SYS_mmap, addr, length, prot, flags, fd, offset);
 #elif SANITIZER_SOLARIS
-  return (void *)(uintptr_t)syscall(SYS_mmap, addr, length, prot, flags, fd,
-                                    offset);
+  return _REAL64(mmap)(addr, length, prot, flags, fd, offset);
 #else
   return (void *)syscall(SYS_mmap, addr, length, prot, flags, fd, offset);
 #endif
@@ -121,6 +138,8 @@ inline int Munmap(void *addr, size_t length) {
 #if SANITIZER_NETBSD
   DEFINE__REAL(int, munmap, void *a, size_t b);
   return _REAL(munmap, addr, length);
+#elif SANITIZER_SOLARIS
+  return _REAL(munmap)(addr, length);
 #else
   return syscall(SYS_munmap, addr, length);
 #endif
@@ -130,6 +149,8 @@ inline int Mprotect(void *addr, size_t length, int prot) {
 #if SANITIZER_NETBSD
   DEFINE__REAL(int, mprotect, void *a, size_t b, int c);
   return _REAL(mprotect, addr, length, prot);
+#elif SANITIZER_SOLARIS
+  return _REAL(mprotect)(addr, length, prot);
 #else
   return syscall(SYS_mprotect, addr, length, prot);
 #endif
diff --git a/compiler-rt/test/safestack/lit.cfg.py b/compiler-rt/test/safestack/lit.cfg.py
index aadb8bf0d5c77..17dfae46a412b 100644
--- a/compiler-rt/test/safestack/lit.cfg.py
+++ b/compiler-rt/test/safestack/lit.cfg.py
@@ -33,5 +33,5 @@
         )
     )
 
-if config.host_os not in ["Linux", "FreeBSD", "NetBSD"]:
+if config.host_os not in ["Linux", "FreeBSD", "NetBSD", "SunOS"]:
     config.unsupported = True

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff fc9cd3272b50f4ee9f18c4ab82c278bbb014d99f 33e0aa19791038782e217c03e2fa7276c6a12b4e --extensions cpp,h -- compiler-rt/lib/safestack/safestack.cpp compiler-rt/lib/safestack/safestack_platform.h

View the diff from clang-format here.

diff --git a/compiler-rt/lib/safestack/safestack.cpp b/compiler-rt/lib/safestack/safestack.cpp
index f01a642987..81ca194dc5 100644
--- a/compiler-rt/lib/safestack/safestack.cpp
+++ b/compiler-rt/lib/safestack/safestack.cpp
@@ -15,15 +15,14 @@
 
 #define SANITIZER_COMMON_NO_REDEFINE_BUILTINS
 
-#include "safestack_platform.h"
-#include "safestack_util.h"
-#include "sanitizer_common/sanitizer_internal_defs.h"
-
 #include <errno.h>
 #include <string.h>
 #include <sys/resource.h>
 
 #include "interception/interception.h"
+#include "safestack_platform.h"
+#include "safestack_util.h"
+#include "sanitizer_common/sanitizer_internal_defs.h"
 
 // interception.h drags in sanitizer_redefine_builtins.h, which in turn
 // creates references to __sanitizer_internal_memcpy etc.  The interceptors
diff --git a/compiler-rt/lib/safestack/safestack_platform.h b/compiler-rt/lib/safestack/safestack_platform.h
index 77eeb9cda6..27e29d6ead 100644
--- a/compiler-rt/lib/safestack/safestack_platform.h
+++ b/compiler-rt/lib/safestack/safestack_platform.h
@@ -13,9 +13,6 @@
 #ifndef SAFESTACK_PLATFORM_H
 #define SAFESTACK_PLATFORM_H
 
-#include "safestack_util.h"
-#include "sanitizer_common/sanitizer_platform.h"
-
 #include <dlfcn.h>
 #include <errno.h>
 #include <stdint.h>
@@ -26,6 +23,9 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include "safestack_util.h"
+#include "sanitizer_common/sanitizer_platform.h"
+
 #if !(SANITIZER_NETBSD || SANITIZER_FREEBSD || SANITIZER_LINUX || \
       SANITIZER_SOLARIS)
 #  error "Support for your platform has not been implemented"

[vitalybuka]

LGTM if undo interception

__sanitizer_internal_mem* fill free to do either here or another patch