[compiler-rt] [safestack] Switch to sanitizer_common functions (PR #98513)
Rainer Orth via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 11 11:12:07 PDT 2024
https://github.com/rorth created https://github.com/llvm/llvm-project/pull/98513
safestack currently uses its own portability layer in `safestack_platform.h`. This is both wasteful and leads to errors easily avoided by using the well-tested functions in `sanitizer_common` instead. This is particularly notable when [[safestack] Support multilib testing](https://github.com/llvm/llvm-project/pull/98002) is applied.
E.g. all Linux/i386 tests `FAIL`ed with
```
safestack CHECK failed: /vol/llvm/src/llvm-project/local/compiler-rt/lib/safestack/safestack.cpp:95 MAP_FAILED != addr
```
because the safestack `mmap` implementation doesn't work there.
Thus this patch switches safestack to use the `sanitizer_common` code instead. It's mostly obvious, with the exception of `TgKill`:
- The `sanitizer_common` Linux `TgKill` implementation is wrong: while safestack used `syscall(SYS_tgkill)`, `sanitizer_common` has `internal_syscall(SYSCALL(tgkill))` instead. However, the latter (based on using the `syscall` insn directly on x86_64) returns `errno` instead of the expeccted `-1`. Besides, `TgKill` had to be moved to `sanitizer_linux_libcdep.cpp`, otherwise the `Sanitizer-x86_64-Test-Nolibc` test failed to link with an undefined reference to `syscall`.
- On Solaris, a similar issue with `thr_kill` went unnoticed before because `sanitizer_common` `TgKill` wasn't used. [[sanitizer_common] Fix TgKill on Solaris](https://github.com/llvm/llvm-project/pull/98000) was merged into this patch.
- On 32-bit Linux/sparc, `pthread-cleanup.c` would `FAIL` because a `tid_t` (`uint64_t`) `tid` arg was passed to `syscall(SYS_tgkill)` while `tid` is actually a `pid_t` (`int`).
Tested on `amd64-pc-solaris2.11`, `sparcv9-sun-solaris2.11`, `x86_64-pc-solaris2.11`, and `sparc64-unknown-linux-gnu`.
>From 3533f4b0fcf30038ae9697a4e008c64855e232b7 Mon Sep 17 00:00:00 2001
From: Rainer Orth <ro at gcc.gnu.org>
Date: Thu, 11 Jul 2024 17:00:08 +0200
Subject: [PATCH] [safestack] Switch to sanitizer_common functions
safestack currently uses its own portability layer in
`safestack_platform.h`. This is both wasteful and leads to errors easily
avoided by using the well-tested functions in `sanitizer_common` instead.
This is particularly notable when ([safestack] Support multilib testing
E.g. all Linux/i386 tests `FAIL`ed with
```
safestack CHECK failed: /vol/llvm/src/llvm-project/local/compiler-rt/lib/safestack/safestack.cpp:95 MAP_FAILED != addr
```
because the safestack `mmap` implementation doesn't work there.
Thus this patch switches safestack to use the `sanitizer_common` code
instead. It's mostly obvious, with the exception of `TgKill`:
- The `sanitizer_common` Linux `TgKill` implementation is wrong: while
safestack used `syscall(SYS_tgkill)`, `sanitizer_common` has
`internal_syscall(SYSCALL(tgkill))` instead. However, the latter (based
on using the `syscall` insn directly on x86_64) returns `errno` instead
of the expeccted `-1`. Besides, `TgKill` had to be moved to
`sanitizer_linux_libcdep.cpp`, otherwise the
`Sanitizer-x86_64-Test-Nolibc` test failed to link with an undefined
reference to `syscall`.
- On Solaris, a similar issue with `thr_kill` went unnoticed before because
`sanitizer_common` `TgKill` wasn't used. ([sanitizer_common] Fix TgKill
on Solaris)[https://github.com/llvm/llvm-project/pull/98000] was merged
into this patch.
- On 32-bit Linux/sparc, `pthread-cleanup.c` would `FAIL` because a `tid_t`
(`uint64_t`) `tid` arg was passed to `syscall(SYS_tgkill)` while `tid` is
actually a `pid_t` (`int`).
Tested on `amd64-pc-solaris2.11`, `sparcv9-sun-solaris2.11`,
`x86_64-pc-solaris2.11`, and `sparc64-unknown-linux-gnu`.
---
compiler-rt/lib/safestack/safestack.cpp | 23 ++-
.../lib/safestack/safestack_platform.h | 161 ------------------
compiler-rt/lib/safestack/safestack_util.h | 5 -
.../lib/sanitizer_common/sanitizer_linux.cpp | 11 --
.../sanitizer_linux_libcdep.cpp | 19 +++
5 files changed, 33 insertions(+), 186 deletions(-)
delete mode 100644 compiler-rt/lib/safestack/safestack_platform.h
diff --git a/compiler-rt/lib/safestack/safestack.cpp b/compiler-rt/lib/safestack/safestack.cpp
index f8ef224f45494..5ef5e77ee8fb4 100644
--- a/compiler-rt/lib/safestack/safestack.cpp
+++ b/compiler-rt/lib/safestack/safestack.cpp
@@ -13,14 +13,18 @@
//
//===----------------------------------------------------------------------===//
-#include "safestack_platform.h"
-#include "safestack_util.h"
-
#include <errno.h>
+#include <sys/mman.h>
#include <sys/resource.h>
+#include <unistd.h>
#include "interception/interception.h"
+#include "safestack_util.h"
+#include "sanitizer_common/sanitizer_common.h"
+#include "sanitizer_common/sanitizer_platform.h"
+#include "sanitizer_common/sanitizer_posix.h"
+using namespace __sanitizer;
using namespace safestack;
// TODO: To make accessing the unsafe stack pointer faster, we plan to
@@ -90,10 +94,11 @@ __thread size_t unsafe_stack_guard = 0;
inline void *unsafe_stack_alloc(size_t size, size_t guard) {
SFS_CHECK(size + guard >= size);
- void *addr = Mmap(nullptr, size + guard, PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANON, -1, 0);
+ void *addr =
+ (void *)internal_mmap(nullptr, size + guard, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANON, -1, 0);
SFS_CHECK(MAP_FAILED != addr);
- Mprotect(addr, guard, PROT_NONE);
+ internal_mprotect(addr, guard, PROT_NONE);
return (char *)addr + guard;
}
@@ -146,7 +151,7 @@ struct thread_stack_ll {
void *stack_base;
size_t size;
pid_t pid;
- ThreadId tid;
+ tid_t tid;
};
/// Linked list of unsafe stacks for threads that are exiting. We delay
@@ -169,7 +174,7 @@ void thread_cleanup_handler(void *_iter) {
pthread_mutex_unlock(&thread_stacks_mutex);
pid_t pid = getpid();
- ThreadId tid = GetTid();
+ tid_t tid = GetTid();
// Free stacks for dead threads
thread_stack_ll **stackp = &temp_stacks;
@@ -177,7 +182,7 @@ void thread_cleanup_handler(void *_iter) {
thread_stack_ll *stack = *stackp;
if (stack->pid != pid ||
(-1 == TgKill(stack->pid, stack->tid, 0) && errno == ESRCH)) {
- Munmap(stack->stack_base, stack->size);
+ internal_munmap(stack->stack_base, stack->size);
*stackp = stack->next;
free(stack);
} else
diff --git a/compiler-rt/lib/safestack/safestack_platform.h b/compiler-rt/lib/safestack/safestack_platform.h
deleted file mode 100644
index 77eeb9cda6e15..0000000000000
--- a/compiler-rt/lib/safestack/safestack_platform.h
+++ /dev/null
@@ -1,161 +0,0 @@
-//===-- safestack_platform.h ----------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file implements platform specific parts of SafeStack runtime.
-//
-//===----------------------------------------------------------------------===//
-
-#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>
-#include <stdio.h>
-#include <stdlib.h>
-#include <sys/mman.h>
-#include <sys/syscall.h>
-#include <sys/types.h>
-#include <unistd.h>
-
-#if !(SANITIZER_NETBSD || SANITIZER_FREEBSD || SANITIZER_LINUX || \
- SANITIZER_SOLARIS)
-# error "Support for your platform has not been implemented"
-#endif
-
-#if SANITIZER_NETBSD
-#include <lwp.h>
-
-extern "C" void *__mmap(void *, size_t, int, int, int, int, off_t);
-#endif
-
-#if SANITIZER_FREEBSD
-#include <sys/thr.h>
-#endif
-
-#if SANITIZER_SOLARIS
-# include <thread.h>
-#endif
-
-namespace safestack {
-
-#if SANITIZER_NETBSD
-static void *GetRealLibcAddress(const char *symbol) {
- void *real = dlsym(RTLD_NEXT, symbol);
- if (!real)
- real = dlsym(RTLD_DEFAULT, symbol);
- if (!real) {
- fprintf(stderr, "safestack GetRealLibcAddress failed for symbol=%s",
- symbol);
- abort();
- }
- return real;
-}
-
-#define _REAL(func, ...) real##_##func(__VA_ARGS__)
-#define DEFINE__REAL(ret_type, func, ...) \
- static ret_type (*real_##func)(__VA_ARGS__) = NULL; \
- if (!real_##func) { \
- real_##func = (ret_type(*)(__VA_ARGS__))GetRealLibcAddress(#func); \
- } \
- 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() {
-#if SANITIZER_NETBSD
- DEFINE__REAL(int, _lwp_self);
- return _REAL(_lwp_self);
-#elif SANITIZER_FREEBSD
- long Tid;
- thr_self(&Tid);
- return Tid;
-#elif SANITIZER_SOLARIS
- return thr_self();
-#else
- return syscall(SYS_gettid);
-#endif
-}
-
-inline int TgKill(pid_t pid, ThreadId tid, int sig) {
-#if SANITIZER_NETBSD
- DEFINE__REAL(int, _lwp_kill, int a, int b);
- (void)pid;
- return _REAL(_lwp_kill, tid, sig);
-#elif SANITIZER_SOLARIS
- (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
- return syscall(SYS_tgkill, pid, tid, sig);
-#endif
-}
-
-inline void *Mmap(void *addr, size_t length, int prot, int flags, int fd,
- off_t offset) {
-#if SANITIZER_NETBSD
- return __mmap(addr, length, prot, flags, fd, 0, offset);
-#elif SANITIZER_FREEBSD && (defined(__aarch64__) || defined(__x86_64__))
- return (void *)__syscall(SYS_mmap, addr, length, prot, flags, fd, offset);
-#elif SANITIZER_SOLARIS
- return _REAL64(mmap)(addr, length, prot, flags, fd, offset);
-#else
- return (void *)syscall(SYS_mmap, addr, length, prot, flags, fd, offset);
-#endif
-}
-
-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
-}
-
-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
-}
-
-} // namespace safestack
-
-#endif // SAFESTACK_PLATFORM_H
diff --git a/compiler-rt/lib/safestack/safestack_util.h b/compiler-rt/lib/safestack/safestack_util.h
index da3f11b54cabc..9f571b36e08c4 100644
--- a/compiler-rt/lib/safestack/safestack_util.h
+++ b/compiler-rt/lib/safestack/safestack_util.h
@@ -28,11 +28,6 @@ namespace safestack {
}; \
} while (false)
-inline size_t RoundUpTo(size_t size, size_t boundary) {
- SFS_CHECK((boundary & (boundary - 1)) == 0);
- return (size + boundary - 1) & ~(boundary - 1);
-}
-
class MutexLock {
public:
explicit MutexLock(pthread_mutex_t &mutex) : mutex_(&mutex) {
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
index f7f5698a5f32d..172a78e5a4e78 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
@@ -566,17 +566,6 @@ tid_t GetTid() {
return internal_syscall(SYSCALL(gettid));
# endif
}
-
-int TgKill(pid_t pid, tid_t tid, int sig) {
-# if SANITIZER_LINUX
- return internal_syscall(SYSCALL(tgkill), pid, tid, sig);
-# elif SANITIZER_FREEBSD
- return internal_syscall(SYSCALL(thr_kill2), pid, tid, sig);
-# elif SANITIZER_SOLARIS
- (void)pid;
- return thr_kill(tid, sig);
-# endif
-}
# endif
# if SANITIZER_GLIBC
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
index c3c717bbdbe4c..38226fbf6d164 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
@@ -33,11 +33,13 @@
# endif
# include <dlfcn.h> // for dlsym()
+# include <errno.h>
# include <link.h>
# include <pthread.h>
# include <signal.h>
# include <sys/mman.h>
# include <sys/resource.h>
+# include <sys/syscall.h>
# include <syslog.h>
# if !defined(ElfW)
@@ -113,6 +115,23 @@ int internal_sigaction(int signum, const void *act, void *oldact) {
# endif
}
+# if !SANITIZER_NETBSD
+int TgKill(pid_t pid, tid_t tid, int sig) {
+# if SANITIZER_LINUX
+ // Cannot use internal_syscall here: returns -errno instead of -1 on failure
+ // on x86_64.
+ return syscall(SYS_tgkill, pid, (pid_t)tid, sig);
+# elif SANITIZER_FREEBSD
+ return internal_syscall(SYSCALL(thr_kill2), pid, tid, sig);
+# elif SANITIZER_SOLARIS
+ (void)pid;
+ errno = thr_kill(tid, sig);
+ // TgKill is expected to return -1 on error, not an errno.
+ return errno != 0 ? -1 : 0;
+# endif
+}
+# endif
+
void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top,
uptr *stack_bottom) {
CHECK(stack_top);
More information about the llvm-commits
mailing list