fix: don't disable profiler on Alpine/musl over missing libgcc_s.so.1 (#720)

Author: jbachorik

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");
     }

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__