diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 910dc7006..8663a8cf8 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -1335,6 +1335,35 @@ int HotspotSupport::walkJavaStack(StackWalkRequest& request) { return java_frames; } +class LockState { +private: + VMClassLoaderData* volatile _cld; +public: + // Non-copyable + LockState(const LockState&) = delete; + LockState& operator=(const LockState&) = delete; + + LockState() : _cld(nullptr) {} + ~LockState() { reset(); } + void lock(VMClassLoaderData* cld); + void reset(); +}; + +void LockState::lock(VMClassLoaderData* cld) { + cld->lock(); + _cld = cld; +} + +void LockState::reset() { + // Assume: if _cld->lock() did not fail, _cld->unlock() should not + // fail as well. + // The siglongjmp cannot protect _cld->lock()/unlock() calls + if (_cld != nullptr) { + _cld->unlock(); + _cld = nullptr; + } +} + static void patchClassLoaderData(JNIEnv* jni, jclass klass) { bool needs_patch = VM::hotspot_version() == 8; if (needs_patch) { @@ -1342,15 +1371,62 @@ static void patchClassLoaderData(JNIEnv* jni, jclass klass) { // Preallocate space for jmethodIDs at the beginning of the list (rather than at the end) // This is relevant only for JDK 8 - later versions do not have this bug if (VMStructs::hasClassLoaderData()) { + ProfiledThread* prof_thread = ProfiledThread::initCurrentThreadSignalSafe(); + assert(prof_thread != nullptr); + JmpCtxScope jmp_scope(prof_thread); + sigjmp_buf crash_protection_ctx; + LockState state; + if (sigsetjmp(crash_protection_ctx, 1) != 0) { + SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); + jmp_scope.restore(); + state.reset(); + return; + } + jmp_scope.install(&crash_protection_ctx); VMKlass *vmklass = VMKlass::fromJavaClass(jni, klass); int method_count = vmklass->methodCount(); if (method_count > 0) { - VMClassLoaderData *cld = vmklass->classLoaderData(); - cld->lock(); - for (int i = 0; i < method_count; i += MethodList::SIZE) { - *cld->methodList() = new MethodList(*cld->methodList()); + // patchClassLoaderData() re-runs for the same class on every ClassPrepare + // replay (profiler restart via loadAllMethodIDsIfNeeded()), RedefineClasses + // and RetransformClasses -- none of which change method_count in practice. + // Without this tag, each re-run would prepend another full set of + // MethodList blocks onto the classloader-wide list that nothing ever frees. + // The tag lives on the jclass itself, so it disappears with the class -- + // no separate bookkeeping to leak or to clean up on unload. + jvmtiEnv* jvmti = VM::jvmti(); + jlong already_patched = 0; + if (jvmti == nullptr || jvmti->GetTag(klass, &already_patched) != JVMTI_ERROR_NONE) { + already_patched = 0; + } + if (method_count > already_patched) { + VMClassLoaderData *cld = vmklass->classLoaderData(); + if (cld == nullptr) { + return; + } + state.lock(cld); + int i; + for (i = (int) already_patched; i < method_count; i += MethodList::SIZE) { + *cld->methodList() = new MethodList(*cld->methodList()); + } + // Release cld's lock before touching the JVMTI tag: cld->lock() + // resolves to HotSpot's Monitor::lock_without_safepoint_check(), and + // GetTag()/SetTag() are full JVMTI entry points that do participate + // in safepoint polling -- calling them while holding a + // safepoint-check-suppressing lock is a JVM-deadlock hazard on its + // own, independent of any fault/siglongjmp path. + state.reset(); + // This SetTag() is not serialized against a concurrent + // patchClassLoaderData() call for the same class (e.g. + // RetransformClasses on one thread racing the fallback on + // the JFR dump thread): both can read the same stale tag and both + // patch. That only wastes one extra round of preallocated blocks in + // the rare concurrent case -- unlike the unguarded original, it + // cannot grow unboundedly, since the racing calls all still need a + // fresh method_count increase to trigger another round. + if (jvmti != nullptr) { + jvmti->SetTag(klass, i); + } } - cld->unlock(); } } } diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h index 38299aba3..82eac63aa 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h @@ -606,6 +606,8 @@ class MethodList { _method[i] = 0x37; } } + + friend class MethodListTestAccessor; }; diff --git a/ddprof-lib/src/test/cpp/hotspotSupport_ut.cpp b/ddprof-lib/src/test/cpp/hotspotSupport_ut.cpp index 42ba2d717..dce1b3023 100644 --- a/ddprof-lib/src/test/cpp/hotspotSupport_ut.cpp +++ b/ddprof-lib/src/test/cpp/hotspotSupport_ut.cpp @@ -4,7 +4,13 @@ #include #include "../../main/cpp/hotspot/hotspotSupport.h" +#include "../../main/cpp/hotspot/vmStructs.h" #include "../../main/cpp/gtest_crash_handler.h" +#include "../../main/cpp/threadLocalData.h" +#include "../../main/cpp/vmEntry.h" + +#include +#include static constexpr char HOTSPOT_SUPPORT_TEST_NAME[] = "HotspotSupportTest"; class HotspotSupportGlobalSetup { @@ -73,3 +79,288 @@ TEST_F(HotspotSupportLoadMethodIDsTest, LoadAllSucceedsAndCallsGetClassMethodsOn "(load_all=true), which is what now applies patchClassLoaderData() " "before allocating jmethodIDs"; } + +// --------------------------------------------------------------------------- +// patchClassLoaderData() tag/resume regression tests +// +// patchClassLoaderData() (the JDK-8062116 preallocation workaround, only +// active on JDK 8) tags each jclass with how many of its methods have +// already been preallocated into the ClassLoaderData-wide MethodList, so a +// replayed ClassPrepare (loadAllMethodIDsIfNeeded() on profiler restart), +// RedefineClasses or RetransformClasses patches only the new tail +// [already_patched, method_count) instead of re-prepending a full set of +// blocks every time. The test above never exercises this: it leaves +// VM::hotspot_version() at its gtest-binary default (not 8), so +// patchClassLoaderData() is a no-op there. These tests force hotspot_version +// 8 and fake the VMKlass/VMClassLoaderData memory layout so the tag/resume +// loop itself runs. +// --------------------------------------------------------------------------- + +// Friend of VM: lets these tests force isHotspot()/hotspot_version()==8 and +// swap VM::jvmti() for a mock. patchClassLoaderData() reads VM::jvmti() +// directly -- not the jvmti argument threaded through +// loadMethodIDsIfNeededImpl -- for GetTag/SetTag, so both must point at the +// same mock. +class VMTestAccessor { +public: + static bool getHotspot() { return VM::_hotspot; } + static void setHotspot(bool v) { VM::_hotspot = v; } + static int getHotspotVersion() { return VM::_hotspot_version; } + static void setHotspotVersion(int v) { VM::_hotspot_version = v; } + static jvmtiEnv* getJvmti() { return VM::_jvmti; } + static void setJvmti(jvmtiEnv* env) { VM::_jvmti = env; } +}; + +// Friend of VMStructs: lets these tests point VMKlass::methodCount()/ +// classLoaderData() and VMClassLoaderData::lock()/unlock()/methodList() at a +// fake in-memory layout instead of real HotSpot metadata. +class VMStructsTestAccessor { +public: + struct State { + bool has_class_loader_data; + int methods_offset; + int class_loader_data_offset; + uint64_t vmklass_size; + uint64_t vmcld_size; + VMStructs::LockFunc lock_func; + VMStructs::LockFunc unlock_func; + }; + + static State save() { + return State{ + VMStructs::_has_class_loader_data, + VMStructs::_methods_offset, + VMStructs::_class_loader_data_offset, + VMStructs::TYPE_SIZE_NAME(VMKlass), + VMStructs::TYPE_SIZE_NAME(VMClassLoaderData), + VMStructs::_lock_func, + VMStructs::_unlock_func, + }; + } + + static void apply(const State& s) { + VMStructs::_has_class_loader_data = s.has_class_loader_data; + VMStructs::_methods_offset = s.methods_offset; + VMStructs::_class_loader_data_offset = s.class_loader_data_offset; + VMStructs::TYPE_SIZE_NAME(VMKlass) = s.vmklass_size; + VMStructs::TYPE_SIZE_NAME(VMClassLoaderData) = s.vmcld_size; + VMStructs::_lock_func = s.lock_func; + VMStructs::_unlock_func = s.unlock_func; + } +}; + +// Friend of MethodList: patchClassLoaderData() prepends nodes to a private +// linked list with no production accessor. Walking _next is the only way to +// count how many nodes a given call actually prepended. +class MethodListTestAccessor { +public: + static const MethodList* next(const MethodList* node) { return node->_next; } +}; + +static int methodListChainLength(const MethodList* head) { + int n = 0; + while (head != nullptr) { + n++; + head = MethodListTestAccessor::next(head); + } + return n; +} + +namespace { + +// Fake ClassLoaderData. VMClassLoaderData::mutex() and methodList() index at +// fixed byte offsets from `this` (sizeof(uintptr_t)*3 and *6+8 respectively) +// baked into vmStructs.h -- not offsets this test controls -- so the fake +// layout must match those exactly. +struct FakePatchCLD { + alignas(sizeof(void*)) char pad0[sizeof(uintptr_t) * 3]; + void* mutex_ptr; // consumed by mutex(); never dereferenced by the no-op lock/unlock mocks below. + char pad1[sizeof(uintptr_t) * 6 + 8 - (sizeof(uintptr_t) * 3 + sizeof(void*))]; + MethodList* method_list_head; +}; +static_assert(offsetof(FakePatchCLD, mutex_ptr) == sizeof(uintptr_t) * 3, + "mutex() offset drifted"); +static_assert(offsetof(FakePatchCLD, method_list_head) == sizeof(uintptr_t) * 6 + 8, + "methodList() offset drifted"); + +// Fake VMKlass: a methods-table pointer (methodCount() masks its low 16 +// bits) and a ClassLoaderData pointer, at offsets this test wires up via +// VMStructsTestAccessor. +struct FakePatchKlass { + int* methods_table; + FakePatchCLD* cld; +}; + +int g_lock_calls = 0; +int g_unlock_calls = 0; +void noopLock(void*) { g_lock_calls++; } +void noopUnlock(void*) { g_unlock_calls++; } + +jlong g_tag = 0; +int g_get_tag_calls = 0; +int g_set_tag_calls = 0; +jlong g_last_set_tag_value = -1; + +jvmtiError JNICALL mock_GetTag(jvmtiEnv*, jobject, jlong* tag_ptr) { + g_get_tag_calls++; + *tag_ptr = g_tag; + return JVMTI_ERROR_NONE; +} +jvmtiError JNICALL mock_SetTag(jvmtiEnv*, jobject, jlong tag) { + g_set_tag_calls++; + g_tag = tag; + g_last_set_tag_value = tag; + return JVMTI_ERROR_NONE; +} + +FakePatchKlass* g_fake_klass_for_jni = nullptr; + +jlong JNICALL mock_GetLongField(JNIEnv*, jobject, jfieldID) { + return (jlong)(intptr_t)g_fake_klass_for_jni; +} + +} // namespace + +class PatchClassLoaderDataTest : public ::testing::Test { +protected: + jvmtiInterface_1_ tbl{}; + _jvmtiEnv mock_jvmti{}; + JNINativeInterface_ jni_tbl{}; + JNIEnv_ mock_jni{}; + + FakePatchCLD fake_cld{}; + FakePatchKlass fake_klass{}; + int methods_header = 0; + + bool saved_hotspot = false; + int saved_hotspot_version = 0; + jvmtiEnv* saved_jvmti = nullptr; + VMStructsTestAccessor::State saved_structs{}; + + void SetUp() override { + saved_hotspot = VMTestAccessor::getHotspot(); + saved_hotspot_version = VMTestAccessor::getHotspotVersion(); + saved_jvmti = VMTestAccessor::getJvmti(); + saved_structs = VMStructsTestAccessor::save(); + + g_tag = 0; + g_get_tag_calls = 0; + g_set_tag_calls = 0; + g_last_set_tag_value = -1; + g_lock_calls = 0; + g_unlock_calls = 0; + g_get_class_methods_calls = 0; + + tbl = jvmtiInterface_1_{}; + tbl.GetTag = &mock_GetTag; + tbl.SetTag = &mock_SetTag; + tbl.GetClassMethods = &mock_GetClassMethods_ok; + tbl.Deallocate = &mock_Deallocate_noop; + mock_jvmti.functions = &tbl; + + jni_tbl = JNINativeInterface_{}; + jni_tbl.GetLongField = &mock_GetLongField; + mock_jni.functions = &jni_tbl; + + fake_cld = FakePatchCLD{}; + fake_klass = FakePatchKlass{}; + fake_klass.cld = &fake_cld; + fake_klass.methods_table = &methods_header; + g_fake_klass_for_jni = &fake_klass; + + VMTestAccessor::setHotspot(true); + VMTestAccessor::setHotspotVersion(8); + VMTestAccessor::setJvmti(&mock_jvmti); + + VMStructsTestAccessor::State s = saved_structs; + s.has_class_loader_data = true; + s.methods_offset = (int)offsetof(FakePatchKlass, methods_table); + s.class_loader_data_offset = (int)offsetof(FakePatchKlass, cld); + s.vmklass_size = sizeof(FakePatchKlass); + s.vmcld_size = sizeof(FakePatchCLD); + s.lock_func = &noopLock; + s.unlock_func = &noopUnlock; + VMStructsTestAccessor::apply(s); + } + + void TearDown() override { + VMStructsTestAccessor::apply(saved_structs); + VMTestAccessor::setJvmti(saved_jvmti); + VMTestAccessor::setHotspotVersion(saved_hotspot_version); + VMTestAccessor::setHotspot(saved_hotspot); + g_fake_klass_for_jni = nullptr; + ProfiledThread::release(); + } + + void setMethodCount(int count) { methods_header = count; } + + void callPatch() { + jclass fake_jclass = reinterpret_cast(0x1); + HotspotSupportTestAccessor::loadMethodIDsIfNeededImpl( + &mock_jvmti, reinterpret_cast(&mock_jni), fake_jclass, /*load_all=*/true); + } +}; + +// The simple case: nothing patched yet (tag defaults to 0), method_count is +// an exact multiple of MethodList::SIZE so already_patched lands exactly on +// method_count -- the boundary the next test replays. +TEST_F(PatchClassLoaderDataTest, FirstCallPatchesFromZeroAndTagsMethodCount) { + setMethodCount(MethodList::SIZE); + + callPatch(); + + EXPECT_EQ(1, g_get_tag_calls); + EXPECT_EQ(1, g_set_tag_calls); + EXPECT_EQ(MethodList::SIZE, g_last_set_tag_value); + EXPECT_EQ(1, methodListChainLength(fake_cld.method_list_head)); + EXPECT_EQ(1, g_lock_calls); + EXPECT_EQ(1, g_unlock_calls); +} + +// The boundary case: a replayed call (same class, unchanged method_count) +// finds already_patched == method_count and must skip the patch block +// entirely -- no lock, no new MethodList node, no SetTag. A mutant that +// widens `method_count > already_patched` to `>=` would re-enter here. +TEST_F(PatchClassLoaderDataTest, SecondCallWithUnchangedMethodCountSkipsPatchEntirely) { + setMethodCount(MethodList::SIZE); + callPatch(); + ASSERT_EQ(1, methodListChainLength(fake_cld.method_list_head)); + ASSERT_EQ(MethodList::SIZE, g_tag); + + g_get_tag_calls = 0; + g_set_tag_calls = 0; + g_lock_calls = 0; + g_unlock_calls = 0; + + callPatch(); + + EXPECT_EQ(1, g_get_tag_calls); + EXPECT_EQ(0, g_set_tag_calls) + << "method_count == already_patched must skip the whole patch block, including SetTag"; + EXPECT_EQ(0, g_lock_calls) << "the ClassLoaderData lock must not be taken when nothing needs patching"; + EXPECT_EQ(1, methodListChainLength(fake_cld.method_list_head)) + << "no new MethodList node should be prepended when nothing changed"; +} + +// Simulates a ClassPrepare replay where the class gained methods since the +// last patch (e.g. RetransformClasses): the second call must patch only the +// new tail [already_patched, method_count), not restart from 0. A mutant +// that restarts the loop at 0 would prepend 3 nodes on the second call +// (ceil(20/8)) instead of 2 (ceil((20-8)/8)), re-walking the already-patched +// [0, 8) range. +TEST_F(PatchClassLoaderDataTest, SecondCallPatchesOnlyNewMethodRange) { + setMethodCount(MethodList::SIZE); // 8 + callPatch(); + ASSERT_EQ(1, methodListChainLength(fake_cld.method_list_head)); + ASSERT_EQ(MethodList::SIZE, g_tag); + + setMethodCount(2 * MethodList::SIZE + 4); // 20 + g_set_tag_calls = 0; + + callPatch(); + + EXPECT_EQ(1, g_set_tag_calls); + EXPECT_EQ(2 * MethodList::SIZE + MethodList::SIZE, g_last_set_tag_value); // 24: next block boundary past 20 + EXPECT_EQ(3, methodListChainLength(fake_cld.method_list_head)) + << "second call must add exactly ceil((20-8)/8) = 2 new nodes on top of the first call's 1"; +}