Skip to content

Commit

Permalink
[safestack] Various Solaris fixes (llvm#99290)
Browse files Browse the repository at this point in the history
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](llvm#98469), this patch
disables the interception as discussed in [Revert "[safestack] Various
Solaris fixes"](llvm#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. 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`.
  • Loading branch information
rorth authored and Harini0924 committed Jul 22, 2024
1 parent fb3a3af commit c7f90f7
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 9 deletions.
2 changes: 1 addition & 1 deletion compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
36 changes: 36 additions & 0 deletions compiler-rt/lib/safestack/safestack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,39 @@
//
//===----------------------------------------------------------------------===//

#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"

// interception.h drags in sanitizer_redefine_builtins.h, which in turn
// creates references to __sanitizer_internal_memcpy etc. The interceptors
// aren't needed here, so just forward to libc.
extern "C" {
SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memcpy(void *dest,
const void *src,
size_t n) {
return memcpy(dest, src, n);
}

SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memmove(
void *dest, const void *src, size_t n) {
return memmove(dest, src, n);
}

SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memset(void *s, int c,
size_t n) {
return memset(s, c, n);
}
} // extern "C"

using namespace safestack;

// TODO: To make accessing the unsafe stack pointer faster, we plan to
Expand Down Expand Up @@ -224,6 +249,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);

Expand Down
35 changes: 28 additions & 7 deletions compiler-rt/lib/safestack/safestack_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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() {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler-rt/test/safestack/lit.cfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit c7f90f7

Please sign in to comment.