diff --git a/src/flightRecorder.cpp b/src/flightRecorder.cpp index 1f4763da..60dcb31d 100644 --- a/src/flightRecorder.cpp +++ b/src/flightRecorder.cpp @@ -173,31 +173,24 @@ class Lookup { } bool fillJavaMethodInfo(MethodInfo* mi, jmethodID method, bool first_time) { - if (VMStructs::hasMethodStructs()) { - // Workaround for JDK-8313816 - VMMethod* vm_method = VMMethod::fromMethodID(method); - if (vm_method == NULL || vm_method->id() == NULL) { - return false; - } + if (VMMethod::isStaleMethodId(method)) { + return false; } - jvmtiEnv* jvmti = VM::jvmti(); - jclass method_class = NULL; char* class_name = NULL; char* method_name = NULL; char* method_sig = NULL; - if (jvmti->GetMethodName(method, &method_name, &method_sig, NULL) == 0 && - jvmti->GetMethodDeclaringClass(method, &method_class) == 0 && - jvmti->GetClassSignature(method_class, &class_name, NULL) == 0) { + jvmtiEnv* jvmti = VM::jvmti(); + jvmtiError err; + + if ((err = jvmti->GetMethodName(method, &method_name, &method_sig, NULL)) == 0 && + (err = jvmti->GetMethodDeclaringClass(method, &method_class)) == 0 && + (err = jvmti->GetClassSignature(method_class, &class_name, NULL)) == 0) { mi->_class = _classes->lookup(class_name + 1, strlen(class_name) - 2); mi->_name = _symbols.lookup(method_name); mi->_sig = _symbols.lookup(method_sig); - } else { - mi->_class = _classes->lookup(""); - mi->_name = _symbols.lookup("jvmtiError"); - mi->_sig = _symbols.lookup("()L;"); } if (method_class) { @@ -207,6 +200,10 @@ class Lookup { jvmti->Deallocate((unsigned char*)method_name); jvmti->Deallocate((unsigned char*)class_name); + if (err != 0) { + return false; + } + if (first_time && jvmti->GetMethodModifiers(method, &mi->_modifiers) != 0) { mi->_modifiers = 0; } diff --git a/src/frameName.cpp b/src/frameName.cpp index 6de12f30..09371efb 100644 --- a/src/frameName.cpp +++ b/src/frameName.cpp @@ -155,13 +155,9 @@ const char* FrameName::typeSuffix(FrameTypeId type) { } void FrameName::javaMethodName(jmethodID method) { - if (VMStructs::hasMethodStructs()) { - // Workaround for JDK-8313816 - VMMethod* vm_method = VMMethod::fromMethodID(method); - if (vm_method == NULL || vm_method->id() == NULL) { - _str.assign("[stale_jmethodID]"); - return; - } + if (VMMethod::isStaleMethodId(method)) { + _str.assign("[stale_jmethodID]"); + return; } jclass method_class = NULL; @@ -186,6 +182,8 @@ void FrameName::javaMethodName(jmethodID method) { } _str.append(method_sig); } + } else if (err == JVMTI_ERROR_INVALID_METHODID) { + _str.assign("[stale_jmethodID]"); } else { char buf[32]; snprintf(buf, sizeof(buf), "[jvmtiError %d]", err); diff --git a/src/stackWalker.cpp b/src/stackWalker.cpp index 8b9a7e43..cf756670 100644 --- a/src/stackWalker.cpp +++ b/src/stackWalker.cpp @@ -53,10 +53,7 @@ static inline void fillFrame(ASGCT_CallFrame& frame, FrameTypeId type, int bci, static jmethodID getMethodId(VMMethod* method) { if (!inDeadZone(method) && aligned((uintptr_t)method)) { - jmethodID method_id = method->id(); - if (!inDeadZone(method_id) && aligned((uintptr_t)method_id) && VMMethod::fromMethodID(method_id) == method) { - return method_id; - } + return method->validatedId(); } return NULL; } diff --git a/src/vmStructs.cpp b/src/vmStructs.cpp index b9b8cd60..c2b31512 100644 --- a/src/vmStructs.cpp +++ b/src/vmStructs.cpp @@ -20,6 +20,7 @@ bool VMStructs::_has_stack_structs = false; bool VMStructs::_has_class_loader_data = false; bool VMStructs::_has_native_thread_id = false; bool VMStructs::_has_perm_gen = false; +bool VMStructs::_can_dereference_jmethod_id = false; bool VMStructs::_compact_object_headers = false; int VMStructs::_klass_name_offset = -1; @@ -519,6 +520,9 @@ void VMStructs::resolveOffsets() { && _thread_exception_offset >= 0 && _constmethod_size >= 0; + // Since JDK-8268406, it is no longer possible to get VMMethod* by dereferencing jmethodID + _can_dereference_jmethod_id = _has_method_structs && VM::hotspot_version() <= 25; + if (_code_heap_addr != NULL && _code_heap_low_addr != NULL && _code_heap_high_addr != NULL) { char* code_heaps = *_code_heap_addr; unsigned int code_heap_count = *(unsigned int*)(code_heaps + _array_len_offset); @@ -665,6 +669,16 @@ jmethodID VMMethod::id() { return NULL; } +jmethodID VMMethod::validatedId() { + jmethodID method_id = id(); + if (goodPtr(method_id)) { + if (!_can_dereference_jmethod_id || *(VMMethod**)method_id == this) { + return method_id; + } + } + return NULL; +} + NMethod* CodeHeap::findNMethod(char* heap, const void* pc) { unsigned char* heap_start = *(unsigned char**)(heap + _code_heap_memory_offset + _vs_low_offset); unsigned char* segmap = *(unsigned char**)(heap + _code_heap_segmap_offset + _vs_low_offset); diff --git a/src/vmStructs.h b/src/vmStructs.h index a572608d..6915e99e 100644 --- a/src/vmStructs.h +++ b/src/vmStructs.h @@ -26,6 +26,7 @@ class VMStructs { static bool _has_class_loader_data; static bool _has_native_thread_id; static bool _has_perm_gen; + static bool _can_dereference_jmethod_id; static bool _compact_object_headers; static int _klass_name_offset; @@ -385,12 +386,18 @@ class VMThread : VMStructs { class VMMethod : VMStructs { public: - static VMMethod* fromMethodID(jmethodID id) { - return *(VMMethod**)id; - } - jmethodID id(); + // Performs extra validation when VMMethod comes from incomplete frame + jmethodID validatedId(); + + // Workaround for JDK-8313816 + static bool isStaleMethodId(jmethodID id) { + if (!_can_dereference_jmethod_id) return false; + VMMethod* vm_method = *(VMMethod**)id; + return vm_method == NULL || vm_method->id() == NULL; + } + const char* bytecode() { return *(const char**) at(_method_constmethod_offset) + _constmethod_size; }