diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 60aa659d8..c45a61c52 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -845,12 +845,37 @@ void Profiler::writeHeapUsage(long value, bool live) { 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 - // frame, our dlopen_hook fires from signal context and tries to refresh the - // library list — Mutex::lock and malloc on a signal stack. By forcing the - // load here, before any signal handler is installed, subsequent calls find - // libgcc_s already mapped and the lazy-load path never runs. + // Force libgcc_s.so.1 to load now and report whether that succeeded. This + // closes two separate lazy-load landmines that happen to share the same + // library and the same fix: + // + // 1. J9's DWARF unwinder can lazily dlopen libgcc_s.so.1 while walking a + // stack (e.g. on aarch64). Our sampling stack walks run from a signal + // handler, so that dlopen call can fire our PLT-patched dlopen_hook() + // from signal context, where it does Mutex::lock/malloc — both + // AS-unsafe. Other JVM-internal lazy loads (observed under Graal on + // aarch64) can trigger the same path. Forcing the load here, before any + // signal handler is installed, means the JVM's later resolve finds + // libgcc_s already mapped and this lazy-load path never runs. + // + // 2. Separately, glibc's pthread_exit()/pthread_cancel() call + // __pthread_unwind(), which invokes _Unwind_ForcedUnwind() + // unconditionally. glibc does not hard-link libgcc_s into + // libc/libpthread; it lazily resolves that symbol via its own private + // __libc_dlopen(LIBGCC_S_SO) the first time a thread exits or is + // cancelled — including our own worker/sampler threads, whose + // start_routine_wrapper (libraryPatcher_linux.cpp) calls pthread_exit(). + // If that lazy load fails, glibc itself calls __libc_fatal("libgcc_s. + // so.1 must be installed for pthread_exit to work\n"), aborting the + // whole process — observed in production on hardened/distroless images + // that ship without libgcc_s.so.1. + // __libc_dlopen is glibc's private loader, not the public PLT-visible + // dlopen, so unlike (1) this does not route through dlopen_hook(). + // + // Whether a load failure here is actually fatal is decided by the caller + // (see checkState()): only landmine (2) is glibc-specific, so a musl host + // missing libgcc_s.so.1 is not at risk and should not fail profiler + // startup over it. // // The handle is intentionally leaked: keeping the refcount > 0 prevents the // library from being unmapped for the remainder of the process lifetime. @@ -1325,9 +1350,12 @@ 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()) { + // prewarmUnwinder() closes a glibc-specific pthread_exit/pthread_cancel + // landmine (see its comment) by force-loading libgcc_s.so.1 up front. + // A failure only matters on glibc, where that landmine is real; musl + // never hits the code path that needs libgcc_s, so a missing library + // there is not a reason to fail profiler startup. + if (!prewarmUnwinder() && !OS::isMusl()) { _state.store(ERROR, std::memory_order_release); return Error("Missing libgcc_s.so.1"); } diff --git a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp index f0a49354e..46c19b953 100644 --- a/ddprof-lib/src/test/cpp/faultInjection_ut.cpp +++ b/ddprof-lib/src/test/cpp/faultInjection_ut.cpp @@ -243,13 +243,19 @@ class ScopedJvmtiMock { 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. +// (d) Value-injection path: Profiler::checkState() (shared by start()/check(), +// and therefore also reached by the -agentpath auto-start path) fails 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. +// +// checkState() only treats that failure as fatal off musl (see its comment: +// musl never hits the pthread_exit path that needs libgcc_s.so.1), so on +// musl an injected failure falls through to the mocked +// JVMSupport::initialize() failure instead of surfacing "Missing +// libgcc_s.so.1" -- expect whichever outcome the current libc implies. TEST_F(FaultInjectionTest, CheckStateSurfacesInjectedPrewarmUnwinderFailure) { #ifdef __linux__ Profiler* p = Profiler::instance(); @@ -259,6 +265,7 @@ TEST_F(FaultInjectionTest, CheckStateSurfacesInjectedPrewarmUnwinderFailure) { ProfilerTestAccessor::setState(p, NEW); ProfiledThread::current()->setFiRng(0x5EED5EED5EED5EEDULL); + const bool prewarmFailureIsFatal = !OS::isMusl(); bool sawInjectedFailure = false; bool sawNonInjectedPrewarm = false; // shouldFire() mixes the fixed RNG seed above with an ASLR-dependent @@ -267,23 +274,50 @@ TEST_F(FaultInjectionTest, CheckStateSurfacesInjectedPrewarmUnwinderFailure) { // 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++) { + // shouldFire() increments FAULTS_INJECTED exactly once whenever it fires, + // and the only fault-injection site reachable from checkState() is the + // INJECT_FAULT_BOOL_LIKELY around prewarmUnwinder()'s dlopen() call -- + // JVMSupport::initialize() below is a plain mock, not an injection site. + // So the counter delta across one checkState() call, not the returned + // error message, is the ground truth for whether this iteration actually + // took the injected path; that lets the two outcomes be set from + // independent evidence instead of both being inferred from one string + // compare. + long long faultsBefore = Counters::getCounter(FAULTS_INJECTED); Error error = p->checkState(); + bool injectedThisCall = Counters::getCounter(FAULTS_INJECTED) > faultsBefore; 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; + if (prewarmFailureIsFatal) { + if (injectedThisCall) { + EXPECT_STREQ("Missing libgcc_s.so.1", error.message()); + 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; + } } else { - // prewarmUnwinder() succeeded (non-injected, ~99% of calls) and fell - // through to the mocked JVMSupport::initialize() failure instead. + // On musl, checkState() doesn't treat a prewarmUnwinder() failure as + // fatal, so both an injected and a non-injected call fall through to + // the same mocked JVMSupport::initialize() failure message -- only the + // counter delta distinguishes them. EXPECT_STREQ("Profiler encountered fatal error", error.message()); - sawNonInjectedPrewarm = true; + if (injectedThisCall) { + sawInjectedFailure = true; + } else { + sawNonInjectedPrewarm = true; + } } ProfilerTestAccessor::setState(p, NEW); } - EXPECT_TRUE(sawInjectedFailure) - << "expected at least one injected prewarmUnwinder() failure within 5000 tries"; + if (prewarmFailureIsFatal) { + 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__