[compiler-rt] [safestack] Various Solaris fixes (PR #99290)

Rainer Orth via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 17 01:11:51 PDT 2024


https://github.com/rorth created https://github.com/llvm/llvm-project/pull/99290

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

- While 540fd42c755f20f7b79c6c79493ec36d8cb9b3d3 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](https://github.com/llvm/llvm-project/pull/98469), this patch disables the interception as discussed in [Revert "[safestack] Various Solaris fixes"](https://github.com/llvm/llvm-project/pull/98541).  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`.

>From 916d9323ada65f55538df5db70c68ecf87916ca4 Mon Sep 17 00:00:00 2001
From: Rainer Orth <ro at gcc.gnu.org>
Date: Wed, 17 Jul 2024 10:08:44 +0200
Subject: [PATCH] [safestack] Various Solaris fixes

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

- While 540fd42c755f20f7b79c6c79493ec36d8cb9b3d3 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](https://github.com/llvm/llvm-project/pull/98469), this patch
  disables the interception as discussed in [Revert "[safestack] Various
  Solaris fixes"](https://github.com/llvm/llvm-project/pull/98541).  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`.
---
 .../cmake/Modules/AllSupportedArchDefs.cmake  |  2 +-
 .../lib/interception/interception_linux.cpp   |  2 ++
 compiler-rt/lib/safestack/safestack.cpp       | 13 +++++++
 .../lib/safestack/safestack_platform.h        | 35 +++++++++++++++----
 compiler-rt/test/safestack/lit.cfg.py         |  2 +-
 5 files changed, 45 insertions(+), 9 deletions(-)

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



More information about the llvm-commits mailing list