From f523bb2f604c79a38ad878fffee2e458a112b37a Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Mon, 14 Jul 2025 08:15:10 -0400 Subject: [PATCH 01/15] v0 --- ddprof-lib/build.gradle | 5 ++- ddprof-lib/src/main/asm/safefetch.S | 53 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 ddprof-lib/src/main/asm/safefetch.S diff --git a/ddprof-lib/build.gradle b/ddprof-lib/build.gradle index 51438cbc..1fcf0699 100644 --- a/ddprof-lib/build.gradle +++ b/ddprof-lib/build.gradle @@ -524,6 +524,7 @@ tasks.whenTaskAdded { task -> if (compileTask != null) { dependsOn compileTask } + dependsOn asmTask linkerArgs.addAll(config.linkerArgs) toolChain = task.toolChain targetPlatform = task.targetPlatform @@ -604,7 +605,9 @@ tasks.register('javadocJar', Jar) { from javadoc.destinationDir } - +tasks.register('asmTask', Exec) { + commandLine 'clang', '-D__aarch64__', 'src/main/asm/safefetch.S', '-o', 'build/obj/safefetch.o' +} tasks.register('scanBuild', Exec) { workingDir "${projectDir}/src/test/make" diff --git a/ddprof-lib/src/main/asm/safefetch.S b/ddprof-lib/src/main/asm/safefetch.S new file mode 100644 index 00000000..3e923461 --- /dev/null +++ b/ddprof-lib/src/main/asm/safefetch.S @@ -0,0 +1,53 @@ + +#define DECLARE_FUNC(func) \ + .globl func %% \ + .hidden func %% \ + .type func, %function %% \ + func + +#if defined(__x86_64__) + .text + + # Support for int SafeFetch32(int* address, int defaultval); + # + # %rdi : address + # %esi : defaultval +DECLARE_FUNC(SafeFetch32_impl): +DECLARE_FUNC(_SafeFetch32_fault): + movl (%rdi), %eax # load target value, may fault + ret +DECLARE_FUNC(_SafeFetch32_continuation): + movl %esi, %eax # return default + ret + +#elif defined(__aarch64__) +#ifdef __APPLE__ + # Support for int SafeFetch32(int* address, int defaultval); + # + # x0 : address + # w1 : defaultval + + # needed to align function start to 4 byte + .align 6 +DECLARE_FUNC(SafeFetch32_impl): +DECLARE_FUNC(_SafeFetch32_fault): + ldr w0, [x0] + ret +DECLARE_FUNC(_SafeFetch32_continuation): + mov x0, x1 + ret +#else + # Support for int SafeFetch32(int* address, int defaultval); + # + # x0 : address + # x1 : defaultval +DECLARE_FUNC(SafeFetch32_impl): +DECLARE_FUNC(_SafeFetch32_fault): + ldr w0, [x0] + ret +DECLARE_FUNC(_SafeFetch32_continuation): + mov x0, x1 + ret +#endif // __APPLE__ + +#endif From 35576dc5711efb6dd614f3c7a71d24b966a32d94 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 15 Jul 2025 10:59:04 -0400 Subject: [PATCH 02/15] v1 --- ddprof-lib/build.gradle | 18 +++++++++++++++++- .../src/main/{asm => assembler}/safefetch.S | 19 +++++++++++++++---- 2 files changed, 32 insertions(+), 5 deletions(-) rename ddprof-lib/src/main/{asm => assembler}/safefetch.S (76%) diff --git a/ddprof-lib/build.gradle b/ddprof-lib/build.gradle index 1fcf0699..d1e0df92 100644 --- a/ddprof-lib/build.gradle +++ b/ddprof-lib/build.gradle @@ -3,6 +3,7 @@ plugins { id 'java' id 'maven-publish' id 'signing' + id 'assembler' id 'com.github.ben-manes.versions' version '0.27.0' id 'de.undercouch.download' version '4.1.1' @@ -606,7 +607,22 @@ tasks.register('javadocJar', Jar) { } tasks.register('asmTask', Exec) { - commandLine 'clang', '-D__aarch64__', 'src/main/asm/safefetch.S', '-o', 'build/obj/safefetch.o' + String reportedArch = System.getProperty('os.arch') + String arch + if (reportedArch == 'x86_64') { + arch = '-D__x86_64__' + } else { + arch = '-D__aarch64__' + } + + String os + if (osIdentifier() == 'macos') { + os = '-D__APPLE__' + } else { + os = '-D__LINUX__' + } + + commandLine 'gcc', arch, os, 'src/main/assembler/safefetch.S', '-c', 'build/obj/safefetch.o' } tasks.register('scanBuild', Exec) { diff --git a/ddprof-lib/src/main/asm/safefetch.S b/ddprof-lib/src/main/assembler/safefetch.S similarity index 76% rename from ddprof-lib/src/main/asm/safefetch.S rename to ddprof-lib/src/main/assembler/safefetch.S index 3e923461..5e66d385 100644 --- a/ddprof-lib/src/main/asm/safefetch.S +++ b/ddprof-lib/src/main/assembler/safefetch.S @@ -1,9 +1,20 @@ +#ifdef __APPLE__ + # macOS prefixes symbols with _ + #define SYMBOL(s) _ ## s + + #define DECLARE_FUNC(func) \ + .globl SYMBOL(func) %% \ + .private_extern SYMBOL(func) %% \ + SYMBOL(func) +#else + #define SYMBOL(s) s -#define DECLARE_FUNC(func) \ - .globl func %% \ - .hidden func %% \ - .type func, %function %% \ + #define DECLARE_FUNC(func) \ + .globl func ; \ + .hidden func ; \ + .type func, %function ; \ func +#endif // __APPLE__ #if defined(__x86_64__) .text From 21f328725ba5322351daddb8fb75aa9996f00b9a Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Tue, 15 Jul 2025 15:52:33 -0400 Subject: [PATCH 03/15] v3 --- ddprof-lib/build.gradle | 4 +-- ddprof-lib/src/main/cpp/profiler.cpp | 7 +++++ ddprof-lib/src/main/cpp/safeAccess.cpp | 39 ++++++++++++++---------- ddprof-lib/src/main/cpp/safeAccess.h | 12 +++----- ddprof-lib/src/main/cpp/vmStructs_dd.cpp | 5 --- ddprof-lib/src/main/cpp/vmStructs_dd.h | 2 -- 6 files changed, 36 insertions(+), 33 deletions(-) diff --git a/ddprof-lib/build.gradle b/ddprof-lib/build.gradle index d1e0df92..494a0c7d 100644 --- a/ddprof-lib/build.gradle +++ b/ddprof-lib/build.gradle @@ -3,7 +3,6 @@ plugins { id 'java' id 'maven-publish' id 'signing' - id 'assembler' id 'com.github.ben-manes.versions' version '0.27.0' id 'de.undercouch.download' version '4.1.1' @@ -621,8 +620,7 @@ tasks.register('asmTask', Exec) { } else { os = '-D__LINUX__' } - - commandLine 'gcc', arch, os, 'src/main/assembler/safefetch.S', '-c', 'build/obj/safefetch.o' + commandLine 'gcc', arch, os, 'src/main/assembler/safefetch.S', '-c', 'safefetch.o' } tasks.register('scanBuild', Exec) { diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index efdae02a..0787ad0f 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -878,6 +878,13 @@ bool Profiler::crashHandler(int signo, siginfo_t *siginfo, void *ucontext) { return false; } + if (SafeAccess::handle_safefetch(signo, pc, ucontext)) { + if (thrd != nullptr) { + thrd->exitCrashHandler(); + } + return true; + } + uintptr_t length = SafeAccess::skipLoad(pc); if (length > 0) { // Skip the fault instruction, as if it successfully loaded NULL diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 82aceeeb..8307b991 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -16,23 +16,30 @@ #include "safeAccess.h" +#include -SafeAccess::SafeFetch32 SafeAccess::_safeFetch32Func = nullptr; - -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 context_pc context_rip +#elif defined(__aarch64__) + #define DU3_PREFIX(s, m) __ ## s.__ ## m + #define context_pc uc_mcontext->DU3_PREFIX(ss,pc) +#endif + +#endif + +extern "C" char _SafeFetch32_continuation[] __attribute__ ((visibility ("hidden"))); +extern "C" char _SafeFetch32_fault[] __attribute__ ((visibility ("hidden"))); + +bool SafeAccess::handle_safefetch(int sig, uintptr_t pc, void* context) { + ucontext_t* uc = (ucontext_t*)context; + if ((sig == SIGSEGV || sig == SIGBUS) && uc != nullptr) { + if (pc == (uintptr_t)_SafeFetch32_fault) { + uc->context_pc == (uintptr_t)_SafeFetch32_continuation; + 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..921cfa49 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, uintptr_t pc, 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); From fc66365e6752d4beae848f1a20517224e0d755e1 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Wed, 16 Jul 2025 11:54:18 +0200 Subject: [PATCH 04/15] Fix the assembler gradle task --- ddprof-lib/build.gradle | 43 +++++++++++++++++++++++++++-------- ddprof-lib/gtest/build.gradle | 7 +++++- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/ddprof-lib/build.gradle b/ddprof-lib/build.gradle index 494a0c7d..773ecb01 100644 --- a/ddprof-lib/build.gradle +++ b/ddprof-lib/build.gradle @@ -516,14 +516,18 @@ tasks.whenTaskAdded { task -> onlyIf { config.active } + group = 'build' description = "Link the ${config.name} build of the library" source = fileTree("$buildDir/obj/main/${config.name}") + source.from(file("${projectDir}/build/obj/main/asm/assembly.o")) + linkedFile = file("$buildDir/lib/main/${config.name}/${osIdentifier()}/${archIdentifier()}/libjavaProfiler.${os().isLinux() ? 'so' : 'dylib'}") def compileTask = tasks.findByName("compileLib${config.name.capitalize()}".toString()) if (compileTask != null) { dependsOn compileTask } + def asmTask = tasks.findByName("asm") dependsOn asmTask linkerArgs.addAll(config.linkerArgs) toolChain = task.toolChain @@ -605,31 +609,52 @@ tasks.register('javadocJar', Jar) { from javadoc.destinationDir } -tasks.register('asmTask', Exec) { - String reportedArch = System.getProperty('os.arch') - String arch - if (reportedArch == 'x86_64') { +tasks.register('asm', Exec) { + group = 'build' + + def arch + if (archIdentifier() == 'x64') { arch = '-D__x86_64__' } else { arch = '-D__aarch64__' } - String os + def os if (osIdentifier() == 'macos') { os = '-D__APPLE__' } else { os = '-D__LINUX__' } - commandLine 'gcc', arch, os, 'src/main/assembler/safefetch.S', '-c', 'safefetch.o' + def asmDir = file("${projectDir}/src/main/assembler") + if (!asmDir.exists()) { + // no assembler files found, skipping + return + } + + doLast { + // Ensure the output directory exists + file("${projectDir}/build/obj/main/asm").mkdirs() + } + + // collect all assembler files and compile them + workingDir asmDir + inputs.files fileTree("${projectDir}/src/main/assembler") { + include '**/*.S' + } + def asmFiles = inputs.files.collect { it.absolutePath } + outputs.dir "${projectDir}/build/obj/main/asm" + // use gcc to compile assembler files + commandLine 'gcc', arch, os, '-c', '-x', 'assembler-with-cpp', + *asmFiles, '-o', "${projectDir}/build/obj/main/asm/assembly.o" } tasks.register('scanBuild', Exec) { workingDir "${projectDir}/src/test/make" commandLine 'scan-build' args "-o", "${projectDir}/build/reports/scan-build", - "--force-analyze-debug-code", - "--use-analyzer", "/usr/bin/clang++", - "make", "-j4", "clean", "all" + "--force-analyze-debug-code", + "--use-analyzer", "/usr/bin/clang++", + "make", "-j4", "clean", "all" } tasks.withType(Test) { diff --git a/ddprof-lib/gtest/build.gradle b/ddprof-lib/gtest/build.gradle index d2604ff8..1792aefd 100644 --- a/ddprof-lib/gtest/build.gradle +++ b/ddprof-lib/gtest/build.gradle @@ -154,7 +154,10 @@ tasks.whenTaskAdded { task -> group = 'verification' description = "Run all Google Tests for the ${config.name} build of the library" } - project(':ddprof-lib').file("src/test/cpp/").eachFile { + def libProject = project(':ddprof-lib') + def asmTask = libProject.tasks.findByName("asm") + + libProject.file("src/test/cpp/").eachFile { def testFile = it def testName = it.name.substring(0, it.name.lastIndexOf('.')) @@ -167,6 +170,7 @@ tasks.whenTaskAdded { task -> group = 'build' description = "Link the Google Test for the ${config.name} build of the library" source = fileTree("$buildDir/obj/gtest/${config.name}/${testName}") + source.from(file("${rootDir}/ddprof-lib/build/obj/main/asm/assembly.o")) linkedFile = binary if (config.name != 'release') { // linking the gtests using the minimizing release flags is making gtest unhappy @@ -183,6 +187,7 @@ tasks.whenTaskAdded { task -> libs = task.libs inputs.files source outputs.file linkedFile + dependsOn asmTask } def gtestExecuteTask = tasks.register("gtest${config.name.capitalize()}_${testName}", Exec) { onlyIf { From 7b48662cada5c42986044e008ff66aa94b44c539 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 16 Jul 2025 11:05:11 -0400 Subject: [PATCH 05/15] v5 --- ddprof-lib/src/main/cpp/profiler.cpp | 15 +++---- ddprof-lib/src/main/cpp/safeAccess.cpp | 21 +++++----- ddprof-lib/src/main/cpp/safeAccess.h | 2 +- ddprof-lib/src/test/cpp/safefetch_ut.cpp | 51 ++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 18 deletions(-) create mode 100644 ddprof-lib/src/test/cpp/safefetch_ut.cpp diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 0787ad0f..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(); @@ -878,13 +886,6 @@ bool Profiler::crashHandler(int signo, siginfo_t *siginfo, void *ucontext) { return false; } - if (SafeAccess::handle_safefetch(signo, pc, ucontext)) { - if (thrd != nullptr) { - thrd->exitCrashHandler(); - } - return true; - } - uintptr_t length = SafeAccess::skipLoad(pc); if (length > 0) { // Skip the fault instruction, as if it successfully loaded NULL diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 8307b991..c803cd3f 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -16,25 +16,26 @@ #include "safeAccess.h" +#include #include - #ifdef __APPLE__ - -#if defined(__x86_64) - #define context_pc context_rip -#elif defined(__aarch64__) - #define DU3_PREFIX(s, m) __ ## s.__ ## m - #define context_pc uc_mcontext->DU3_PREFIX(ss,pc) -#endif - + #if defined(__x86_64) + #define context_pc context_rip + #elif defined(__aarch64__) + #define DU3_PREFIX(s, m) __ ## s.__ ## m + #define context_pc uc_mcontext->DU3_PREFIX(ss,pc) + #endif +#else + #define context_pc uc_mcontext.gregs[REG_RIP] #endif extern "C" char _SafeFetch32_continuation[] __attribute__ ((visibility ("hidden"))); extern "C" char _SafeFetch32_fault[] __attribute__ ((visibility ("hidden"))); -bool SafeAccess::handle_safefetch(int sig, uintptr_t pc, void* context) { +bool SafeAccess::handle_safefetch(int sig, void* context) { ucontext_t* uc = (ucontext_t*)context; + uintptr_t pc = uc->context_pc; if ((sig == SIGSEGV || sig == SIGBUS) && uc != nullptr) { if (pc == (uintptr_t)_SafeFetch32_fault) { uc->context_pc == (uintptr_t)_SafeFetch32_continuation; diff --git a/ddprof-lib/src/main/cpp/safeAccess.h b/ddprof-lib/src/main/cpp/safeAccess.h index 921cfa49..1f8b27c0 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.h +++ b/ddprof-lib/src/main/cpp/safeAccess.h @@ -38,7 +38,7 @@ class SafeAccess { return SafeFetch32_impl(ptr, errorValue); } - static bool handle_safefetch(int sig, uintptr_t pc, void* context); + 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/test/cpp/safefetch_ut.cpp b/ddprof-lib/src/test/cpp/safefetch_ut.cpp new file mode 100644 index 00000000..4e36b862 --- /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(signo, siginfo, context); + } else if (signo == SIGSEGV) { + 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); +} From ab1b644898a45f9fa3d20a42350a94be69a4e4d0 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 16 Jul 2025 11:14:24 -0400 Subject: [PATCH 06/15] Fix safefetch handler --- 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 c803cd3f..a764ab36 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -38,7 +38,7 @@ bool SafeAccess::handle_safefetch(int sig, void* context) { uintptr_t pc = uc->context_pc; if ((sig == SIGSEGV || sig == SIGBUS) && uc != nullptr) { if (pc == (uintptr_t)_SafeFetch32_fault) { - uc->context_pc == (uintptr_t)_SafeFetch32_continuation; + uc->context_pc = (uintptr_t)_SafeFetch32_continuation; return true; } } From bef720017229118acbf2c6391150b48377a6b6f0 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 16 Jul 2025 11:42:48 -0400 Subject: [PATCH 07/15] context_pc for aarch64 Linux --- ddprof-lib/src/main/cpp/safeAccess.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index a764ab36..c30f0834 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -20,14 +20,18 @@ #include #ifdef __APPLE__ - #if defined(__x86_64) + #if defined(__x86_64__) #define context_pc context_rip #elif defined(__aarch64__) #define DU3_PREFIX(s, m) __ ## s.__ ## m #define context_pc uc_mcontext->DU3_PREFIX(ss,pc) #endif #else - #define context_pc uc_mcontext.gregs[REG_RIP] + #if defined(__x86_64__) + #define context_pc uc_mcontext.gregs[REG_RIP] + #elif defined(__aarch64__) + #define context_pc uc_mcontext.gregs[_REG_ELR] + #endif #endif extern "C" char _SafeFetch32_continuation[] __attribute__ ((visibility ("hidden"))); From aca18b73bfd47f9fd2cf0dcac3fce06e1d7cdace Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 16 Jul 2025 11:50:23 -0400 Subject: [PATCH 08/15] 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 c30f0834..b3567a92 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -30,7 +30,7 @@ #if defined(__x86_64__) #define context_pc uc_mcontext.gregs[REG_RIP] #elif defined(__aarch64__) - #define context_pc uc_mcontext.gregs[_REG_ELR] + #define context_pc uc_mcontext.regs[_REG_ELR] #endif #endif From 8ac912ee0aa5a98b70a273ff2235b9afe56d4a0b Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 16 Jul 2025 13:55:00 -0400 Subject: [PATCH 09/15] 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 b3567a92..90379913 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -30,7 +30,7 @@ #if defined(__x86_64__) #define context_pc uc_mcontext.gregs[REG_RIP] #elif defined(__aarch64__) - #define context_pc uc_mcontext.regs[_REG_ELR] + #define context_pc uc_mcontext.regs[REG_ELR] #endif #endif From 8d20ad6b7d449a72b542c59c2e479ec0bdb0c8a4 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Wed, 16 Jul 2025 15:01:48 -0400 Subject: [PATCH 10/15] 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 90379913..ff5aa5c1 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -30,7 +30,7 @@ #if defined(__x86_64__) #define context_pc uc_mcontext.gregs[REG_RIP] #elif defined(__aarch64__) - #define context_pc uc_mcontext.regs[REG_ELR] + #define context_pc uc_mcontext.pc #endif #endif From 33261bcefbc38e26d70cd67f33995e1dee5c5fb6 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Thu, 17 Jul 2025 12:24:34 -0400 Subject: [PATCH 11/15] Use inline assembly --- ddprof-lib/build.gradle | 43 --------------- ddprof-lib/gtest/build.gradle | 7 +-- ddprof-lib/src/main/assembler/safefetch.S | 64 ---------------------- ddprof-lib/src/main/cpp/safeAccess.cpp | 67 +++++++++++++++++++++-- ddprof-lib/src/main/cpp/safeAccess.h | 5 +- 5 files changed, 67 insertions(+), 119 deletions(-) delete mode 100644 ddprof-lib/src/main/assembler/safefetch.S diff --git a/ddprof-lib/build.gradle b/ddprof-lib/build.gradle index 773ecb01..b25ffc57 100644 --- a/ddprof-lib/build.gradle +++ b/ddprof-lib/build.gradle @@ -516,19 +516,15 @@ tasks.whenTaskAdded { task -> onlyIf { config.active } - group = 'build' description = "Link the ${config.name} build of the library" source = fileTree("$buildDir/obj/main/${config.name}") - source.from(file("${projectDir}/build/obj/main/asm/assembly.o")) linkedFile = file("$buildDir/lib/main/${config.name}/${osIdentifier()}/${archIdentifier()}/libjavaProfiler.${os().isLinux() ? 'so' : 'dylib'}") def compileTask = tasks.findByName("compileLib${config.name.capitalize()}".toString()) if (compileTask != null) { dependsOn compileTask } - def asmTask = tasks.findByName("asm") - dependsOn asmTask linkerArgs.addAll(config.linkerArgs) toolChain = task.toolChain targetPlatform = task.targetPlatform @@ -609,45 +605,6 @@ tasks.register('javadocJar', Jar) { from javadoc.destinationDir } -tasks.register('asm', Exec) { - group = 'build' - - def arch - if (archIdentifier() == 'x64') { - arch = '-D__x86_64__' - } else { - arch = '-D__aarch64__' - } - - def os - if (osIdentifier() == 'macos') { - os = '-D__APPLE__' - } else { - os = '-D__LINUX__' - } - def asmDir = file("${projectDir}/src/main/assembler") - if (!asmDir.exists()) { - // no assembler files found, skipping - return - } - - doLast { - // Ensure the output directory exists - file("${projectDir}/build/obj/main/asm").mkdirs() - } - - // collect all assembler files and compile them - workingDir asmDir - inputs.files fileTree("${projectDir}/src/main/assembler") { - include '**/*.S' - } - def asmFiles = inputs.files.collect { it.absolutePath } - outputs.dir "${projectDir}/build/obj/main/asm" - // use gcc to compile assembler files - commandLine 'gcc', arch, os, '-c', '-x', 'assembler-with-cpp', - *asmFiles, '-o', "${projectDir}/build/obj/main/asm/assembly.o" -} - tasks.register('scanBuild', Exec) { workingDir "${projectDir}/src/test/make" commandLine 'scan-build' diff --git a/ddprof-lib/gtest/build.gradle b/ddprof-lib/gtest/build.gradle index 1792aefd..d2604ff8 100644 --- a/ddprof-lib/gtest/build.gradle +++ b/ddprof-lib/gtest/build.gradle @@ -154,10 +154,7 @@ tasks.whenTaskAdded { task -> group = 'verification' description = "Run all Google Tests for the ${config.name} build of the library" } - def libProject = project(':ddprof-lib') - def asmTask = libProject.tasks.findByName("asm") - - libProject.file("src/test/cpp/").eachFile { + project(':ddprof-lib').file("src/test/cpp/").eachFile { def testFile = it def testName = it.name.substring(0, it.name.lastIndexOf('.')) @@ -170,7 +167,6 @@ tasks.whenTaskAdded { task -> group = 'build' description = "Link the Google Test for the ${config.name} build of the library" source = fileTree("$buildDir/obj/gtest/${config.name}/${testName}") - source.from(file("${rootDir}/ddprof-lib/build/obj/main/asm/assembly.o")) linkedFile = binary if (config.name != 'release') { // linking the gtests using the minimizing release flags is making gtest unhappy @@ -187,7 +183,6 @@ tasks.whenTaskAdded { task -> libs = task.libs inputs.files source outputs.file linkedFile - dependsOn asmTask } def gtestExecuteTask = tasks.register("gtest${config.name.capitalize()}_${testName}", Exec) { onlyIf { diff --git a/ddprof-lib/src/main/assembler/safefetch.S b/ddprof-lib/src/main/assembler/safefetch.S deleted file mode 100644 index 5e66d385..00000000 --- a/ddprof-lib/src/main/assembler/safefetch.S +++ /dev/null @@ -1,64 +0,0 @@ -#ifdef __APPLE__ - # macOS prefixes symbols with _ - #define SYMBOL(s) _ ## s - - #define DECLARE_FUNC(func) \ - .globl SYMBOL(func) %% \ - .private_extern SYMBOL(func) %% \ - SYMBOL(func) -#else - #define SYMBOL(s) s - - #define DECLARE_FUNC(func) \ - .globl func ; \ - .hidden func ; \ - .type func, %function ; \ - func -#endif // __APPLE__ - -#if defined(__x86_64__) - .text - - # Support for int SafeFetch32(int* address, int defaultval); - # - # %rdi : address - # %esi : defaultval -DECLARE_FUNC(SafeFetch32_impl): -DECLARE_FUNC(_SafeFetch32_fault): - movl (%rdi), %eax # load target value, may fault - ret -DECLARE_FUNC(_SafeFetch32_continuation): - movl %esi, %eax # return default - ret - -#elif defined(__aarch64__) -#ifdef __APPLE__ - # Support for int SafeFetch32(int* address, int defaultval); - # - # x0 : address - # w1 : defaultval - - # needed to align function start to 4 byte - .align 6 -DECLARE_FUNC(SafeFetch32_impl): -DECLARE_FUNC(_SafeFetch32_fault): - ldr w0, [x0] - ret -DECLARE_FUNC(_SafeFetch32_continuation): - mov x0, x1 - ret -#else - # Support for int SafeFetch32(int* address, int defaultval); - # - # x0 : address - # x1 : defaultval -DECLARE_FUNC(SafeFetch32_impl): -DECLARE_FUNC(_SafeFetch32_fault): - ldr w0, [x0] - ret -DECLARE_FUNC(_SafeFetch32_continuation): - mov x0, x1 - ret -#endif // __APPLE__ - -#endif diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index ff5aa5c1..9aa1eda9 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -34,15 +34,74 @@ #endif #endif -extern "C" char _SafeFetch32_continuation[] __attribute__ ((visibility ("hidden"))); -extern "C" char _SafeFetch32_fault[] __attribute__ ((visibility ("hidden"))); +#if defined(__x86_64__) + #ifdef __APPLE__ + asm(R"( + .globl _safefetch32_impl + .private_extern _safefetch32_impl + _safefetch32_impl: + movl (%rdi), %eax + ret + .globl _safefetch32_continuation + .private_extern _safefetch32_continuation + _safefetch32_continuation: + movl %esi, %eax + ret + )"); + #else + asm(R"( + .globl safefetch32_impl + .hidden safefetch32_impl + .type safefetch32_imp, %function + safefetch32_impl: + movl (%rdi), %eax + ret + .globl safefetch32_continuation + .hidden safefetch32_continuation + .type safefetch32_continuation, %function + safefetch32_continuation: + 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_continuation + .private_extern _safefetch32_continuation + _safefetch32_continuation: + mov x0, x1 + ret + )"); + #else + asm(R"( + .globl safefetch32_impl + .hidden safefetch32_impl + .type safefetch32_impl, %function + safefetch32_impl: + ldr w0, [x0] + ret + .globl safefetch32_continuation + .hidden safefetch32_continuation + .type safefetch32_continuation, %function + safefetch32_continuation: + mov x0, x1 + ret + )"); + #endif +#endif bool SafeAccess::handle_safefetch(int sig, void* context) { ucontext_t* uc = (ucontext_t*)context; uintptr_t pc = uc->context_pc; if ((sig == SIGSEGV || sig == SIGBUS) && uc != nullptr) { - if (pc == (uintptr_t)_SafeFetch32_fault) { - uc->context_pc = (uintptr_t)_SafeFetch32_continuation; + if (pc == (uintptr_t)safefetch32_impl) { + uc->context_pc = (uintptr_t)safefetch32_continuation; return true; } } diff --git a/ddprof-lib/src/main/cpp/safeAccess.h b/ddprof-lib/src/main/cpp/safeAccess.h index 1f8b27c0..19e83beb 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.h +++ b/ddprof-lib/src/main/cpp/safeAccess.h @@ -22,7 +22,8 @@ #include #include -extern "C" int SafeFetch32_impl(int* adr, int errValue); +extern "C" int safefetch32_impl(int* adr, int errValue); +extern "C" int safefetch32_continuation(int* adr, int errValue); #ifdef __clang__ #define NOINLINE __attribute__((noinline)) @@ -35,7 +36,7 @@ class SafeAccess { public: static inline int safeFetch32(int* ptr, int errorValue) { - return SafeFetch32_impl(ptr, errorValue); + return safefetch32_impl(ptr, errorValue); } static bool handle_safefetch(int sig, void* context); From ca9c47b0caf7857fff5ae1cff62d76eb81ee9b51 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Thu, 17 Jul 2025 13:59:53 -0400 Subject: [PATCH 12/15] fix --- ddprof-lib/src/main/cpp/safeAccess.h | 1 - ddprof-lib/src/test/cpp/safefetch_ut.cpp | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ddprof-lib/src/main/cpp/safeAccess.h b/ddprof-lib/src/main/cpp/safeAccess.h index 19e83beb..4030ffa0 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.h +++ b/ddprof-lib/src/main/cpp/safeAccess.h @@ -23,7 +23,6 @@ #include extern "C" int safefetch32_impl(int* adr, int errValue); -extern "C" int safefetch32_continuation(int* adr, int errValue); #ifdef __clang__ #define NOINLINE __attribute__((noinline)) diff --git a/ddprof-lib/src/test/cpp/safefetch_ut.cpp b/ddprof-lib/src/test/cpp/safefetch_ut.cpp index 4e36b862..3519fa44 100644 --- a/ddprof-lib/src/test/cpp/safefetch_ut.cpp +++ b/ddprof-lib/src/test/cpp/safefetch_ut.cpp @@ -12,9 +12,9 @@ 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) { + if (signo == SIGBUS && orig_busHandler != nullptr) { orig_busHandler(signo, siginfo, context); - } else if (signo == SIGSEGV) { + } else if (signo == SIGSEGV && orig_segvHandler != nullptr) { orig_segvHandler(signo, siginfo, context); } } From 66c69adb92a6672f3caeac9da5241103d99ca517 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Thu, 17 Jul 2025 14:06:13 -0400 Subject: [PATCH 13/15] Cleanup --- ddprof-lib/src/main/cpp/safeAccess.cpp | 44 ++++++++++++++------------ 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 9aa1eda9..411c4f3b 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -19,18 +19,20 @@ #include #include +extern "C" int safefetch32_cont(int* adr, int errValue); + #ifdef __APPLE__ #if defined(__x86_64__) - #define context_pc context_rip + #define current_pc context_rip #elif defined(__aarch64__) #define DU3_PREFIX(s, m) __ ## s.__ ## m - #define context_pc uc_mcontext->DU3_PREFIX(ss,pc) + #define current_pc uc_mcontext->DU3_PREFIX(ss,pc) #endif #else #if defined(__x86_64__) - #define context_pc uc_mcontext.gregs[REG_RIP] + #define current_pc uc_mcontext.gregs[REG_RIP] #elif defined(__aarch64__) - #define context_pc uc_mcontext.pc + #define current_pc uc_mcontext.pc #endif #endif @@ -42,24 +44,25 @@ _safefetch32_impl: movl (%rdi), %eax ret - .globl _safefetch32_continuation - .private_extern _safefetch32_continuation - _safefetch32_continuation: + .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_continuation - .hidden safefetch32_continuation - .type safefetch32_continuation, %function - safefetch32_continuation: + .globl safefetch32_cont + .hidden safefetch32_cont + .type safefetch32_cont, %function + safefetch32_cont: movl %esi, %eax ret )"); @@ -72,24 +75,25 @@ _safefetch32_impl: ldr w0, [x0] ret - .globl _safefetch32_continuation - .private_extern _safefetch32_continuation - _safefetch32_continuation: + .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_continuation - .hidden safefetch32_continuation - .type safefetch32_continuation, %function - safefetch32_continuation: + .globl safefetch32_cont + .hidden safefetch32_cont + .type safefetch32_cont, %function + safefetch32_cont: mov x0, x1 ret )"); @@ -98,10 +102,10 @@ bool SafeAccess::handle_safefetch(int sig, void* context) { ucontext_t* uc = (ucontext_t*)context; - uintptr_t pc = uc->context_pc; + uintptr_t pc = uc->current_pc; if ((sig == SIGSEGV || sig == SIGBUS) && uc != nullptr) { if (pc == (uintptr_t)safefetch32_impl) { - uc->context_pc = (uintptr_t)safefetch32_continuation; + uc->current_pc = (uintptr_t)safefetch32_cont; return true; } } From ca37ca6b1408f4d16b6df41ac05d90b4e88f4cf1 Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Thu, 17 Jul 2025 14:18:06 -0400 Subject: [PATCH 14/15] formatting --- ddprof-lib/build.gradle | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ddprof-lib/build.gradle b/ddprof-lib/build.gradle index b25ffc57..51438cbc 100644 --- a/ddprof-lib/build.gradle +++ b/ddprof-lib/build.gradle @@ -519,7 +519,6 @@ tasks.whenTaskAdded { task -> group = 'build' description = "Link the ${config.name} build of the library" source = fileTree("$buildDir/obj/main/${config.name}") - linkedFile = file("$buildDir/lib/main/${config.name}/${osIdentifier()}/${archIdentifier()}/libjavaProfiler.${os().isLinux() ? 'so' : 'dylib'}") def compileTask = tasks.findByName("compileLib${config.name.capitalize()}".toString()) if (compileTask != null) { @@ -605,13 +604,15 @@ tasks.register('javadocJar', Jar) { from javadoc.destinationDir } + + tasks.register('scanBuild', Exec) { workingDir "${projectDir}/src/test/make" commandLine 'scan-build' args "-o", "${projectDir}/build/reports/scan-build", - "--force-analyze-debug-code", - "--use-analyzer", "/usr/bin/clang++", - "make", "-j4", "clean", "all" + "--force-analyze-debug-code", + "--use-analyzer", "/usr/bin/clang++", + "make", "-j4", "clean", "all" } tasks.withType(Test) { From 1c299c4353a3a4de1db8373ac85efbf34f5fec7a Mon Sep 17 00:00:00 2001 From: Zhengyu Gu Date: Fri, 18 Jul 2025 09:10:02 -0400 Subject: [PATCH 15/15] add comment --- ddprof-lib/src/main/cpp/safeAccess.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ddprof-lib/src/main/cpp/safeAccess.cpp b/ddprof-lib/src/main/cpp/safeAccess.cpp index 411c4f3b..2fea8de3 100644 --- a/ddprof-lib/src/main/cpp/safeAccess.cpp +++ b/ddprof-lib/src/main/cpp/safeAccess.cpp @@ -36,6 +36,13 @@ extern "C" int safefetch32_cont(int* adr, int errValue); #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"(