diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 30a4576b..d268d73c 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -886,28 +886,6 @@ bool Profiler::crashHandler(int signo, siginfo_t *siginfo, void *ucontext) { return false; } - uintptr_t length = SafeAccess::skipLoad(pc); - if (length > 0) { - // Skip the fault instruction, as if it successfully loaded NULL - frame.pc() += length; - frame.retval() = 0; - if (thrd != nullptr) { - thrd->exitCrashHandler(); - } - return true; - } - - length = SafeAccess::skipLoadArg(pc); - if (length > 0) { - // Act as if the load returned default_value argument - frame.pc() += length; - frame.retval() = frame.arg1(); - if (thrd != nullptr) { - thrd->exitCrashHandler(); - } - return true; - } - if (WX_MEMORY && Trap::isFaultInstruction(pc)) { if (thrd != nullptr) { thrd->exitCrashHandler(); diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 2fea8de3..0451fcd8 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -20,6 +20,7 @@ #include extern "C" int safefetch32_cont(int* adr, int errValue); +extern "C" int64_t safefetch64_cont(int64_t* adr, int64_t errValue); #ifdef __APPLE__ #if defined(__x86_64__) @@ -37,12 +38,12 @@ extern "C" int safefetch32_cont(int* adr, int errValue); #endif /** - Loading a 32-bit value from specific address, the errValue will be returned + Loading a 32-bit/64-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` + to `safefetch32_cont/safefetch64_cont`, upon returning from signal handler, + `safefetch32_cont/safefetch64_cont` returns `errValue` **/ - #if defined(__x86_64__) #ifdef __APPLE__ asm(R"( @@ -56,6 +57,16 @@ extern "C" int safefetch32_cont(int* adr, int errValue); _safefetch32_cont: movl %esi, %eax ret + .globl _safefetch64_impl + .private_extern _safefetch64_impl + _safefetch64_impl: + movq (%rdi), %rax + ret + .globl _safefetch64_cont + .private_extern _safefetch64_cont + _safefetch64_cont: + movq %rsi, %rax + ret )"); #else asm(R"( @@ -72,6 +83,18 @@ extern "C" int safefetch32_cont(int* adr, int errValue); safefetch32_cont: movl %esi, %eax ret + .globl safefetch64_impl + .hidden safefetch64_impl + .type safefetch64_imp, %function + safefetch64_impl: + movq (%rdi), %rax + ret + .globl safefetch64_cont + .hidden safefetch64_cont + .type safefetch64_cont, %function + safefetch64_cont: + movq %rsi, %rax + ret )"); #endif // __APPLE__ #elif defined(__aarch64__) @@ -85,6 +108,16 @@ extern "C" int safefetch32_cont(int* adr, int errValue); .globl _safefetch32_cont .private_extern _safefetch32_cont _safefetch32_cont: + mov w0, w1 + ret + .globl _safefetch64_impl + .private_extern _safefetch64_impl + _safefetch64_impl: + ldr x0, [x0] + ret + .globl _safefetch64_cont + .private_extern _safefetch64_cont + _safefetch64_cont: mov x0, x1 ret )"); @@ -101,6 +134,18 @@ extern "C" int safefetch32_cont(int* adr, int errValue); .hidden safefetch32_cont .type safefetch32_cont, %function safefetch32_cont: + mov w0, w1 + ret + .globl safefetch64_impl + .hidden safefetch64_impl + .type safefetch64_impl, %function + safefetch64_impl: + ldr x0, [x0] + ret + .globl safefetch64_cont + .hidden safefetch64_cont + .type safefetch64_cont, %function + safefetch64_cont: mov x0, x1 ret )"); @@ -114,6 +159,9 @@ bool SafeAccess::handle_safefetch(int sig, void* context) { if (pc == (uintptr_t)safefetch32_impl) { uc->current_pc = (uintptr_t)safefetch32_cont; return true; + } else if (pc == (uintptr_t)safefetch64_impl) { + uc->current_pc = (uintptr_t)safefetch64_cont; + return true; } } return false; diff --git a/ddprof-lib/src/main/cpp/safeAccess.h b/ddprof-lib/src/main/cpp/safeAccess.h index 4030ffa0..56e34006 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.h +++ b/ddprof-lib/src/main/cpp/safeAccess.h @@ -23,6 +23,7 @@ #include extern "C" int safefetch32_impl(int* adr, int errValue); +extern "C" int64_t safefetch64_impl(int64_t* adr, int64_t errValue); #ifdef __clang__ #define NOINLINE __attribute__((noinline)) @@ -38,62 +39,31 @@ class SafeAccess { 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; + static inline int64_t safeFetch64(int64_t* ptr, int64_t errorValue) { + return safefetch64_impl(ptr, errorValue); } - NOINLINE NOADDRSANITIZE __attribute__((aligned(16))) static u32 load32(u32 *ptr, - u32 default_value) { - return *ptr; - } + static bool handle_safefetch(int sig, void* context); - NOINLINE NOADDRSANITIZE __attribute__((aligned(16))) static void * - loadPtr(void **ptr, void *default_value) { - return *ptr; + static inline void *load(void **ptr) { + return loadPtr(ptr, nullptr); } - NOADDRSANITIZE static uintptr_t skipLoad(uintptr_t pc) { - if (pc - (uintptr_t)load < sizeSafeLoadFunc) { -#if defined(__x86_64__) - return *(u16 *)pc == 0x8b48 ? 3 : 0; // mov rax, [reg] -#elif defined(__i386__) - return *(u8 *)pc == 0x8b ? 2 : 0; // mov eax, [reg] -#elif defined(__arm__) || defined(__thumb__) - return (*(instruction_t *)pc & 0x0e50f000) == 0x04100000 - ? 4 - : 0; // ldr r0, [reg] -#elif defined(__aarch64__) - return (*(instruction_t *)pc & 0xffc0001f) == 0xf9400000 - ? 4 - : 0; // ldr x0, [reg] -#else - return sizeof(instruction_t); -#endif - } - return 0; + static inline u32 load32(u32 *ptr, u32 default_value) { + int res = safefetch32_impl((int*)ptr, (int)default_value); + return static_cast(res); } - static uintptr_t skipLoadArg(uintptr_t pc) { -#if defined(__aarch64__) - if ((pc - (uintptr_t)load32) < sizeSafeLoadFunc || - (pc - (uintptr_t)loadPtr) < sizeSafeLoadFunc) { - return 4; - } -#endif - return 0; + static inline void *loadPtr(void** ptr, void* default_value) { + #if defined(__x86_64__) || defined(__aarch64__) + int64_t res = safefetch64_impl((int64_t*)ptr, (int64_t)reinterpret_cast(default_value)); + return (void*)static_cast(res); + #elif defined(__i386__) || defined(__arm__) || defined(__thumb__) + int res = safefetch32_impl((int*)ptr, (int)default_value); + return (void*)res; + #endif + return *ptr; } -#ifndef __SANITIZE_ADDRESS__ - constexpr static size_t sizeSafeLoadFunc = 16; -#else - // asan significantly increases the size of the load function - // checking disassembled code can help adjust the value - // gdb --batch -ex 'disas _ZN10SafeAccess4loadEPPv' ./elfparser_ut - // I see that the functions can also have a 156 bytes size for the load32 - // and 136 for the loadPtr functions - constexpr static inline size_t sizeSafeLoadFunc = 132; -#endif }; #endif // _SAFEACCESS_H diff --git a/ddprof-lib/src/test/cpp/safefetch_ut.cpp b/ddprof-lib/src/test/cpp/safefetch_ut.cpp index 3519fa44..f3bf5d05 100644 --- a/ddprof-lib/src/test/cpp/safefetch_ut.cpp +++ b/ddprof-lib/src/test/cpp/safefetch_ut.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include "safeAccess.h" @@ -34,18 +35,64 @@ class SafeFetchTest : public ::testing::Test { }; -TEST_F(SafeFetchTest, validAccess) { +TEST_F(SafeFetchTest, validAccess32) { int i = 42; int* p = &i; int res = SafeAccess::safeFetch32(p, -1); EXPECT_EQ(res, i); + i = INT_MAX; + res = SafeAccess::safeFetch32(p, -1); + EXPECT_EQ(res, i); + i = INT_MIN; + res = SafeAccess::safeFetch32(p, 0); + EXPECT_EQ(res, i); } -TEST_F(SafeFetchTest, invalidAccess) { +TEST_F(SafeFetchTest, invalidAccess32) { int* p = nullptr; int res = SafeAccess::safeFetch32(p, -1); EXPECT_EQ(res, -1); res = SafeAccess::safeFetch32(p, -2); EXPECT_EQ(res, -2); } + +TEST_F(SafeFetchTest, validAccess64) { + int64_t i = 42; + int64_t* p = &i; + int64_t res = SafeAccess::safeFetch64(p, -1); + EXPECT_EQ(res, i); + i = LONG_MIN; + res = SafeAccess::safeFetch64(p, -19); + EXPECT_EQ(res, i); + i = LONG_MAX; + res = SafeAccess::safeFetch64(p, -1); + EXPECT_EQ(res, i); +} + +TEST_F(SafeFetchTest, invalidAccess64) { + int64_t* p = nullptr; + int64_t res = SafeAccess::safeFetch64(p, -1); + EXPECT_EQ(res, -1); + res = SafeAccess::safeFetch64(p, -2); + EXPECT_EQ(res, -2); +} + +TEST_F(SafeFetchTest, validAccessPtr) { + char c = 'a'; + void* p = (void*)&c; + void** pp = &p; + void* res = SafeAccess::loadPtr(pp, nullptr); + EXPECT_EQ(res, p); +} + +TEST_F(SafeFetchTest, invalidAccessPtr) { + int a, b; + void* ap = (void*)&a; + void* bp = (void*)&b; + void** pp = nullptr; + void* res = SafeAccess::loadPtr(pp, ap); + EXPECT_EQ(res, ap); + res = SafeAccess::loadPtr(pp, bp); + EXPECT_EQ(res, bp); +}