diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index efdae02a..30a4576b 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -867,6 +867,14 @@ bool Profiler::crashHandler(int signo, siginfo_t *siginfo, void *ucontext) { // we are already in a crash handler; don't recurse! return false; } + + if (SafeAccess::handle_safefetch(signo, ucontext)) { + if (thrd != nullptr) { + thrd->exitCrashHandler(); + } + return true; + } + uintptr_t fault_address = (uintptr_t)siginfo->si_addr; StackFrame frame(ucontext); uintptr_t pc = frame.pc(); diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 82aceeeb..2fea8de3 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -16,23 +16,105 @@ #include "safeAccess.h" +#include +#include -SafeAccess::SafeFetch32 SafeAccess::_safeFetch32Func = nullptr; +extern "C" int safefetch32_cont(int* adr, int errValue); -void SafeAccess::initSafeFetch(CodeCache* libjvm) { - // Hotspot JVM's safefetch implementation appears better, e.g. it actually returns errorValue, - // when the address is invalid. let's use it whenever possible. - // When the methods are not available, fallback to SafeAccess::load32() implementation. - _safeFetch32Func = (SafeFetch32)libjvm->findSymbol("SafeFetch32_impl"); - if (_safeFetch32Func == nullptr && !WX_MEMORY) { - // jdk11 stub implementation other than Macosx/aarch64 - void** entry = (void**)libjvm->findSymbol("_ZN12StubRoutines18_safefetch32_entryE"); - if (entry != nullptr && *entry != nullptr) { - _safeFetch32Func = (SafeFetch32)*entry; +#ifdef __APPLE__ + #if defined(__x86_64__) + #define current_pc context_rip + #elif defined(__aarch64__) + #define DU3_PREFIX(s, m) __ ## s.__ ## m + #define current_pc uc_mcontext->DU3_PREFIX(ss,pc) + #endif +#else + #if defined(__x86_64__) + #define current_pc uc_mcontext.gregs[REG_RIP] + #elif defined(__aarch64__) + #define current_pc uc_mcontext.pc + #endif +#endif + +/** + Loading a 32-bit value from specific address, the errValue will be returned + if the address is invalid. + The load is protected by the 'handle_safefetch` signal handler, who sets next `pc` + to `safefetch32_cont`, upon returning from signal handler, `safefetch32_cont` returns `errValue` + **/ + +#if defined(__x86_64__) + #ifdef __APPLE__ + asm(R"( + .globl _safefetch32_impl + .private_extern _safefetch32_impl + _safefetch32_impl: + movl (%rdi), %eax + ret + .globl _safefetch32_cont + .private_extern _safefetch32_cont + _safefetch32_cont: + movl %esi, %eax + ret + )"); + #else + asm(R"( + .text + .globl safefetch32_impl + .hidden safefetch32_impl + .type safefetch32_imp, %function + safefetch32_impl: + movl (%rdi), %eax + ret + .globl safefetch32_cont + .hidden safefetch32_cont + .type safefetch32_cont, %function + safefetch32_cont: + movl %esi, %eax + ret + )"); + #endif // __APPLE__ +#elif defined(__aarch64__) + #ifdef __APPLE__ + asm(R"( + .globl _safefetch32_impl + .private_extern _safefetch32_impl + _safefetch32_impl: + ldr w0, [x0] + ret + .globl _safefetch32_cont + .private_extern _safefetch32_cont + _safefetch32_cont: + mov x0, x1 + ret + )"); + #else + asm(R"( + .text + .globl safefetch32_impl + .hidden safefetch32_impl + .type safefetch32_impl, %function + safefetch32_impl: + ldr w0, [x0] + ret + .globl safefetch32_cont + .hidden safefetch32_cont + .type safefetch32_cont, %function + safefetch32_cont: + mov x0, x1 + ret + )"); + #endif +#endif + +bool SafeAccess::handle_safefetch(int sig, void* context) { + ucontext_t* uc = (ucontext_t*)context; + uintptr_t pc = uc->current_pc; + if ((sig == SIGSEGV || sig == SIGBUS) && uc != nullptr) { + if (pc == (uintptr_t)safefetch32_impl) { + uc->current_pc = (uintptr_t)safefetch32_cont; + return true; } } - // Fallback - if (_safeFetch32Func == nullptr) { - _safeFetch32Func = (SafeFetch32)load32; - } + return false; } diff --git a/ddprof-lib/src/main/cpp/safeAccess.h b/ddprof-lib/src/main/cpp/safeAccess.h index 993c359c..4030ffa0 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.h +++ b/ddprof-lib/src/main/cpp/safeAccess.h @@ -22,6 +22,8 @@ #include #include +extern "C" int safefetch32_impl(int* adr, int errValue); + #ifdef __clang__ #define NOINLINE __attribute__((noinline)) #else @@ -30,18 +32,14 @@ #define NOADDRSANITIZE __attribute__((no_sanitize("address"))) class SafeAccess { -private: - typedef int (*SafeFetch32)(int* ptr, int errorValue); - static SafeFetch32 _safeFetch32Func; - public: - static void initSafeFetch(CodeCache* libjvm); static inline int safeFetch32(int* ptr, int errorValue) { - assert(_safeFetch32Func != nullptr); - return _safeFetch32Func(ptr, errorValue); + return safefetch32_impl(ptr, errorValue); } + static bool handle_safefetch(int sig, void* context); + NOINLINE NOADDRSANITIZE __attribute__((aligned(16))) static void *load(void **ptr) { return *ptr; } diff --git a/ddprof-lib/src/main/cpp/vmStructs_dd.cpp b/ddprof-lib/src/main/cpp/vmStructs_dd.cpp index 4dc043a5..d8efce3b 100644 --- a/ddprof-lib/src/main/cpp/vmStructs_dd.cpp +++ b/ddprof-lib/src/main/cpp/vmStructs_dd.cpp @@ -44,7 +44,6 @@ namespace ddprof { initOffsets(); initJvmFunctions(); initUnsafeFunctions(); - initSafeFetch(libjvm); } void VMStructs_::initOffsets() { @@ -99,10 +98,6 @@ namespace ddprof { } } - void VMStructs_::initSafeFetch(CodeCache* libjvm) { - SafeAccess::initSafeFetch(libjvm); - } - const void *VMStructs_::findHeapUsageFunc() { if (VM::hotspot_version() < 17) { // For JDK 11 it is really unreliable to find the memory_usage function - diff --git a/ddprof-lib/src/main/cpp/vmStructs_dd.h b/ddprof-lib/src/main/cpp/vmStructs_dd.h index 431aa432..c4547140 100644 --- a/ddprof-lib/src/main/cpp/vmStructs_dd.h +++ b/ddprof-lib/src/main/cpp/vmStructs_dd.h @@ -52,8 +52,6 @@ namespace ddprof { static void initOffsets(); static void initJvmFunctions(); static void initUnsafeFunctions(); - // We need safe access for all jdk versions - static void initSafeFetch(CodeCache* libjvm); static void checkNativeBinding(jvmtiEnv *jvmti, JNIEnv *jni, jmethodID method, void *address); diff --git a/ddprof-lib/src/test/cpp/safefetch_ut.cpp b/ddprof-lib/src/test/cpp/safefetch_ut.cpp new file mode 100644 index 00000000..3519fa44 --- /dev/null +++ b/ddprof-lib/src/test/cpp/safefetch_ut.cpp @@ -0,0 +1,51 @@ +#include +#include +#include + +#include "safeAccess.h" +#include "os.h" + + +static void (*orig_segvHandler)(int signo, siginfo_t *siginfo, void *ucontext); +static void (*orig_busHandler)(int signo, siginfo_t *siginfo, void *ucontext); + + +void signal_handle_wrapper(int signo, siginfo_t* siginfo, void* context) { + if (!SafeAccess::handle_safefetch(signo, context)) { + if (signo == SIGBUS && orig_busHandler != nullptr) { + orig_busHandler(signo, siginfo, context); + } else if (signo == SIGSEGV && orig_segvHandler != nullptr) { + orig_segvHandler(signo, siginfo, context); + } + } +} + +class SafeFetchTest : public ::testing::Test { +protected: + void SetUp() override { + orig_segvHandler = OS::replaceSigsegvHandler(signal_handle_wrapper); + orig_busHandler = OS::replaceSigbusHandler(signal_handle_wrapper); + } + + void TearDown() override { + OS::replaceSigsegvHandler(orig_segvHandler); + OS::replaceSigbusHandler(orig_busHandler); + } +}; + + +TEST_F(SafeFetchTest, validAccess) { + int i = 42; + int* p = &i; + int res = SafeAccess::safeFetch32(p, -1); + EXPECT_EQ(res, i); +} + + +TEST_F(SafeFetchTest, invalidAccess) { + int* p = nullptr; + int res = SafeAccess::safeFetch32(p, -1); + EXPECT_EQ(res, -1); + res = SafeAccess::safeFetch32(p, -2); + EXPECT_EQ(res, -2); +}