diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index a622baf638..64c9e2d761 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -1204,7 +1204,7 @@ void Recording::writeSettings(Buffer *buf, Arguments &args) { } writeBoolSetting(buf, T_ACTIVE_RECORDING, "debugSymbols", - VMStructs::libjvm()->hasDebugSymbols()); + VM::libjvm()->hasDebugSymbols()); writeBoolSetting(buf, T_ACTIVE_RECORDING, "kernelSymbols", Symbols::haveKernelSymbols()); writeStringSetting(buf, T_ACTIVE_RECORDING, "cpuEngine", diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp index ccbca45089..73d7e2aec6 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp @@ -342,9 +342,7 @@ void VMStructs::initOffsets() { } void VMStructs::resolveOffsets() { - if (VM::isOpenJ9() || VM::isZing()) { - return; - } + assert(VM::isHotspot() && "Should not reach here"); if (_klass_offset_addr != NULL) { _klass = (jfieldID)(uintptr_t)(*(int*)_klass_offset_addr << 2 | 2); @@ -842,7 +840,8 @@ VMFlag* VMFlag::find(const char* name) { for (size_t i = 0; i < count; i++) { VMFlag* f = VMFlag::cast(*(const char**)_flags_addr + i * VMFlag::type_size()); - if (f->name() != NULL && strcmp(f->name(), name) == 0 && f->addr() != NULL) { + const char* fname = f->name(); + if (fname != nullptr && strcmp(fname, name) == 0 && f->addr() != nullptr) { return f; } } @@ -863,7 +862,8 @@ VMFlag *VMFlag::find(const char *name, int type_mask) { size_t count = *(size_t*)_flag_count; for (size_t i = 0; i < count; i++) { VMFlag* f = VMFlag::cast(*(const char**)_flags_addr + i * VMFlag::type_size()); - if (f->name() != NULL && strcmp(f->name(), name) == 0) { + const char* fname = f->name(); + if (fname != nullptr && strcmp(fname, name) == 0) { int masked = 0x1 << f->type(); if (masked & type_mask) { return (VMFlag*)f; diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h index 9ef2586851..139d430a34 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.h @@ -449,18 +449,11 @@ class VMStructs { static void checkNativeBinding(jvmtiEnv *jvmti, JNIEnv *jni, jmethodID method, void *address); static const void *findHeapUsageFunc(); - const char* at(int offset) { - const char* ptr = (const char*)this + offset; - assert(crashProtectionActive() || SafeAccess::isReadable(ptr)); - // Poison only the returned pointer; the assert above sees the real ptr. - return INJECT_FAULT_ADDRESS_RARE(ptr); - } + inline const char* at(int offset); + inline const char* at(int offset) const; - const char* at(int offset) const { - const char* ptr = (const char*)this + offset; - assert(crashProtectionActive() || SafeAccess::isReadable(ptr)); - return INJECT_FAULT_ADDRESS_RARE(ptr); - } + template + T load_at_offset(int offset) const; static bool goodPtr(const void* ptr) { return (uintptr_t)ptr >= 0x1000 && ((uintptr_t)ptr & (sizeof(uintptr_t) - 1)) == 0; @@ -1136,10 +1129,7 @@ DECLARE(VMFlag) static VMFlag* find(const char* name); static VMFlag *find(const char* name, std::initializer_list types); - const char* name() { - assert(_flag_name_offset >= 0); - return *(const char**) at(_flag_name_offset); - } + inline const char* name() const; int type(); diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h b/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h index 2b7edb3250..5de4c94cad 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h @@ -10,6 +10,7 @@ #include "hotspot/hotspotSupport.h" #include "hotspot/vmStructs.h" #include "jvmThread.h" +#include "safeAccess.h" #include "threadLocalData.h" inline bool crashProtectionActive() { @@ -28,6 +29,31 @@ inline T* cast_to(const void* ptr) { return reinterpret_cast(const_cast(ptr)); } +inline const char* VMStructs::at(int offset) { + const char* ptr = (const char*)this + offset; + assert(crashProtectionActive() || SafeAccess::isReadable(ptr)); + // Poison only the returned pointer; the assert above sees the real ptr. + return INJECT_FAULT_ADDRESS_RARE(ptr); +} + +inline const char* VMStructs::at(int offset) const { + const char* ptr = (const char*)this + offset; + assert(crashProtectionActive() || SafeAccess::isReadable(ptr)); + return INJECT_FAULT_ADDRESS_RARE(ptr); +} + +template +T VMStructs::load_at_offset(int offset) const { + const char* raw = (const char*)this + offset; + if (safe) { + // SafeAccess loads must work even when crash protection isn't active; avoid at() asserts. + return (T)SafeAccess::loadPtr((void**)INJECT_FAULT_ADDRESS_RARE(raw), nullptr); + } + assert(crashProtectionActive() || SafeAccess::isReadable(raw)); + return *((T*)INJECT_FAULT_ADDRESS_RARE(raw)); +} + + VMThread* VMThread::current() { assert(VM::isHotspot()); return VMThread::cast(JVMThread::current()); @@ -188,5 +214,10 @@ u16 VMConstMethod::signatureIndex() const { return *(u16*)at(_constmethod_sig_index_offset); } +// This method may be called without crash protection, so read the name via SafeAccess. +inline const char* VMFlag::name() const { + assert(_flag_name_offset >= 0); + return load_at_offset(_flag_name_offset); +} #endif // _HOTSPOT_VMSTRUCTS_INLINE_H diff --git a/ddprof-lib/src/main/cpp/libraries.cpp b/ddprof-lib/src/main/cpp/libraries.cpp index a0c8e83ca2..66382e9d1b 100644 --- a/ddprof-lib/src/main/cpp/libraries.cpp +++ b/ddprof-lib/src/main/cpp/libraries.cpp @@ -1,7 +1,11 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + #include "codeCache.h" #include "common.h" #include "findLibraryImpl.h" -#include "hotspot/vmStructs.h" #include "libraries.h" #include "libraryPatcher.h" #include "log.h" @@ -191,7 +195,7 @@ const void *Libraries::resolveSymbol(const char *name) { } CodeCache *Libraries::findJvmLibrary(const char *j9_lib_name) { - return VM::isOpenJ9() ? findLibraryByName(j9_lib_name) : VMStructs::libjvm(); + return VM::isOpenJ9() ? findLibraryByName(j9_lib_name) : VM::libjvm(); } CodeCache *Libraries::findLibraryByName(const char *lib_name) { diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index ab674ab9c2..2e303ae8c5 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -1264,7 +1264,7 @@ Error Profiler::checkJvmCapabilities() { } } - if (!VMStructs::libjvm()->hasDebugSymbols() && !VM::isOpenJ9()) { + if (!VM::libjvm()->hasDebugSymbols()) { Log::warn("Install JVM debug symbols to improve profile accuracy"); } diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index 8eadc643d1..670c54af6c 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -58,7 +58,7 @@ jvmtiError(JNICALL *VM::_orig_RedefineClasses)(jvmtiEnv *, jint, jvmtiError(JNICALL *VM::_orig_RetransformClasses)(jvmtiEnv *, jint, const jclass *classes); -void *VM::_libjvm; +CodeCache* VM::_libjvm = nullptr; AsyncGetCallTrace VM::_asyncGetCallTrace; JVM_GetManagement VM::_getManagement; @@ -204,15 +204,20 @@ int JavaVersionAccess::get_hotspot_version(char* prop_value) { } CodeCache* VM::openJvmLibrary() { + CodeCache* lib = __atomic_load_n(&_libjvm, __ATOMIC_ACQUIRE); + if (lib != nullptr) { + return lib; + } + if ((void*)_asyncGetCallTrace == nullptr) { return nullptr; } Libraries* libraries = Libraries::instance(); - CodeCache *lib = - isOpenJ9() + lib = isOpenJ9() ? libraries->findJvmLibrary("libj9vm") : libraries->findLibraryByAddress((const void *)_asyncGetCallTrace); + __atomic_store_n(&_libjvm, lib, __ATOMIC_RELEASE); return lib; } @@ -250,9 +255,9 @@ bool VM::initShared(JavaVM* vm) { prop = NULL; } - _libjvm = getLibraryHandle("libjvm.so"); - _asyncGetCallTrace = (AsyncGetCallTrace)dlsym(_libjvm, "AsyncGetCallTrace"); - _getManagement = (JVM_GetManagement)dlsym(_libjvm, "JVM_GetManagement"); + void *libjvm = getLibraryHandle("libjvm.so"); + _asyncGetCallTrace = (AsyncGetCallTrace)dlsym(libjvm, "AsyncGetCallTrace"); + _getManagement = (JVM_GetManagement)dlsym(libjvm, "JVM_GetManagement"); Libraries *libraries = Libraries::instance(); libraries->updateSymbols(false); @@ -328,9 +333,6 @@ bool VM::initShared(JavaVM* vm) { return false; } - // Initialize VMStructs - VMStructs::init(lib); - // Mark thread entry points for all JVMs (critical for correct stack unwinding) lib->mark(isThreadEntry, MARK_THREAD_ENTRY); @@ -449,6 +451,15 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { return false; } + // Under Agent_OnLoad (attach == false), this is the first native entry point and + // VM_INIT has not fired yet, so VMStructs would otherwise stay uninitialized until + // VM::VMInit() runs -- but CodeHeap::available()/VMFlag::find() below are used + // synchronously in this function. Get VMStructs (and crash-protection signal + // handlers) ready now; VM::ready() is idempotent, so the later VM::VMInit() + // callback (attach == false) or the direct call from VM::initLibrary() (already + // run before a JNI-triggered attach == true call gets here) is a safe no-op. + ready(jvmti(), jni()); + if (!attach && hotspot_version() == 8 && OS::isLinux()) { // Workaround for JDK-8185348 char *func = (char *)lib->findSymbol( @@ -517,6 +528,8 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { NULL); if (hotspot_version() == 0 || !CodeHeap::available()) { + TEST_LOG("CompiledMethodLoad workaround: hotspot_version=%d CodeHeap::available=%d", + hotspot_version(), CodeHeap::available()); // Workaround for JDK-8173361: avoid CompiledMethodLoad events when possible _jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_COMPILED_METHOD_LOAD, NULL); @@ -524,6 +537,7 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { // DebugNonSafepoints is automatically enabled with CompiledMethodLoad, // otherwise we set the flag manually VMFlag* f = VMFlag::find("DebugNonSafepoints", {VMFlag::Type::Bool}); + TEST_LOG("DebugNonSafepoints flag %s", f != NULL ? "found" : "not found"); if (f != NULL && f->isDefault()) { f->set(1); } @@ -557,12 +571,26 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { return true; } -// Run late initialization when JVM is ready +// Run late initialization when JVM is ready. May be called more than once (from +// initProfilerBridge() directly, and later from the VMInit JVMTI callback, or from +// initLibrary() followed by a JNI-triggered attach) -- the VMStructs init below only +// ever runs once. void VM::ready(jvmtiEnv *jvmti, JNIEnv *jni) { - Profiler::check_JDK_8313796_workaround(); - Profiler::setupSignalHandlers(); - if (isHotspot()) { + // Hotspot specific + static bool init_signal = false; + static SpinLock lock; + ExclusiveLockGuard guard(&lock); + if (!init_signal) { + Profiler::check_JDK_8313796_workaround(); + Profiler::setupSignalHandlers(); + init_signal = true; + } + if (VM::isHotspot()) { JitWriteProtection jit(true); + CodeCache* lib = openJvmLibrary(); + assert(lib != nullptr && "JVM library must have been loaded"); + // Initialize VMStructs + VMStructs::init(lib); VMStructs::ready(); } } diff --git a/ddprof-lib/src/main/cpp/vmEntry.h b/ddprof-lib/src/main/cpp/vmEntry.h index 875e7c39b1..75725ef151 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.h +++ b/ddprof-lib/src/main/cpp/vmEntry.h @@ -148,6 +148,7 @@ class VM { static bool _can_sample_objects; static bool _can_intercept_binding; static bool _is_adaptive_gc_boundary_flag_set; + static CodeCache *_libjvm; // HotSpot JFR async stack-trace extension (optional, JDK 27+). // _request_stack_trace is atomic (RELEASE/ACQUIRE) because canRequestStackTrace() @@ -172,7 +173,12 @@ class VM { static CodeCache* openJvmLibrary(); public: - static void *_libjvm; + static inline CodeCache* libjvm() { + CodeCache* lib = __atomic_load_n(&_libjvm, __ATOMIC_ACQUIRE); + assert(lib != nullptr && "Out of order initialization sequence"); + return lib; + } + static AsyncGetCallTrace _asyncGetCallTrace; static JVM_GetManagement _getManagement; diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java index f7643e8fbc..c0003f03b4 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/LibraryLoader.java @@ -1,3 +1,7 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ package com.datadoghq.profiler; import java.io.IOException; @@ -107,13 +111,7 @@ private static Result loadLibrary(final String libraryLocation, String scratchDi if (state.get() == LoadingState.UNAVAILABLE) { return Result.UNAVAILABLE; } - Path libraryPath = libraryLocation != null ? Paths.get(libraryLocation) : null; - if (libraryPath == null) { - OperatingSystem os = OperatingSystem.current(); - String qualifier = (os == OperatingSystem.linux && os.isMusl()) ? "musl" : null; - - libraryPath = libraryFromClasspath(os, Arch.current(), qualifier, Paths.get(scratchDir != null ? scratchDir : System.getProperty("java.io.tmpdir"))); - } + Path libraryPath = libraryLocation != null ? Paths.get(libraryLocation) : resolveLibraryPath(scratchDir); System.load(libraryPath.toAbsolutePath().toString()); return Result.SUCCESS; } catch (Throwable t) { @@ -124,6 +122,28 @@ private static Result loadLibrary(final String libraryLocation, String scratchDi } } + /** + * Resolves the on-disk path of the bundled native library for the current OS/arch, extracting it + * from the classpath if necessary. Package-visible so tests can obtain a real file path (e.g. to + * pass via {@code -agentpath:}) without loading the library into the calling process. + * + * @param scratchDir The working scratch dir where to store the temp library file, or {@code null} + * to use {@code java.io.tmpdir} + * @return The library absolute path + * @throws IOException if an I/O error occurs while extracting the library + * @throws IllegalStateException if the resource is not found on the classpath + */ + static Path resolveLibraryPath(String scratchDir) throws IOException { + OperatingSystem os = OperatingSystem.current(); + String qualifier = (os == OperatingSystem.linux && os.isMusl()) ? "musl" : null; + return libraryFromClasspath( + os, + Arch.current(), + qualifier, + Paths.get(scratchDir != null ? scratchDir : System.getProperty("java.io.tmpdir"))) + .toAbsolutePath(); + } + /** * Locates a library on class-path (eg. in a JAR) and creates a publicly accessible temporary copy * of the library which can then be used by the application by its absolute path. diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java index acb4ce3e2a..f6b983f603 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java @@ -1,9 +1,15 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + package com.datadoghq.profiler; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import java.lang.management.ManagementFactory; +import java.nio.file.Path; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; @@ -42,6 +48,60 @@ void sanityInitailizationTest() throws Exception { assertFalse(initProfilerFound.get(), "initProfilerBridge found"); } + /** + * Regression test for the Agent_OnLoad (static {@code -agentpath:}) startup path: unlike the + * {@code library}/{@code profiler} targets used elsewhere in this file, which load the native + * library via {@code System.load} ({@code JNI_OnLoad} -> {@code VM::initLibrary}), an + * {@code -agentpath:} JVM argument makes {@code Agent_OnLoad} -> {@code VM::initProfilerBridge} + * the *first* native entry point invoked, with no prior call to {@code VM::ready()}. + * {@code initProfilerBridge()} looks up the {@code DebugNonSafepoints} VM flag via + * {@code VMFlag::find()}, gated on {@code CodeHeap::available()} -- both depend on + * {@code VMStructs} offsets/addresses populated by {@code VMStructs::init()}. If that + * initialization has been deferred to the asynchronous {@code VMInit} callback without also + * covering this call site, {@code CodeHeap::available()} reads {@code false} (its backing + * address is still unset) and the code takes the JDK-8173361 workaround path instead of ever + * calling {@code VMFlag::find()} -- silently disabling {@code CompiledMethodLoad} events and + * skipping the {@code DebugNonSafepoints} detection, even on a JVM where both should have + * succeeded. + */ + @Test + void agentOnLoadVMFlagDetectionTest() throws Exception { + String config = System.getProperty("ddprof_test.config"); + assumeTrue("debug".equals(config)); + + Path agentLib = LibraryLoader.resolveLibraryPath(System.getProperty("java.io.tmpdir")); + + AtomicReference workaroundLine = new AtomicReference<>(null); + AtomicReference flagLine = new AtomicReference<>(null); + + boolean rslt = launch("noop", + Collections.singletonList("-agentpath:" + agentLib.toAbsolutePath()), + null, + l -> { + if (l.contains("[TEST::INFO] CompiledMethodLoad workaround:")) { + workaroundLine.set(l); + return LineConsumerResult.STOP; + } + if (l.contains("[TEST::INFO] DebugNonSafepoints flag")) { + flagLine.set(l); + return LineConsumerResult.CONTINUE; + } + return LineConsumerResult.CONTINUE; + }, + null + ).inTime; + + assertTrue(rslt); + assertNull(workaroundLine.get(), + "VM::initProfilerBridge() took the JDK-8173361 CompiledMethodLoad workaround path (" + + workaroundLine.get() + ") instead of reaching the DebugNonSafepoints lookup -- " + + "CodeHeap::available() was false, meaning VMStructs was not initialized yet during " + + "Agent_OnLoad (startup-ordering regression)"); + assertNotNull(flagLine.get(), "expected DebugNonSafepoints flag lookup log line was not observed"); + assertTrue(flagLine.get().contains("flag found"), + "VMFlag::find(\"DebugNonSafepoints\") returned NULL during Agent_OnLoad: " + flagLine.get()); + } + @Test void jvmVersionTest() throws Exception { String config = System.getProperty("ddprof_test.config");