Fix VMStructs::initJvmFunctions call order (async-profiler#1658)

fandreuz · web-flow · commit 872be632202b · 2026-01-15T10:58:30.000Z
diff --git a/src/profiler.cpp b/src/profiler.cpp
@@ -869,11 +869,6 @@ void Profiler::crashHandler(int signo, siginfo_t* siginfo, void* ucontext) {
         StackWalker::checkFault();
     }
 
-    // Workaround for JDK-8313796. Setting cstack=dwarf also helps
-    if (VMStructs::isInterpretedFrameValidFunc((const void*)pc) && frame.skipFaultInstruction()) {
-        return;
-    }
-
     if (WX_MEMORY && Trap::isFaultInstruction(pc)) {
         return;
     }
diff --git a/src/stackFrame.h b/src/stackFrame.h
@@ -75,8 +75,6 @@ class StackFrame {
 
     void adjustSP(const void* entry, const void* pc, uintptr_t& sp);
 
-    bool skipFaultInstruction();
-
     bool checkInterruptedSyscall();
 
     // Check if PC points to a syscall instruction
diff --git a/src/stackFrame_aarch64.cpp b/src/stackFrame_aarch64.cpp
@@ -353,10 +353,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {
     }
 }
 
-bool StackFrame::skipFaultInstruction() {
-    return false;
-}
-
 bool StackFrame::checkInterruptedSyscall() {
 #ifdef __APPLE__
     // We are not interested in syscalls that do not check error code, e.g. semaphore_wait_trap
diff --git a/src/stackFrame_arm.cpp b/src/stackFrame_arm.cpp
@@ -125,10 +125,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {
     // Not needed
 }
 
-bool StackFrame::skipFaultInstruction() {
-    return false;
-}
-
 bool StackFrame::checkInterruptedSyscall() {
     return retval() == (uintptr_t)-EINTR;
 }
diff --git a/src/stackFrame_i386.cpp b/src/stackFrame_i386.cpp
@@ -146,10 +146,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {
     // Not needed
 }
 
-bool StackFrame::skipFaultInstruction() {
-    return false;
-}
-
 bool StackFrame::checkInterruptedSyscall() {
     return retval() == (uintptr_t)-EINTR;
 }
diff --git a/src/stackFrame_loongarch64.cpp b/src/stackFrame_loongarch64.cpp
@@ -101,10 +101,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {
     // Not yet implemented
 }
 
-bool StackFrame::skipFaultInstruction() {
-    return false;
-}
-
 bool StackFrame::checkInterruptedSyscall() {
     return retval() == (uintptr_t)-EINTR;
 }
diff --git a/src/stackFrame_ppc64.cpp b/src/stackFrame_ppc64.cpp
@@ -146,10 +146,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {
     // Not needed
 }
 
-bool StackFrame::skipFaultInstruction() {
-    return false;
-}
-
 bool StackFrame::checkInterruptedSyscall() {
     return retval() == (uintptr_t)-EINTR;
 }
diff --git a/src/stackFrame_riscv64.cpp b/src/stackFrame_riscv64.cpp
@@ -101,10 +101,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {
     // Not yet implemented
 }
 
-bool StackFrame::skipFaultInstruction() {
-    return false;
-}
-
 bool StackFrame::checkInterruptedSyscall() {
     return retval() == (uintptr_t)-EINTR;
 }
diff --git a/src/stackFrame_x64.cpp b/src/stackFrame_x64.cpp
@@ -247,40 +247,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {
     // Not needed
 }
 
-// Skip failed MOV instruction by writing 0 to destination register
-bool StackFrame::skipFaultInstruction() {
-    unsigned int insn = *(unsigned int*)pc();
-    if ((insn & 0x80fff8) == 0x008b48) {
-        // mov r64, [r64 + offs]
-        unsigned int reg = ((insn << 1) & 8) | ((insn >> 19) & 7);
-        switch (reg) {
-            case 0x0: REG(RAX, rax) = 0; break;
-            case 0x1: REG(RCX, rcx) = 0; break;
-            case 0x2: REG(RDX, rdx) = 0; break;
-            case 0x3: REG(RBX, rbx) = 0; break;
-            case 0x4: return false;  // Do not modify RSP
-            case 0x5: REG(RBP, rbp) = 0; break;
-            case 0x6: REG(RSI, rsi) = 0; break;
-            case 0x7: REG(RDI, rdi) = 0; break;
-            case 0x8: REG(R8 , r8 ) = 0; break;
-            case 0x9: REG(R9 , r9 ) = 0; break;
-            case 0xa: REG(R10, r10) = 0; break;
-            case 0xb: REG(R11, r11) = 0; break;
-            case 0xc: REG(R12, r12) = 0; break;
-            case 0xd: REG(R13, r13) = 0; break;
-            case 0xe: REG(R14, r14) = 0; break;
-            case 0xf: REG(R15, r15) = 0; break;
-        }
-
-        unsigned int insn_size = 3;
-        if ((insn & 0x070000) == 0x040000) insn_size++;
-        if ((insn & 0x400000) == 0x400000) insn_size++;
-        pc() += insn_size;
-        return true;
-    }
-    return false;
-}
-
 bool StackFrame::checkInterruptedSyscall() {
 #ifdef __APPLE__
     // We are not interested in syscalls that do not check error code, e.g. semaphore_wait_trap
diff --git a/src/vmStructs.cpp b/src/vmStructs.cpp
@@ -105,8 +105,6 @@ int VMStructs::_interpreter_frame_bcp_offset = 0;
 unsigned char VMStructs::_unsigned5_base = 0;
 const void** VMStructs::_call_stub_return_addr = NULL;
 const void* VMStructs::_call_stub_return = NULL;
-const void* VMStructs::_interpreted_frame_valid_start = NULL;
-const void* VMStructs::_interpreted_frame_valid_end = NULL;
 
 jfieldID VMStructs::_eetop;
 jfieldID VMStructs::_tid;
@@ -133,7 +131,6 @@ void VMStructs::init(CodeCache* libjvm) {
     if (libjvm != NULL) {
         _libjvm = libjvm;
         initOffsets();
-        initJvmFunctions();
     }
 }
 
@@ -479,11 +476,16 @@ void VMStructs::resolveOffsets() {
             && _comp_task_offset >= 0
             && _comp_method_offset >= 0;
 
-    _has_class_loader_data = _class_loader_data_offset >= 0
-            && _class_loader_data_next_offset == sizeof(uintptr_t) * 8 + 8
-            && _methods_offset >= 0
-            && _klass != NULL
-            && _lock_func != NULL && _unlock_func != NULL;
+    if (VM::hotspot_version() == 8) {
+        _lock_func = (LockFunc)_libjvm->findSymbol("_ZN7Monitor28lock_without_safepoint_checkEv");
+        _unlock_func = (LockFunc)_libjvm->findSymbol("_ZN7Monitor6unlockEv");
+        _has_class_loader_data = _class_loader_data_offset >= 0
+                && _class_loader_data_next_offset == sizeof(uintptr_t) * 8 + 8
+                && _methods_offset >= 0
+                && _klass != NULL
+                && _lock_func != NULL
+                && _unlock_func != NULL;
+    }
 
 #if defined(__x86_64__) || defined(__i386__)
     _interpreter_frame_bcp_offset = VM::hotspot_version() >= 11 ? -8 : VM::hotspot_version() == 8 ? -7 : 0;
@@ -556,21 +558,6 @@ void VMStructs::resolveOffsets() {
     }
 }
 
-void VMStructs::initJvmFunctions() {
-    if (VM::hotspot_version() == 8) {
-        _lock_func = (LockFunc)_libjvm->findSymbol("_ZN7Monitor28lock_without_safepoint_checkEv");
-        _unlock_func = (LockFunc)_libjvm->findSymbol("_ZN7Monitor6unlockEv");
-    }
-
-    if (VM::hotspot_version() > 0) {
-        CodeBlob* blob = _libjvm->findBlob("_ZNK5frame26is_interpreted_frame_validEP10JavaThread");
-        if (blob != NULL) {
-            _interpreted_frame_valid_start = blob->_start;
-            _interpreted_frame_valid_end = blob->_end;
-        }
-    }
-}
-
 void VMStructs::patchSafeFetch() {
     // Workarounds for JDK-8307549 and JDK-8321116
     if (WX_MEMORY && VM::hotspot_version() == 17) {
diff --git a/src/vmStructs.h b/src/vmStructs.h
@@ -111,8 +111,6 @@ class VMStructs {
     static unsigned char _unsigned5_base;
     static const void** _call_stub_return_addr;
     static const void* _call_stub_return;
-    static const void* _interpreted_frame_valid_start;
-    static const void* _interpreted_frame_valid_end;
 
     static jfieldID _eetop;
     static jfieldID _tid;
@@ -129,7 +127,6 @@ class VMStructs {
     static void initOffsets();
     static void resolveOffsets();
     static void patchSafeFetch();
-    static void initJvmFunctions();
     static void initTLS(void* vm_thread);
     static void initThreadBridge();
 
@@ -182,10 +179,6 @@ class VMStructs {
     static bool hasJavaThreadId() {
         return _tid != NULL;
     }
-
-    static bool isInterpretedFrameValidFunc(const void* pc) {
-        return pc >= _interpreted_frame_valid_start && pc < _interpreted_frame_valid_end;
-    }
 };
 
 

Original file line number	Diff line number	Diff line change
`@@ -869,11 +869,6 @@ void Profiler::crashHandler(int signo, siginfo_t* siginfo, void* ucontext) {`
`869`	`869`	`StackWalker::checkFault();`
`870`	`870`	`}`
`871`	`871`
`872`		`- // Workaround for JDK-8313796. Setting cstack=dwarf also helps`
`873`		`- if (VMStructs::isInterpretedFrameValidFunc((const void*)pc) && frame.skipFaultInstruction()) {`
`874`		`- return;`
`875`		`- }`
`876`		`-`
`877`	`872`	`if (WX_MEMORY && Trap::isFaultInstruction(pc)) {`
`878`	`873`	`return;`
`879`	`874`	`}`
Original file line number	Diff line number	Diff line change
`@@ -353,10 +353,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {`
`353`	`353`	`}`
`354`	`354`	`}`
`355`	`355`
`356`		`-bool StackFrame::skipFaultInstruction() {`
`357`		`- return false;`
`358`		`-}`
`359`		`-`
`360`	`356`	`bool StackFrame::checkInterruptedSyscall() {`
`361`	`357`	`#ifdef __APPLE__`
`362`	`358`	`// We are not interested in syscalls that do not check error code, e.g. semaphore_wait_trap`
Original file line number	Diff line number	Diff line change
`@@ -125,10 +125,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {`
`125`	`125`	`// Not needed`
`126`	`126`	`}`
`127`	`127`
`128`		`-bool StackFrame::skipFaultInstruction() {`
`129`		`- return false;`
`130`		`-}`
`131`		`-`
`132`	`128`	`bool StackFrame::checkInterruptedSyscall() {`
`133`	`129`	`return retval() == (uintptr_t)-EINTR;`
`134`	`130`	`}`
Original file line number	Diff line number	Diff line change
`@@ -146,10 +146,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {`
`146`	`146`	`// Not needed`
`147`	`147`	`}`
`148`	`148`
`149`		`-bool StackFrame::skipFaultInstruction() {`
`150`		`- return false;`
`151`		`-}`
`152`		`-`
`153`	`149`	`bool StackFrame::checkInterruptedSyscall() {`
`154`	`150`	`return retval() == (uintptr_t)-EINTR;`
`155`	`151`	`}`
Original file line number	Diff line number	Diff line change
`@@ -101,10 +101,6 @@ void StackFrame::adjustSP(const void* entry, const void* pc, uintptr_t& sp) {`
`101`	`101`	`// Not yet implemented`
`102`	`102`	`}`
`103`	`103`
`104`		`-bool StackFrame::skipFaultInstruction() {`
`105`		`- return false;`
`106`		`-}`
`107`		`-`
`108`	`104`	`bool StackFrame::checkInterruptedSyscall() {`
`109`	`105`	`return retval() == (uintptr_t)-EINTR;`
`110`	`106`	`}`