From ffd54c7bd2948f81718bce72665c880e94da3f42 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Mon, 3 Aug 2026 15:49:05 +0200 Subject: [PATCH 1/3] fix: avoid crash when libgcc_s is unavailable --- ddprof-lib/src/main/cpp/faultInjection.h | 30 +++++ ddprof-lib/src/main/cpp/profiler.cpp | 22 +++- ddprof-lib/src/main/cpp/profiler.h | 2 +- ddprof-lib/src/test/cpp/faultInjection_ut.cpp | 110 ++++++++++++++++++ ddprof-lib/src/test/cpp/jvmSupport_ut.cpp | 19 ++- 5 files changed, 175 insertions(+), 8 deletions(-) diff --git a/ddprof-lib/src/main/cpp/faultInjection.h b/ddprof-lib/src/main/cpp/faultInjection.h index e19f4bb1e9..a88dbe5c41 100644 --- a/ddprof-lib/src/main/cpp/faultInjection.h +++ b/ddprof-lib/src/main/cpp/faultInjection.h @@ -28,6 +28,12 @@ // pc = SafeAccess::load(INJECT_FAULT_ADDRESS_LIKELY((void**)fp)); // VMMethod* m = ((VMMethod**)INJECT_FAULT_ADDRESS_UNLIKELY(fp))[off]; // +// INJECT_FAULT_BOOL_* wraps the *result* of a call that already ran for +// real, forcing it to report `false` so a caller's failure-handling path +// (not its memory-safety recovery path) gets exercised, e.g.: +// +// return INJECT_FAULT_BOOL_LIKELY(dlopen(name, flags) != nullptr); +// // The three tiers name their firing frequency: RARE 0.01%, UNLIKELY 0.1%, // LIKELY 1%. See faultInjection.cpp for the poison-address and PRNG details. @@ -76,6 +82,19 @@ inline T injectAddress(T ptr, u64 threshold, const char* fn) { } return ptr; } + +// Returns orig unchanged, or `faulty` when the tier fires. Unlike +// injectAddress() (which fakes an input about to be dereferenced), this fakes +// the *outcome* of a call that already ran for real — e.g. making a +// successful dlopen() appear to have failed, to exercise a caller's error +// path without needing the library to actually be absent. +template +inline T injectValue(T orig, T faulty, u64 threshold, const char* fn) { + if (__builtin_expect(shouldFire(threshold, fn), 0)) { + return faulty; + } + return orig; +} } // namespace faultinj #define INJECT_FAULT_ADDRESS_RARE(ptr) \ @@ -85,6 +104,13 @@ inline T injectAddress(T ptr, u64 threshold, const char* fn) { #define INJECT_FAULT_ADDRESS_LIKELY(ptr) \ ::faultinj::injectAddress((ptr), ::faultinj::PROB_LIKELY, __func__) +#define INJECT_FAULT_BOOL_RARE(v) \ + ::faultinj::injectValue((v), false, ::faultinj::PROB_RARE, __func__) +#define INJECT_FAULT_BOOL_UNLIKELY(v) \ + ::faultinj::injectValue((v), false, ::faultinj::PROB_UNLIKELY, __func__) +#define INJECT_FAULT_BOOL_LIKELY(v) \ + ::faultinj::injectValue((v), false, ::faultinj::PROB_LIKELY, __func__) + #else // __FAULT_INJECTION__ not defined — strict identity, zero cost. #define INJECT_FAULT_ADDRESS_RARE(ptr) (ptr) @@ -99,6 +125,10 @@ inline T injectAddress(T ptr, u64 threshold, const char* fn) { #define INJECT_FAULT_LONG_UNLIKELY(v) (v) #define INJECT_FAULT_LONG_LIKELY(v) (v) +#define INJECT_FAULT_BOOL_RARE(v) (v) +#define INJECT_FAULT_BOOL_UNLIKELY(v) (v) +#define INJECT_FAULT_BOOL_LIKELY(v) (v) + #endif // __FAULT_INJECTION__ #endif // _FAULT_INJECTION_H diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 7c631b002b..32ff6e21c2 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -843,7 +843,7 @@ void Profiler::writeHeapUsage(long value, bool live) { _locks[lock_index].unlock(); } -void Profiler::prewarmUnwinder() { +bool Profiler::prewarmUnwinder() { #ifdef __linux__ // J9 on aarch64 (and other JVMs) lazily loads libgcc_s.so.1 from its DWARF // unwinder during stack walks. When that happens inside a signal handler @@ -864,7 +864,13 @@ void Profiler::prewarmUnwinder() { // dlopen by SONAME is the only mechanism that works under static-libgcc. // libgcc_s.so.1 has been the stable SONAME since 2002; a bump would // constitute a glibc/GCC C++ ABI break and is treated as a fixed contract. - (void)dlopen("libgcc_s.so.1", RTLD_LAZY | RTLD_GLOBAL); + // + // INJECT_FAULT_BOOL_LIKELY lets fault-injection builds force this to + // report failure without the library actually being absent, so + // checkState()'s "Missing libgcc_s.so" path can be exercised in CI. + return INJECT_FAULT_BOOL_LIKELY(dlopen("libgcc_s.so.1", RTLD_LAZY | RTLD_GLOBAL) != nullptr); +#else + return true; #endif } @@ -1291,6 +1297,13 @@ Error Profiler::checkState() { if (s == ERROR) { return Error("Profiler encountered fatal error"); } else if (s == NEW) { + // Force libgcc_s to load now (idempotent dlopen) so the JVM's DWARF + // unwinder cannot lazy-load it later from signal context. + if (!prewarmUnwinder()) { + _state.store(ERROR, std::memory_order_release); + return Error("Missing libgcc_s.so.1"); + } + // Make sure JVMSupport is initialized // In theory, it should be initialized in JVMTI::VMInit() callback, // but the callback arrives too late, after this method is called. @@ -1306,6 +1319,7 @@ Error Profiler::checkState() { Error Profiler::init() { MutexLocker ml(_state_lock); + State s = state(); if (s == ERROR) { return Error("Profiler encountered fatal error"); @@ -1336,10 +1350,6 @@ Error Profiler::start(Arguments &args, bool reset) { return error; } - // Force libgcc_s to load now (idempotent dlopen) so the JVM's DWARF - // unwinder cannot lazy-load it later from signal context. - prewarmUnwinder(); - error = checkJvmCapabilities(); if (error) { return error; diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index 946c4c1e6d..3e4ffffced 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -152,7 +152,7 @@ class alignas(alignof(SpinLock)) Profiler { void **_dlopen_entry; static void *dlopen_hook(const char *filename, int flags); void switchLibraryTrap(bool enable); - static void prewarmUnwinder(); + static bool prewarmUnwinder(); void disableEngines(); diff --git a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp index d1d25c250b..e3d94f8e14 100644 --- a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp +++ b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp @@ -10,10 +10,14 @@ #include #include +#include + #include "faultInjection.h" #include "safeAccess.h" #include "os.h" +#include "profiler.h" #include "threadLocalData.h" +#include "vmEntry.h" #include "hotspot/hotspotSupport.h" #include "../../main/cpp/gtest_crash_handler.h" @@ -47,6 +51,18 @@ TEST(FaultInjectionTest, DisabledValueMacrosAreIdentity) { EXPECT_EQ(INJECT_FAULT_LONG_RARE(l), l); EXPECT_EQ(INJECT_FAULT_LONG_UNLIKELY(l), l); EXPECT_EQ(INJECT_FAULT_LONG_LIKELY(l), l); + + // BOOL must be identity both ways -- an accidental non-identity expansion + // (e.g. always forcing false) would otherwise only show up as a silent + // behavioural change in a production build, never a compile error. + bool t = true; + bool f = false; + EXPECT_EQ(INJECT_FAULT_BOOL_RARE(t), t); + EXPECT_EQ(INJECT_FAULT_BOOL_UNLIKELY(t), t); + EXPECT_EQ(INJECT_FAULT_BOOL_LIKELY(t), t); + EXPECT_EQ(INJECT_FAULT_BOOL_RARE(f), f); + EXPECT_EQ(INJECT_FAULT_BOOL_UNLIKELY(f), f); + EXPECT_EQ(INJECT_FAULT_BOOL_LIKELY(f), f); } #else // __FAULT_INJECTION__ enabled (built under -PenableFaultInjection) @@ -180,4 +196,98 @@ TEST_F(FaultInjectionTest, WalkVmSigsetjmpRecoversFromInjectedFault) { SUCCEED(); } +// Friend of Profiler (see profiler.h) — lets this test force the internal +// state to a known value so checkState() can be exercised deterministically +// (matches the pattern in jvmSupport_ut.cpp). +class ProfilerTestAccessor { +public: + static void setState(Profiler* p, State s) { + p->_state.store(s, std::memory_order_release); + } + static State getState(Profiler* p) { + return p->_state.load(std::memory_order_acquire); + } +}; + +// Friend of VM (see vmEntry.h) — lets this test install a mock jvmtiEnv, the +// same seam jvmSupport_ut.cpp uses. checkState() (below) checks +// prewarmUnwinder() before JVMSupport::initialize(), so the injected-failure +// path never touches this at all; it exists only so the ~99% non-injected +// iterations, which do fall through into JVMSupport::initialize(), fail +// gracefully instead of crashing on a null VM::_jvmti in this no-live-JVM +// binary. Unlike a JVMThread-level fake (which would permanently flip +// JVMThread::isInitialized() for the rest of the process, since ThreadLocal +// pthread keys are never invalidated), this is a plain pointer swap that +// ScopedJvmtiMock restores on scope exit -- no state leaks into later tests. +class VMTestAccessor { +public: + static jvmtiEnv* getJvmti() { return VM::_jvmti; } + static void setJvmti(jvmtiEnv* env) { VM::_jvmti = env; } +}; + +static jvmtiError JNICALL mock_GetCurrentThread_fails(jvmtiEnv*, jthread*) { + return JVMTI_ERROR_INTERNAL; +} + +class ScopedJvmtiMock { +public: + ScopedJvmtiMock() : _orig(VMTestAccessor::getJvmti()) { + _tbl.GetCurrentThread = &mock_GetCurrentThread_fails; + _env.functions = &_tbl; + VMTestAccessor::setJvmti(&_env); + } + ~ScopedJvmtiMock() { VMTestAccessor::setJvmti(_orig); } + +private: + jvmtiInterface_1_ _tbl{}; + _jvmtiEnv _env{}; + jvmtiEnv* _orig; +}; + +// (d) Value-injection path: PROF-15395 fixed Profiler::checkState() (shared by +// start()/check(), and therefore also reached by the -agentpath auto-start +// path) to fail cleanly instead of crashing later when libgcc_s.so.1 can't be +// loaded. libgcc_s.so.1 is always present in this test environment, so +// INJECT_FAULT_BOOL_LIKELY on prewarmUnwinder()'s return value is what makes +// that failure path reachable here: the real dlopen() still runs and +// succeeds, but the caller is deterministically told it failed. +TEST_F(FaultInjectionTest, CheckStateSurfacesInjectedPrewarmUnwinderFailure) { +#ifdef __linux__ + Profiler* p = Profiler::instance(); + // checkState() checks prewarmUnwinder() before JVMSupport::initialize(), so + // reaching the injected-failure path below needs nothing but the NEW state. + ScopedJvmtiMock jvmti_mock; + ProfilerTestAccessor::setState(p, NEW); + ProfiledThread::current()->setFiRng(0x5EED5EED5EED5EEDULL); + + bool sawInjectedFailure = false; + bool sawNonInjectedPrewarm = false; + // shouldFire() mixes the fixed RNG seed above with an ASLR-dependent + // per-call-site address, so which outcome the *first* call produces is not + // deterministic run to run -- the injected failure can land before a + // non-injected call is observed. Keep iterating (and un-latching the ERROR + // state that every outcome here leaves behind) until both have been seen. + for (int i = 0; i < 5000 && !(sawInjectedFailure && sawNonInjectedPrewarm); i++) { + Error error = p->checkState(); + ASSERT_TRUE((bool)error) << "checkState() must fail here: either the " + "injected prewarmUnwinder() failure or the " + "mocked JVMSupport::initialize() failure"; + if (std::strcmp(error.message(), "Missing libgcc_s.so.1") == 0) { + sawInjectedFailure = true; + } else { + // prewarmUnwinder() succeeded (non-injected, ~99% of calls) and fell + // through to the mocked JVMSupport::initialize() failure instead. + EXPECT_STREQ("Profiler encountered fatal error", error.message()); + sawNonInjectedPrewarm = true; + } + ProfilerTestAccessor::setState(p, NEW); + } + + EXPECT_TRUE(sawInjectedFailure) + << "expected at least one injected prewarmUnwinder() failure within 5000 tries"; + EXPECT_TRUE(sawNonInjectedPrewarm) + << "expected at least one non-injected prewarmUnwinder() success within 5000 tries"; +#endif // __linux__ +} + #endif // __FAULT_INJECTION__ diff --git a/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp index 6557567ebc..c203d296e3 100644 --- a/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp +++ b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp @@ -4,6 +4,7 @@ */ #include +#include #include "jvmSupport.h" #include "jvmThread.h" #include "vmEntry.h" @@ -111,7 +112,23 @@ TEST_F(JvmSupportInitFailureTest, JVMSupportInitializeFailsWhenJVMThreadFails) { TEST_F(JvmSupportInitFailureTest, CheckStateBlocksOnInitFailureAndLatchesError) { Profiler* p = Profiler::instance(); - Error error = p->checkState(); + // Under -PenableFaultInjection, checkState() checks prewarmUnwinder() + // before JVMSupport::initialize() (see profiler.cpp), so an injected + // fault could occasionally surface "Missing libgcc_s.so" here instead of + // the JVMSupport::initialize() failure this test targets. Retry past any + // such injected failure. If it persists across retries, libgcc_s is likely + // genuinely absent on this host and this test cannot exercise the intended path. + Error error = Error::OK; + for (int i = 0; i < 100; i++) { + error = p->checkState(); + if (!error || std::strcmp(error.message(), "Missing libgcc_s.so.1") != 0) { + break; + } + ProfilerTestAccessor::setState(p, NEW); + } + if (error && std::strcmp(error.message(), "Missing libgcc_s.so.1") == 0) { + GTEST_SKIP() << "libgcc_s.so.1 is missing on this host; cannot exercise JVMSupport::initialize() failure path"; + } bool has_error = (bool)error; EXPECT_TRUE(has_error); From e884210f6d47ba441dbec3b151f2e045fb5ded9c Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Mon, 3 Aug 2026 15:54:53 +0200 Subject: [PATCH 2/3] fix: avoid retaining invalid VM method pointers --- ddprof-lib/src/main/cpp/flightRecorder.cpp | 8 +++ .../src/main/cpp/hotspot/hotspotSupport.cpp | 25 +++++---- .../src/main/cpp/hotspot/hotspotSupport.h | 8 +++ .../src/test/cpp/hotspotMethodId_ut.cpp | 55 +++++++++++++++++++ 4 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 95875ed016..6064e594cc 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -560,6 +560,14 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) { jint bci = frame.bci; jmethodID method_id = frame.method_id; + // HotSpot's VM stack walker uses this sentinel when it could not validate a + // Method*. It is not a JNI/JVMTI jmethodID and must never reach + // fillJavaMethodInfo(). Keep the frame structurally intact, but serialize it + // as the shared unknown method. + if (VM::isHotspot() && method_id == JMETHODID_NOT_WALKABLE) { + method_id = nullptr; + } + // Resolve native method if (FrameType::isRawPointer(bci)) { method_id = JVMSupport::resolve(frame.method); diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 2b319b1009..ae11bdb425 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -191,9 +191,14 @@ static void fillFrameRaw(ASGCT_CallFrame& frame, FrameTypeId type, int bci, cons frame.method = static_cast(method); } -static void fillFrame(ASGCT_CallFrame& frame, FrameTypeId type, int bci, jmethodID method_id, const VMMethod* method) { - // Pack JMETHODID_NOT_WALKABLE frame as raw pointer frame, so it can not resolved into nullptr to shared code. - if (method_id != nullptr && method_id != JMETHODID_NOT_WALKABLE) { +void HotspotSupport::fillJavaFrame(ASGCT_CallFrame& frame, FrameTypeId type, int bci, + jmethodID method_id, const VMMethod* method) { + if (method_id == JMETHODID_NOT_WALKABLE) { + // The Method* failed validation while walking. Preserve only the sentinel; + // retaining the Method* would defer a dereference of that invalid metadata + // until the dump thread resolves the frame. + fillFrame(frame, type, bci, method_id); + } else if (method_id != nullptr) { fillFrame(frame, type, bci, method_id); } else { assert(method != nullptr); @@ -513,7 +518,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex const char* bytecode_start = method->bytecode(); const char* bcp = ((const char**)fp)[bcp_offset]; int bci = bytecode_start == NULL || bcp < bytecode_start ? 0 : bcp - bytecode_start; - fillFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); sp = ((uintptr_t*)fp)[InterpreterFrame::sender_sp_offset]; pc = stripPointer(((void**)fp)[FRAME_PC_SLOT]); fp = *(uintptr_t*)INJECT_FAULT_ADDRESS_UNLIKELY(fp); @@ -526,7 +531,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex jmethodID method_id = getMethodId(method); if (method_id != JMETHODID_NOT_WALKABLE) { Counters::increment(WALKVM_JAVA_FRAME_OK); - fillFrame(frames[depth++], FRAME_INTERPRETED, 0, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_INTERPRETED, 0, method_id, method); if (is_plausible_interpreter_frame) { uintptr_t* fp_addr = (uintptr_t*)INJECT_FAULT_ADDRESS_UNLIKELY(fp); pc = stripPointer(((void**)fp_addr)[FRAME_PC_SLOT]); @@ -565,7 +570,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex VMMethod* method = nm->method(); jmethodID method_id = method->id(); - fillFrame(frames[depth++], type, 0, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], type, 0, method_id, method); if (nm->isFrameCompleteAt(pc)) { if (depth == 1 && frame.unwindEpilogue(nm, (uintptr_t&)pc, sp, fp)) { @@ -584,7 +589,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex } VMMethod* method = scope.method(); jmethodID method_id = method->id(); - fillFrame(frames[depth++], type, scope.bci(), method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], type, scope.bci(), method_id, method); } while (scope_offset > 0 && depth < max_depth); } @@ -704,7 +709,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex if (method != nullptr) { jmethodID method_id = method->id(); if (method_id != JMETHODID_NOT_WALKABLE) { - fillFrame(frames[depth++], FRAME_JIT_COMPILED, 0, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_JIT_COMPILED, 0, method_id, method); } } } else if (resolution.mark == MARK_THREAD_ENTRY) { @@ -764,7 +769,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex const char* bytecode_start = method->bytecode(); const char* bcp = ((const char**)recovery_fp)[bcp_offset]; int bci = bytecode_start == NULL || bcp < bytecode_start ? 0 : bcp - bytecode_start; - fillFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); sp = ((uintptr_t*)recovery_fp)[InterpreterFrame::sender_sp_offset]; pc = stripPointer(((void**)recovery_fp)[FRAME_PC_SLOT]); fp = *(uintptr_t*)recovery_fp; @@ -920,7 +925,7 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex const char* bytecode_start = method->bytecode(); const char* bcp = ((const char**)anchor_fp)[bcp_offset]; int bci = bytecode_start == NULL || bcp < bytecode_start ? 0 : bcp - bytecode_start; - fillFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); + HotspotSupport::fillJavaFrame(frames[depth++], FRAME_INTERPRETED, bci, method_id, method); sp = ((uintptr_t*)anchor_fp)[InterpreterFrame::sender_sp_offset]; pc = stripPointer(((void**)anchor_fp)[FRAME_PC_SLOT]); fp = *(uintptr_t*)anchor_fp; diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h index e905d55c4a..7d5a6ea805 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.h @@ -9,6 +9,7 @@ #include "hotspot/hotspotStackFrame.h" #include "hotspot/jitCodeCache.h" +#include "frame.h" #include "stackFrame.h" #include "stackWalker.h" @@ -16,6 +17,7 @@ #include class ProfiledThread; +class VMMethod; class HotspotSupport { friend class JVMSupport; @@ -59,6 +61,12 @@ class HotspotSupport { // Resolve a method to a jmethodID at dumping time static jmethodID resolve(const void* method); + + // Store a Java frame captured from HotSpot metadata. A null jmethodID + // retains the raw Method* fallback; the rejected-ID sentinel is stored as + // an ordinary frame so it can be resolved to the shared unknown method. + static void fillJavaFrame(ASGCT_CallFrame& frame, FrameTypeId type, int bci, + jmethodID method_id, const VMMethod* method); }; #endif // _HOTSPOT_HOTSPOTSUPPORT_H diff --git a/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp b/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp new file mode 100644 index 0000000000..c7eff90687 --- /dev/null +++ b/ddprof-lib/src/test/cpp/hotspotMethodId_ut.cpp @@ -0,0 +1,55 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +#include "../../main/cpp/flightRecorder.h" +#include "../../main/cpp/hotspot/hotspotSupport.h" +#include "../../main/cpp/hotspot/vmStructs.h" + +// Test-only friend accessor for VM internals. It exists solely so this test +// can exercise HotSpot's rejected-jmethodID handling. +class VMTestAccessor { +public: + static bool getHotspot() { return VM::_hotspot; } + static void setHotspot(bool value) { VM::_hotspot = value; } +}; + +class HotspotMethodIdVMHotspotGuard { +private: + bool _saved; + +public: + HotspotMethodIdVMHotspotGuard() : _saved(VMTestAccessor::getHotspot()) { + VMTestAccessor::setHotspot(true); + } + + ~HotspotMethodIdVMHotspotGuard() { + VMTestAccessor::setHotspot(_saved); + } +}; + +TEST(HotspotMethodIdTest, RejectedMethodIdStaysNonRawAndResolvesToUnknown) { + HotspotMethodIdVMHotspotGuard hotspot; + ASGCT_CallFrame frame{}; + + // The pointer is intentionally invalid. A rejected jmethodID must retain + // only its sentinel and must not preserve this Method* for dump-time use. + HotspotSupport::fillJavaFrame(frame, FRAME_JIT_COMPILED, 17, + JMETHODID_NOT_WALKABLE, + reinterpret_cast(1)); + + EXPECT_FALSE(FrameType::isRawPointer(frame.bci)); + EXPECT_EQ(frame.method_id, JMETHODID_NOT_WALKABLE); + + StringDictionary classes; + MethodMap methods; + Lookup lookup(nullptr, &methods, &classes); + MethodInfo* info = lookup.resolveMethod(frame); + + ASSERT_NE(info, nullptr); + EXPECT_EQ(info->_type, FRAME_NATIVE); + EXPECT_EQ(methods.size(), 1U); +} From d0ba1b5b421c744073e1192da6a8dd4293a64353 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Mon, 3 Aug 2026 17:35:19 +0200 Subject: [PATCH 3/3] fix: omit fault injection from libgcc backport --- ddprof-lib/src/main/cpp/faultInjection.h | 30 ----- ddprof-lib/src/main/cpp/profiler.cpp | 6 +- ddprof-lib/src/test/cpp/faultInjection_ut.cpp | 110 ------------------ ddprof-lib/src/test/cpp/jvmSupport_ut.cpp | 19 +-- 4 files changed, 2 insertions(+), 163 deletions(-) diff --git a/ddprof-lib/src/main/cpp/faultInjection.h b/ddprof-lib/src/main/cpp/faultInjection.h index a88dbe5c41..e19f4bb1e9 100644 --- a/ddprof-lib/src/main/cpp/faultInjection.h +++ b/ddprof-lib/src/main/cpp/faultInjection.h @@ -28,12 +28,6 @@ // pc = SafeAccess::load(INJECT_FAULT_ADDRESS_LIKELY((void**)fp)); // VMMethod* m = ((VMMethod**)INJECT_FAULT_ADDRESS_UNLIKELY(fp))[off]; // -// INJECT_FAULT_BOOL_* wraps the *result* of a call that already ran for -// real, forcing it to report `false` so a caller's failure-handling path -// (not its memory-safety recovery path) gets exercised, e.g.: -// -// return INJECT_FAULT_BOOL_LIKELY(dlopen(name, flags) != nullptr); -// // The three tiers name their firing frequency: RARE 0.01%, UNLIKELY 0.1%, // LIKELY 1%. See faultInjection.cpp for the poison-address and PRNG details. @@ -82,19 +76,6 @@ inline T injectAddress(T ptr, u64 threshold, const char* fn) { } return ptr; } - -// Returns orig unchanged, or `faulty` when the tier fires. Unlike -// injectAddress() (which fakes an input about to be dereferenced), this fakes -// the *outcome* of a call that already ran for real — e.g. making a -// successful dlopen() appear to have failed, to exercise a caller's error -// path without needing the library to actually be absent. -template -inline T injectValue(T orig, T faulty, u64 threshold, const char* fn) { - if (__builtin_expect(shouldFire(threshold, fn), 0)) { - return faulty; - } - return orig; -} } // namespace faultinj #define INJECT_FAULT_ADDRESS_RARE(ptr) \ @@ -104,13 +85,6 @@ inline T injectValue(T orig, T faulty, u64 threshold, const char* fn) { #define INJECT_FAULT_ADDRESS_LIKELY(ptr) \ ::faultinj::injectAddress((ptr), ::faultinj::PROB_LIKELY, __func__) -#define INJECT_FAULT_BOOL_RARE(v) \ - ::faultinj::injectValue((v), false, ::faultinj::PROB_RARE, __func__) -#define INJECT_FAULT_BOOL_UNLIKELY(v) \ - ::faultinj::injectValue((v), false, ::faultinj::PROB_UNLIKELY, __func__) -#define INJECT_FAULT_BOOL_LIKELY(v) \ - ::faultinj::injectValue((v), false, ::faultinj::PROB_LIKELY, __func__) - #else // __FAULT_INJECTION__ not defined — strict identity, zero cost. #define INJECT_FAULT_ADDRESS_RARE(ptr) (ptr) @@ -125,10 +99,6 @@ inline T injectValue(T orig, T faulty, u64 threshold, const char* fn) { #define INJECT_FAULT_LONG_UNLIKELY(v) (v) #define INJECT_FAULT_LONG_LIKELY(v) (v) -#define INJECT_FAULT_BOOL_RARE(v) (v) -#define INJECT_FAULT_BOOL_UNLIKELY(v) (v) -#define INJECT_FAULT_BOOL_LIKELY(v) (v) - #endif // __FAULT_INJECTION__ #endif // _FAULT_INJECTION_H diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 32ff6e21c2..cbc579ce68 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -864,11 +864,7 @@ bool Profiler::prewarmUnwinder() { // dlopen by SONAME is the only mechanism that works under static-libgcc. // libgcc_s.so.1 has been the stable SONAME since 2002; a bump would // constitute a glibc/GCC C++ ABI break and is treated as a fixed contract. - // - // INJECT_FAULT_BOOL_LIKELY lets fault-injection builds force this to - // report failure without the library actually being absent, so - // checkState()'s "Missing libgcc_s.so" path can be exercised in CI. - return INJECT_FAULT_BOOL_LIKELY(dlopen("libgcc_s.so.1", RTLD_LAZY | RTLD_GLOBAL) != nullptr); + return dlopen("libgcc_s.so.1", RTLD_LAZY | RTLD_GLOBAL) != nullptr; #else return true; #endif diff --git a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp index e3d94f8e14..d1d25c250b 100644 --- a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp +++ b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp @@ -10,14 +10,10 @@ #include #include -#include - #include "faultInjection.h" #include "safeAccess.h" #include "os.h" -#include "profiler.h" #include "threadLocalData.h" -#include "vmEntry.h" #include "hotspot/hotspotSupport.h" #include "../../main/cpp/gtest_crash_handler.h" @@ -51,18 +47,6 @@ TEST(FaultInjectionTest, DisabledValueMacrosAreIdentity) { EXPECT_EQ(INJECT_FAULT_LONG_RARE(l), l); EXPECT_EQ(INJECT_FAULT_LONG_UNLIKELY(l), l); EXPECT_EQ(INJECT_FAULT_LONG_LIKELY(l), l); - - // BOOL must be identity both ways -- an accidental non-identity expansion - // (e.g. always forcing false) would otherwise only show up as a silent - // behavioural change in a production build, never a compile error. - bool t = true; - bool f = false; - EXPECT_EQ(INJECT_FAULT_BOOL_RARE(t), t); - EXPECT_EQ(INJECT_FAULT_BOOL_UNLIKELY(t), t); - EXPECT_EQ(INJECT_FAULT_BOOL_LIKELY(t), t); - EXPECT_EQ(INJECT_FAULT_BOOL_RARE(f), f); - EXPECT_EQ(INJECT_FAULT_BOOL_UNLIKELY(f), f); - EXPECT_EQ(INJECT_FAULT_BOOL_LIKELY(f), f); } #else // __FAULT_INJECTION__ enabled (built under -PenableFaultInjection) @@ -196,98 +180,4 @@ TEST_F(FaultInjectionTest, WalkVmSigsetjmpRecoversFromInjectedFault) { SUCCEED(); } -// Friend of Profiler (see profiler.h) — lets this test force the internal -// state to a known value so checkState() can be exercised deterministically -// (matches the pattern in jvmSupport_ut.cpp). -class ProfilerTestAccessor { -public: - static void setState(Profiler* p, State s) { - p->_state.store(s, std::memory_order_release); - } - static State getState(Profiler* p) { - return p->_state.load(std::memory_order_acquire); - } -}; - -// Friend of VM (see vmEntry.h) — lets this test install a mock jvmtiEnv, the -// same seam jvmSupport_ut.cpp uses. checkState() (below) checks -// prewarmUnwinder() before JVMSupport::initialize(), so the injected-failure -// path never touches this at all; it exists only so the ~99% non-injected -// iterations, which do fall through into JVMSupport::initialize(), fail -// gracefully instead of crashing on a null VM::_jvmti in this no-live-JVM -// binary. Unlike a JVMThread-level fake (which would permanently flip -// JVMThread::isInitialized() for the rest of the process, since ThreadLocal -// pthread keys are never invalidated), this is a plain pointer swap that -// ScopedJvmtiMock restores on scope exit -- no state leaks into later tests. -class VMTestAccessor { -public: - static jvmtiEnv* getJvmti() { return VM::_jvmti; } - static void setJvmti(jvmtiEnv* env) { VM::_jvmti = env; } -}; - -static jvmtiError JNICALL mock_GetCurrentThread_fails(jvmtiEnv*, jthread*) { - return JVMTI_ERROR_INTERNAL; -} - -class ScopedJvmtiMock { -public: - ScopedJvmtiMock() : _orig(VMTestAccessor::getJvmti()) { - _tbl.GetCurrentThread = &mock_GetCurrentThread_fails; - _env.functions = &_tbl; - VMTestAccessor::setJvmti(&_env); - } - ~ScopedJvmtiMock() { VMTestAccessor::setJvmti(_orig); } - -private: - jvmtiInterface_1_ _tbl{}; - _jvmtiEnv _env{}; - jvmtiEnv* _orig; -}; - -// (d) Value-injection path: PROF-15395 fixed Profiler::checkState() (shared by -// start()/check(), and therefore also reached by the -agentpath auto-start -// path) to fail cleanly instead of crashing later when libgcc_s.so.1 can't be -// loaded. libgcc_s.so.1 is always present in this test environment, so -// INJECT_FAULT_BOOL_LIKELY on prewarmUnwinder()'s return value is what makes -// that failure path reachable here: the real dlopen() still runs and -// succeeds, but the caller is deterministically told it failed. -TEST_F(FaultInjectionTest, CheckStateSurfacesInjectedPrewarmUnwinderFailure) { -#ifdef __linux__ - Profiler* p = Profiler::instance(); - // checkState() checks prewarmUnwinder() before JVMSupport::initialize(), so - // reaching the injected-failure path below needs nothing but the NEW state. - ScopedJvmtiMock jvmti_mock; - ProfilerTestAccessor::setState(p, NEW); - ProfiledThread::current()->setFiRng(0x5EED5EED5EED5EEDULL); - - bool sawInjectedFailure = false; - bool sawNonInjectedPrewarm = false; - // shouldFire() mixes the fixed RNG seed above with an ASLR-dependent - // per-call-site address, so which outcome the *first* call produces is not - // deterministic run to run -- the injected failure can land before a - // non-injected call is observed. Keep iterating (and un-latching the ERROR - // state that every outcome here leaves behind) until both have been seen. - for (int i = 0; i < 5000 && !(sawInjectedFailure && sawNonInjectedPrewarm); i++) { - Error error = p->checkState(); - ASSERT_TRUE((bool)error) << "checkState() must fail here: either the " - "injected prewarmUnwinder() failure or the " - "mocked JVMSupport::initialize() failure"; - if (std::strcmp(error.message(), "Missing libgcc_s.so.1") == 0) { - sawInjectedFailure = true; - } else { - // prewarmUnwinder() succeeded (non-injected, ~99% of calls) and fell - // through to the mocked JVMSupport::initialize() failure instead. - EXPECT_STREQ("Profiler encountered fatal error", error.message()); - sawNonInjectedPrewarm = true; - } - ProfilerTestAccessor::setState(p, NEW); - } - - EXPECT_TRUE(sawInjectedFailure) - << "expected at least one injected prewarmUnwinder() failure within 5000 tries"; - EXPECT_TRUE(sawNonInjectedPrewarm) - << "expected at least one non-injected prewarmUnwinder() success within 5000 tries"; -#endif // __linux__ -} - #endif // __FAULT_INJECTION__ diff --git a/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp index c203d296e3..6557567ebc 100644 --- a/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp +++ b/ddprof-lib/src/test/cpp/jvmSupport_ut.cpp @@ -4,7 +4,6 @@ */ #include -#include #include "jvmSupport.h" #include "jvmThread.h" #include "vmEntry.h" @@ -112,23 +111,7 @@ TEST_F(JvmSupportInitFailureTest, JVMSupportInitializeFailsWhenJVMThreadFails) { TEST_F(JvmSupportInitFailureTest, CheckStateBlocksOnInitFailureAndLatchesError) { Profiler* p = Profiler::instance(); - // Under -PenableFaultInjection, checkState() checks prewarmUnwinder() - // before JVMSupport::initialize() (see profiler.cpp), so an injected - // fault could occasionally surface "Missing libgcc_s.so" here instead of - // the JVMSupport::initialize() failure this test targets. Retry past any - // such injected failure. If it persists across retries, libgcc_s is likely - // genuinely absent on this host and this test cannot exercise the intended path. - Error error = Error::OK; - for (int i = 0; i < 100; i++) { - error = p->checkState(); - if (!error || std::strcmp(error.message(), "Missing libgcc_s.so.1") != 0) { - break; - } - ProfilerTestAccessor::setState(p, NEW); - } - if (error && std::strcmp(error.message(), "Missing libgcc_s.so.1") == 0) { - GTEST_SKIP() << "libgcc_s.so.1 is missing on this host; cannot exercise JVMSupport::initialize() failure path"; - } + Error error = p->checkState(); bool has_error = (bool)error; EXPECT_TRUE(has_error);