[compiler-rt] ab6263c - Revert 502f0cc0e38 "[GWP-ASan] Split the unwinder into segv/non-segv."
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 21 02:18:23 PDT 2020
Author: Hans Wennborg
Date: 2020-07-21T11:18:07+02:00
New Revision: ab6263c9258ccd0e3c9cb4f675979f22cfba6131
URL: https://github.com/llvm/llvm-project/commit/ab6263c9258ccd0e3c9cb4f675979f22cfba6131
DIFF: https://github.com/llvm/llvm-project/commit/ab6263c9258ccd0e3c9cb4f675979f22cfba6131.diff
LOG: Revert 502f0cc0e38 "[GWP-ASan] Split the unwinder into segv/non-segv."
It was causing tests to fail in -DCOMPILER_RT_BUILD_BUILTINS=OFF builds:
GwpAsan-Unittest :: ./GwpAsan-x86_64-Test/BacktraceGuardedPoolAllocator.DoubleFree
GwpAsan-Unittest :: ./GwpAsan-x86_64-Test/BacktraceGuardedPoolAllocator.UseAfterFree
see comment on the code review.
> Summary:
> Splits the unwinder into a non-segv (for allocation/deallocation traces) and a
> segv unwinder. This ensures that implementations can select an accurate, slower
> unwinder in the segv handler (if they choose to use the GWP-ASan provided one).
> This is important as fast frame-pointer unwinders (like the sanitizer unwinder)
> don't like unwinding through signal handlers.
>
> Reviewers: morehouse, cryptoad
>
> Reviewed By: morehouse, cryptoad
>
> Subscribers: cryptoad, mgorny, eugenis, pcc, #sanitizers
>
> Tags: #sanitizers
>
> Differential Revision: https://reviews.llvm.org/D83994
This reverts commit 502f0cc0e3889229e923e187f38dda91324ae139.
Added:
Modified:
compiler-rt/lib/gwp_asan/optional/backtrace_linux_libc.cpp
compiler-rt/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp
compiler-rt/lib/gwp_asan/optional/segv_handler.h
compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
compiler-rt/lib/gwp_asan/tests/harness.h
compiler-rt/lib/scudo/scudo_allocator.cpp
compiler-rt/lib/scudo/standalone/combined.h
Removed:
compiler-rt/test/gwp_asan/backtrace.c
################################################################################
diff --git a/compiler-rt/lib/gwp_asan/optional/backtrace_linux_libc.cpp b/compiler-rt/lib/gwp_asan/optional/backtrace_linux_libc.cpp
index 92eb293dab49..bb0aad224a14 100644
--- a/compiler-rt/lib/gwp_asan/optional/backtrace_linux_libc.cpp
+++ b/compiler-rt/lib/gwp_asan/optional/backtrace_linux_libc.cpp
@@ -23,14 +23,6 @@ size_t Backtrace(uintptr_t *TraceBuffer, size_t Size) {
return backtrace(reinterpret_cast<void **>(TraceBuffer), Size);
}
-// We don't need any custom handling for the Segv backtrace - the libc unwinder
-// has no problems with unwinding through a signal handler. Force inlining here
-// to avoid the additional frame.
-GWP_ASAN_ALWAYS_INLINE size_t SegvBacktrace(uintptr_t *TraceBuffer, size_t Size,
- void * /*Context*/) {
- return Backtrace(TraceBuffer, Size);
-}
-
static void PrintBacktrace(uintptr_t *Trace, size_t TraceLength,
gwp_asan::crash_handler::Printf_t Printf) {
if (TraceLength == 0) {
@@ -61,8 +53,4 @@ crash_handler::PrintBacktrace_t getPrintBacktraceFunction() {
return PrintBacktrace;
}
} // namespace options
-
-namespace crash_handler {
-SegvBacktrace_t getSegvBacktraceFunction() { return SegvBacktrace; }
-} // namespace crash_handler
} // namespace gwp_asan
diff --git a/compiler-rt/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp b/compiler-rt/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp
index a8083e4e64cb..3ac4b52bfc27 100644
--- a/compiler-rt/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp
+++ b/compiler-rt/lib/gwp_asan/optional/backtrace_sanitizer_common.cpp
@@ -22,45 +22,28 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(uptr pc, uptr bp,
void *context,
bool request_fast,
u32 max_depth) {
- if (!StackTrace::WillUseFastUnwind(request_fast))
- return Unwind(max_depth, pc, 0, context, 0, 0, false);
-
- uptr top = 0;
- uptr bottom = 0;
- GetThreadStackTopAndBottom(/*at_initialization*/ false, &top, &bottom);
-
- return Unwind(max_depth, pc, bp, context, top, bottom, request_fast);
+ if (!StackTrace::WillUseFastUnwind(request_fast)) {
+ return Unwind(max_depth, pc, bp, context, 0, 0, request_fast);
+ }
+ Unwind(max_depth, pc, 0, context, 0, 0, false);
}
namespace {
-size_t BacktraceCommon(uintptr_t *TraceBuffer, size_t Size, void *Context) {
- // Use the slow sanitizer unwinder in the segv handler. Fast frame pointer
- // unwinders can end up dropping frames because the kernel sigreturn() frame's
- // return address is the return address at time of fault. This has the result
- // of never actually capturing the PC where the signal was raised.
- bool UseFastUnwind = (Context == nullptr);
-
+size_t Backtrace(uintptr_t *TraceBuffer, size_t Size) {
__sanitizer::BufferedStackTrace Trace;
Trace.Reset();
if (Size > __sanitizer::kStackTraceMax)
Size = __sanitizer::kStackTraceMax;
Trace.Unwind((__sanitizer::uptr)__builtin_return_address(0),
- (__sanitizer::uptr)__builtin_frame_address(0), Context,
- UseFastUnwind, Size - 1);
+ (__sanitizer::uptr)__builtin_frame_address(0),
+ /* ucontext */ nullptr,
+ /* fast unwind */ true, Size - 1);
memcpy(TraceBuffer, Trace.trace, Trace.size * sizeof(uintptr_t));
return Trace.size;
}
-size_t Backtrace(uintptr_t *TraceBuffer, size_t Size) {
- return BacktraceCommon(TraceBuffer, Size, nullptr);
-}
-
-size_t SegvBacktrace(uintptr_t *TraceBuffer, size_t Size, void *Context) {
- return BacktraceCommon(TraceBuffer, Size, Context);
-}
-
static void PrintBacktrace(uintptr_t *Trace, size_t TraceLength,
gwp_asan::crash_handler::Printf_t Printf) {
__sanitizer::StackTrace StackTrace;
@@ -94,8 +77,4 @@ crash_handler::PrintBacktrace_t getPrintBacktraceFunction() {
return PrintBacktrace;
}
} // namespace options
-
-namespace crash_handler {
-SegvBacktrace_t getSegvBacktraceFunction() { return SegvBacktrace; }
-} // namespace crash_handler
} // namespace gwp_asan
diff --git a/compiler-rt/lib/gwp_asan/optional/segv_handler.h b/compiler-rt/lib/gwp_asan/optional/segv_handler.h
index 0fed4f2e012e..10af15055e2a 100644
--- a/compiler-rt/lib/gwp_asan/optional/segv_handler.h
+++ b/compiler-rt/lib/gwp_asan/optional/segv_handler.h
@@ -59,15 +59,6 @@ typedef void (*PrintBacktrace_t)(uintptr_t *TraceBuffer, size_t TraceLength,
// without any symbolization.
PrintBacktrace_t getBasicPrintBacktraceFunction();
-// Returns a function pointer to a backtrace function that's suitable for
-// unwinding through a signal handler. This is important primarily for frame-
-// pointer based unwinders, DWARF or other unwinders can simply provide the
-// normal backtrace function as the implementation here. On POSIX, SignalContext
-// should be the `ucontext_t` from the signal handler.
-typedef size_t (*SegvBacktrace_t)(uintptr_t *TraceBuffer, size_t Size,
- void *SignalContext);
-SegvBacktrace_t getSegvBacktraceFunction();
-
// Install the SIGSEGV crash handler for printing use-after-free and heap-
// buffer-{under|over}flow exceptions if the user asked for it. This is platform
// specific as even though POSIX and Windows both support registering handlers
@@ -76,14 +67,14 @@ SegvBacktrace_t getSegvBacktraceFunction();
// before this function.
void installSignalHandlers(gwp_asan::GuardedPoolAllocator *GPA, Printf_t Printf,
PrintBacktrace_t PrintBacktrace,
- SegvBacktrace_t SegvBacktrace);
+ options::Backtrace_t Backtrace);
void uninstallSignalHandlers();
void dumpReport(uintptr_t ErrorPtr, const gwp_asan::AllocatorState *State,
const gwp_asan::AllocationMetadata *Metadata,
- SegvBacktrace_t SegvBacktrace, Printf_t Printf,
- PrintBacktrace_t PrintBacktrace, void *Context);
+ options::Backtrace_t Backtrace, Printf_t Printf,
+ PrintBacktrace_t PrintBacktrace);
} // namespace crash_handler
} // namespace gwp_asan
diff --git a/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp b/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
index 1bd7a606c213..22589b893604 100644
--- a/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
+++ b/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
@@ -23,14 +23,14 @@ using gwp_asan::Error;
using gwp_asan::GuardedPoolAllocator;
using gwp_asan::crash_handler::PrintBacktrace_t;
using gwp_asan::crash_handler::Printf_t;
-using gwp_asan::crash_handler::SegvBacktrace_t;
+using gwp_asan::options::Backtrace_t;
struct sigaction PreviousHandler;
bool SignalHandlerInstalled;
gwp_asan::GuardedPoolAllocator *GPAForSignalHandler;
Printf_t PrintfForSignalHandler;
PrintBacktrace_t PrintBacktraceForSignalHandler;
-SegvBacktrace_t BacktraceForSignalHandler;
+Backtrace_t BacktraceForSignalHandler;
static void sigSegvHandler(int sig, siginfo_t *info, void *ucontext) {
if (GPAForSignalHandler) {
@@ -40,7 +40,7 @@ static void sigSegvHandler(int sig, siginfo_t *info, void *ucontext) {
reinterpret_cast<uintptr_t>(info->si_addr),
GPAForSignalHandler->getAllocatorState(),
GPAForSignalHandler->getMetadataRegion(), BacktraceForSignalHandler,
- PrintfForSignalHandler, PrintBacktraceForSignalHandler, ucontext);
+ PrintfForSignalHandler, PrintBacktraceForSignalHandler);
}
// Process any previous handlers.
@@ -138,11 +138,11 @@ PrintBacktrace_t getBasicPrintBacktraceFunction() {
void installSignalHandlers(gwp_asan::GuardedPoolAllocator *GPA, Printf_t Printf,
PrintBacktrace_t PrintBacktrace,
- SegvBacktrace_t SegvBacktrace) {
+ options::Backtrace_t Backtrace) {
GPAForSignalHandler = GPA;
PrintfForSignalHandler = Printf;
PrintBacktraceForSignalHandler = PrintBacktrace;
- BacktraceForSignalHandler = SegvBacktrace;
+ BacktraceForSignalHandler = Backtrace;
struct sigaction Action;
Action.sa_sigaction = sigSegvHandler;
@@ -160,8 +160,8 @@ void uninstallSignalHandlers() {
void dumpReport(uintptr_t ErrorPtr, const gwp_asan::AllocatorState *State,
const gwp_asan::AllocationMetadata *Metadata,
- SegvBacktrace_t SegvBacktrace, Printf_t Printf,
- PrintBacktrace_t PrintBacktrace, void *Context) {
+ options::Backtrace_t Backtrace, Printf_t Printf,
+ PrintBacktrace_t PrintBacktrace) {
assert(State && "dumpReport missing Allocator State.");
assert(Metadata && "dumpReport missing Metadata.");
assert(Printf && "dumpReport missing Printf.");
@@ -194,8 +194,7 @@ void dumpReport(uintptr_t ErrorPtr, const gwp_asan::AllocatorState *State,
// Print the fault backtrace.
static constexpr unsigned kMaximumStackFramesForCrashTrace = 512;
uintptr_t Trace[kMaximumStackFramesForCrashTrace];
- size_t TraceLength =
- SegvBacktrace(Trace, kMaximumStackFramesForCrashTrace, Context);
+ size_t TraceLength = Backtrace(Trace, kMaximumStackFramesForCrashTrace);
PrintBacktrace(Trace, TraceLength, Printf);
diff --git a/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt b/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
index f88d90c19d5b..feac23df9fe5 100644
--- a/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
@@ -5,8 +5,7 @@ set(GWP_ASAN_UNITTEST_CFLAGS
${COMPILER_RT_GTEST_CFLAGS}
-I${COMPILER_RT_SOURCE_DIR}/lib/
-O2
- -g
- -fno-omit-frame-pointer)
+ -g)
file(GLOB GWP_ASAN_HEADERS ../*.h)
set(GWP_ASAN_UNITTESTS
diff --git a/compiler-rt/lib/gwp_asan/tests/harness.h b/compiler-rt/lib/gwp_asan/tests/harness.h
index d303b2cfa647..e47254e13c46 100644
--- a/compiler-rt/lib/gwp_asan/tests/harness.h
+++ b/compiler-rt/lib/gwp_asan/tests/harness.h
@@ -86,8 +86,7 @@ class BacktraceGuardedPoolAllocator : public ::testing::Test {
gwp_asan::crash_handler::installSignalHandlers(
&GPA, gwp_asan::test::getPrintfFunction(),
- gwp_asan::options::getPrintBacktraceFunction(),
- gwp_asan::crash_handler::getSegvBacktraceFunction());
+ gwp_asan::options::getPrintBacktraceFunction(), Opts.Backtrace);
}
void TearDown() override {
diff --git a/compiler-rt/lib/scudo/scudo_allocator.cpp b/compiler-rt/lib/scudo/scudo_allocator.cpp
index 343f85a4ef88..d9023c2f7ab6 100644
--- a/compiler-rt/lib/scudo/scudo_allocator.cpp
+++ b/compiler-rt/lib/scudo/scudo_allocator.cpp
@@ -29,7 +29,6 @@
# include "gwp_asan/guarded_pool_allocator.h"
# include "gwp_asan/optional/backtrace.h"
# include "gwp_asan/optional/options_parser.h"
-#include "gwp_asan/optional/segv_handler.h"
#endif // GWP_ASAN_HOOKS
#include <errno.h>
@@ -680,8 +679,7 @@ void initScudo() {
if (Opts.InstallSignalHandlers)
gwp_asan::crash_handler::installSignalHandlers(
&GuardedAlloc, __sanitizer::Printf,
- gwp_asan::options::getPrintBacktraceFunction(),
- gwp_asan::crash_handler::getSegvBacktraceFunction());
+ gwp_asan::options::getPrintBacktraceFunction(), Opts.Backtrace);
#endif // GWP_ASAN_HOOKS
}
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index ae085befc4f1..3bb41eca88f7 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -192,7 +192,7 @@ class Allocator {
if (Opt.InstallSignalHandlers)
gwp_asan::crash_handler::installSignalHandlers(
&GuardedAlloc, Printf, gwp_asan::options::getPrintBacktraceFunction(),
- gwp_asan::crash_handler::getSegvBacktraceFunction());
+ Opt.Backtrace);
#endif // GWP_ASAN_HOOKS
}
diff --git a/compiler-rt/test/gwp_asan/backtrace.c b/compiler-rt/test/gwp_asan/backtrace.c
deleted file mode 100644
index 0ba32f85cbf9..000000000000
--- a/compiler-rt/test/gwp_asan/backtrace.c
+++ /dev/null
@@ -1,29 +0,0 @@
-// REQUIRES: gwp_asan
-// RUN: %clang_gwp_asan %s -g -o %t
-// RUN: %expect_crash %t 2>&1 | FileCheck %s
-
-#include <stdlib.h>
-
-__attribute__((noinline)) void *allocate_mem() { return malloc(1); }
-
-__attribute__((noinline)) void free_mem(void *ptr) { free(ptr); }
-
-__attribute__((noinline)) void touch_mem(void *ptr) {
- volatile char sink = *((volatile char *)ptr);
-}
-
-// CHECK: Use After Free
-// CHECK: touch_mem
-// CHECK: was deallocated
-// CHECK: free_mem
-// CHECK: was allocated
-// CHECK: allocate_mem
-
-int main() {
- for (unsigned i = 0; i < 0x10000; ++i) {
- void *ptr = allocate_mem();
- free_mem(ptr);
- touch_mem(ptr);
- }
- return 0;
-}
More information about the llvm-commits
mailing list