Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 13
[fault-injection] Startup should setup signal handlers before initializing VMStructs and protect VMFlag::name() call#674
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
a18740f4db801fe11fe6909bc99212432870c95622736748600316b13848d4094a667c042465b9d3fd616c9f1138da863cf8cdcf6fd7237dd258efaeefd7aea546a5c040565b3a5577d4fe0808d01cffe2f65d85a5ec8f8File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<T*>(const_cast<void*>(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 <typename T, bool safe> | ||
| 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)); | ||
| } | ||
Copilot marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| 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<const char*, true /* safe load */>(_flag_name_offset); | ||
zhengyu123 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| #endif // _HOTSPOT_VMSTRUCTS_INLINE_H | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1264,7 +1264,7 @@ Error Profiler::checkJvmCapabilities() { | ||
| } | ||
| } | ||
| if (!VMStructs::libjvm()->hasDebugSymbols() && !VM::isOpenJ9()) { | ||
| if (!VM::libjvm()->hasDebugSymbols()) { | ||
zhengyu123 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| Log::warn("Install JVM debug symbols to improve profile accuracy"); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
| } | ||
zhengyu123 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if ((void*)_asyncGetCallTrace == nullptr) { | ||
| return nullptr; | ||
| } | ||
zhengyu123 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| 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,13 +528,16 @@ 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); | ||
| } else { | ||
| // 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) { | ||
zhengyu123 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| 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(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
| } | ||
Copilot marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| static AsyncGetCallTrace _asyncGetCallTrace; | ||
| static JVM_GetManagement _getManagement; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 { | ||
zhengyu123 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| 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(); | ||
| } | ||
Copilot marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| /** | ||
| * 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. | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.