From 6e2a715d1cc97ed92c74cf3a45530f98662a3033 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 18 Jul 2025 15:13:13 -0400 Subject: [PATCH 1/5] v0 --- ddprof-lib/src/main/cpp/safeAccess.cpp | 48 ++++++++++++++++++++++++ ddprof-lib/src/main/cpp/safeAccess.h | 25 ++++++++---- ddprof-lib/src/test/cpp/safefetch_ut.cpp | 19 +++++++++- 3 files changed, 83 insertions(+), 9 deletions(-) diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 2fea8de3..0cbcc5aa 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" int safefetch64_cont(int64_t* adr, int64_t errValue); #ifdef __APPLE__ #if defined(__x86_64__) @@ -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 + safefetch32_impl: + movq (%rdi), %rax + ret + .globl safefetch64_cont + .hidden safefetch64_cont + .type safefetch64_cont, %function + safefetch32_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)safefetch32_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..4b84c983 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" int safefetch64_impl(int64_t* adr, int64_t errValue); #ifdef __clang__ #define NOINLINE __attribute__((noinline)) @@ -38,19 +39,29 @@ class SafeAccess { return safefetch32_impl(ptr, errorValue); } + static inline int safeFetch64(int64_t* ptr, int64_t errorValue) { + return safefetch64_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 void *load(void **ptr) { + return loadPtr(ptr, nullptr); } - NOINLINE NOADDRSANITIZE __attribute__((aligned(16))) static u32 load32(u32 *ptr, - u32 default_value) { - return *ptr; + static inline u32 load32(u32 *ptr, u32 default_value) { + int res = safefetch32_impl((int*)ptr, (int)default_value); + return static_cast(res); } - NOINLINE NOADDRSANITIZE __attribute__((aligned(16))) static void * - loadPtr(void **ptr, void *default_value) { + 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)default_value); + return (void*)res; + #elif defined(__i386__) || defined(__arm__) || defined(__thumb__) + int res = safefetch32_impl((int*)ptr, (int)default_value); + return (void*)res; + #endif return *ptr; } diff --git a/ddprof-lib/src/test/cpp/safefetch_ut.cpp b/ddprof-lib/src/test/cpp/safefetch_ut.cpp index 3519fa44..d832ebdb 100644 --- a/ddprof-lib/src/test/cpp/safefetch_ut.cpp +++ b/ddprof-lib/src/test/cpp/safefetch_ut.cpp @@ -34,7 +34,7 @@ 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); @@ -42,10 +42,25 @@ TEST_F(SafeFetchTest, validAccess) { } -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); +} + +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); +} From ae754ea899e9ace4f700e164415a60e835cce0ba Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 21 Jul 2025 09:24:20 -0400 Subject: [PATCH 2/5] v1 --- ddprof-lib/src/main/cpp/safeAccess.cpp | 8 +++--- ddprof-lib/src/main/cpp/safeAccess.h | 10 ++++---- ddprof-lib/src/test/cpp/safefetch_ut.cpp | 32 ++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 0cbcc5aa..6c7ff85c 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -20,7 +20,7 @@ #include extern "C" int safefetch32_cont(int* adr, int errValue); -extern "C" int safefetch64_cont(int64_t* adr, int64_t errValue); +extern "C" int64_t safefetch64_cont(int64_t* adr, int64_t errValue); #ifdef __APPLE__ #if defined(__x86_64__) @@ -74,7 +74,7 @@ extern "C" int safefetch64_cont(int64_t* adr, int64_t errValue); .globl safefetch32_impl .hidden safefetch32_impl .type safefetch32_imp, %function - safefetch32_impl: + safefetch64_impl: movl (%rdi), %eax ret .globl safefetch32_cont @@ -86,7 +86,7 @@ extern "C" int safefetch64_cont(int64_t* adr, int64_t errValue); .globl safefetch64_impl .hidden safefetch64_impl .type safefetch64_imp, %function - safefetch32_impl: + safefetch64_impl: movq (%rdi), %rax ret .globl safefetch64_cont @@ -160,7 +160,7 @@ bool SafeAccess::handle_safefetch(int sig, void* context) { uc->current_pc = (uintptr_t)safefetch32_cont; return true; } else if (pc == (uintptr_t)safefetch64_impl) { - uc->current_pc = (uintptr_t)safefetch32_cont; + uc->current_pc = (uintptr_t)safefetch64_cont; return true; } } diff --git a/ddprof-lib/src/main/cpp/safeAccess.h b/ddprof-lib/src/main/cpp/safeAccess.h index 4b84c983..192aa992 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.h +++ b/ddprof-lib/src/main/cpp/safeAccess.h @@ -23,7 +23,7 @@ #include extern "C" int safefetch32_impl(int* adr, int errValue); -extern "C" int safefetch64_impl(int64_t* adr, int64_t errValue); +extern "C" int64_t safefetch64_impl(int64_t* adr, int64_t errValue); #ifdef __clang__ #define NOINLINE __attribute__((noinline)) @@ -39,7 +39,7 @@ class SafeAccess { return safefetch32_impl(ptr, errorValue); } - static inline int safeFetch64(int64_t* ptr, int64_t errorValue) { + static inline int64_t safeFetch64(int64_t* ptr, int64_t errorValue) { return safefetch64_impl(ptr, errorValue); } @@ -54,10 +54,10 @@ class SafeAccess { return static_cast(res); } - static inline void *loadPtr(void **ptr, void *default_value) { + 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)default_value); - return (void*)res; + 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; diff --git a/ddprof-lib/src/test/cpp/safefetch_ut.cpp b/ddprof-lib/src/test/cpp/safefetch_ut.cpp index d832ebdb..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" @@ -39,6 +40,12 @@ TEST_F(SafeFetchTest, validAccess32) { 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); } @@ -55,6 +62,12 @@ TEST_F(SafeFetchTest, validAccess64) { 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) { @@ -64,3 +77,22 @@ TEST_F(SafeFetchTest, invalidAccess64) { 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); +} From d519dd28569401efe97535d25450183b83ce4c7a Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 21 Jul 2025 09:32:13 -0400 Subject: [PATCH 3/5] Fix --- ddprof-lib/src/main/cpp/safeAccess.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 6c7ff85c..1ce4c437 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -92,7 +92,7 @@ extern "C" int64_t safefetch64_cont(int64_t* adr, int64_t errValue); .globl safefetch64_cont .hidden safefetch64_cont .type safefetch64_cont, %function - safefetch32_cont: + safefetch64_cont: movq %rsi, %rax ret )"); From dce4c5fa22329a0ae0b0563ab1d5cb88d3548b72 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 21 Jul 2025 11:53:12 -0400 Subject: [PATCH 4/5] v2 --- ddprof-lib/src/main/cpp/safeAccess.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 1ce4c437..0451fcd8 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -38,12 +38,12 @@ extern "C" int64_t safefetch64_cont(int64_t* adr, int64_t 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"( @@ -74,7 +74,7 @@ extern "C" int64_t safefetch64_cont(int64_t* adr, int64_t errValue); .globl safefetch32_impl .hidden safefetch32_impl .type safefetch32_imp, %function - safefetch64_impl: + safefetch32_impl: movl (%rdi), %eax ret .globl safefetch32_cont From 27b5838a2105ab1a3e5a34306181a3cdf0145d7e Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 21 Jul 2025 14:53:35 -0400 Subject: [PATCH 5/5] Remove old safe access handler --- ddprof-lib/src/main/cpp/profiler.cpp | 22 --------------- ddprof-lib/src/main/cpp/safeAccess.h | 41 ---------------------------- 2 files changed, 63 deletions(-) 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.h b/ddprof-lib/src/main/cpp/safeAccess.h index 192aa992..56e34006 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.h +++ b/ddprof-lib/src/main/cpp/safeAccess.h @@ -64,47 +64,6 @@ class SafeAccess { #endif return *ptr; } - - 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 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; - } -#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