Never patch the profiler's own import table

ddprof-lib/src/main/cpp/libraryPatcher.h

@@ -18,9 +18,16 @@ typedef struct _patchEntry {
 
 
 class LibraryPatcher {
+  friend class LibraryPatcherTestAccessor;
+
 private:
   static SpinLock    _lock;
-  static const char* _profiler_name;
+  // Set by initialize(), which Profiler::start() calls just before the first
+  // library scan. Patching must not begin any earlier: pthread_create_hook()
+  // routes newly created threads through Profiler::registerThread(), which
+  // dereferences state that only exists once the profiler is running. Read from
+  // the Libraries refresher thread, hence atomic with release/acquire.
+  static std::atomic<bool> _initialized;
   static PatchEntry  _patched_entries[MAX_NATIVE_LIBS];
   static int         _size;
   static bool        _patch_pthread_create;
@@ -38,6 +45,11 @@ class LibraryPatcher {
   static void patch_pthread_create();
   static void patch_pthread_setspecific();
   static void patch_sigaction_in_library(CodeCache* lib);
+  // An address known to lie inside this library; how is_profiler_library()
+  // recognises the profiler's own mapping.
+  static const void* self_anchor();
+  // True when `lib` is the profiler's own library, which must never be patched.
+  static bool is_profiler_library(CodeCache* lib);
 public:
   // True while socket hooks are installed; read by Profiler::dlopen_hook
   // to decide whether to re-patch after a new library is loaded.

ddprof-lib/src/main/cpp/libraryPatcher_linux.cpp

@@ -12,18 +12,15 @@
 #include "nativeSocketSampler.h"
 #include "profiler.h"
 
-#include <cassert>
 #include <dlfcn.h>
 #include <mutex>
-#include <limits.h>
 #include <setjmp.h>
 #include <string.h>
-#include <stdlib.h>
 
 typedef void* (*func_start_routine)(void*);
 
 SpinLock LibraryPatcher::_lock;
-const char* LibraryPatcher::_profiler_name = nullptr;
+std::atomic<bool> LibraryPatcher::_initialized{false};
 PatchEntry LibraryPatcher::_patched_entries[MAX_NATIVE_LIBS];
 int        LibraryPatcher::_size = 0;
 PatchEntry LibraryPatcher::_sigaction_entries[MAX_NATIVE_LIBS];
@@ -33,16 +30,40 @@ int        LibraryPatcher::_socket_size = 0;
 std::atomic<bool> LibraryPatcher::_socket_active{false};
 
 void LibraryPatcher::initialize() {
-  if (_profiler_name == nullptr) {
-    Dl_info info;
-    void* caller_address = __builtin_return_address(0); // Get return address of caller
-    bool ret = dladdr(caller_address, &info);
-    assert(ret);
-    _profiler_name = realpath(info.dli_fname, nullptr);
+  if (!_initialized.load(std::memory_order_acquire)) {
     _size = 0;
+    // Release so the reset above is visible to any thread that sees the flag.
+    _initialized.store(true, std::memory_order_release);
   }
 }
 
+const void* LibraryPatcher::self_anchor() {
+  // Any code address in this translation unit lies inside the profiler's own
+  // mapping, so its address identifies us. A function is used rather than a
+  // static variable because a CodeCache spans a library's executable segments
+  // (see Symbols::parseLibraries), which do not cover .data/.bss.
+  return (const void*)&self_anchor;
+}
+
+// The profiler must never patch its own import table. pthread_create_hook()
+// below reaches the real pthread_create() through this library's own PLT, so a
+// self-patch makes the hook call itself: the recursion exhausts the thread
+// stack and the process dies of SIGSEGV without even an hs_err file, because
+// the JVM's crash handler needs stack of its own to run.
+//
+// Recognising ourselves by mapped address range is the whole check. Comparing
+// realpath(lib->name()) with our own path used to be, and it silently fails:
+// dd-trace-java extracts libjavaProfiler.so to a temporary file and unlinks it
+// once loaded, so realpath() on the still-mapped path returns nullptr and the
+// comparison reported "not self". Whether that mattered depended on a library
+// re-scan landing after the unlink, which is why the crashes looked random.
+//
+// Every native library cache carries its mapping bounds (Symbols::parseLibraries
+// builds them from /proc/self/maps), so the range test needs no fallback.
+bool LibraryPatcher::is_profiler_library(CodeCache* lib) {
+  return lib != nullptr && lib->contains(self_anchor());
+}
+
 class RoutineInfo {
 private:
   func_start_routine _routine;
@@ -406,8 +427,9 @@ static int pthread_create_hook(pthread_t* thread,
 }
 
 void LibraryPatcher::patch_libraries() {
-   // LibraryPatcher has yet initialized, only happens in Gtest
-   if (_profiler_name == nullptr) {
+   // Profiler::start() has not run yet, so the hook would have nowhere to
+   // register new threads. Also the case in Gtest, which never initializes.
+   if (!_initialized.load(std::memory_order_acquire)) {
      return;
    }
 
@@ -419,10 +441,7 @@ void LibraryPatcher::patch_libraries() {
 void LibraryPatcher::patch_library_unlocked(CodeCache* lib) {
   if (lib->name() == nullptr) return;
 
-  char path[PATH_MAX];
-  char* resolved_path = realpath(lib->name(), path);
-  if (resolved_path != nullptr &&     //  filter out virtual file, e.g. [vdso], etc.
-      strcmp(resolved_path, _profiler_name) == 0) { // Don't patch self
+  if (is_profiler_library(lib)) { // Don't patch self
     return;
   }
 
@@ -489,12 +508,10 @@ void LibraryPatcher::patch_pthread_create() {
 // (like wasmtime) that install broken signal handlers calling malloc().
 void LibraryPatcher::patch_sigaction_in_library(CodeCache* lib) {
   if (lib->name() == nullptr) return;
-  if (_profiler_name == nullptr) return;  // Not initialized yet
+  if (!_initialized.load(std::memory_order_acquire)) return;  // Not initialized yet
 
   // Don't patch ourselves
-  char path[PATH_MAX];
-  char* resolved_path = realpath(lib->name(), path);
-  if (resolved_path != nullptr && strcmp(resolved_path, _profiler_name) == 0) {
+  if (is_profiler_library(lib)) {
     return;
   }
 
@@ -598,20 +615,7 @@ bool LibraryPatcher::patch_socket_functions() {
   const CodeCacheArray& native_libs = Libraries::instance()->native_libs();
   int num_of_libs = native_libs.count();
 
-  // Pre-resolve all library paths before acquiring the lock: realpath() may
-  // block on I/O and must not be called while holding _lock.
-  // We only need the is-self flag per library, so avoid a huge stack allocation.
-  static_assert(MAX_NATIVE_LIBS > 0, "MAX_NATIVE_LIBS must be positive");
-  bool is_self[MAX_NATIVE_LIBS];
   int capped = (num_of_libs <= MAX_NATIVE_LIBS) ? num_of_libs : MAX_NATIVE_LIBS;
-  for (int index = 0; index < capped; index++) {
-    CodeCache* lib = native_libs.at(index);
-    is_self[index] = false;
-    if (lib == nullptr || lib->name() == nullptr) continue;
-    char path[PATH_MAX];
-    char* rp = realpath(lib->name(), path);
-    is_self[index] = (rp != nullptr && strcmp(rp, _profiler_name) == 0);
-  }
 
   ExclusiveLockGuard locker(&_lock);
   // Re-check under the lock only on re-entry (when hooks are already installed):
@@ -651,7 +655,10 @@ bool LibraryPatcher::patch_socket_functions() {
     if (lib == nullptr) continue;
     if (lib->name() == nullptr) continue;
 
-    if (is_self[index]) {
+    // Checked here rather than in a pre-pass keyed by index: the library array
+    // can grow between the two, and a flag applied to the wrong entry could let
+    // us patch ourselves.
+    if (is_profiler_library(lib)) {
       continue;
     }
 

ddprof-lib/src/main/cpp/profiler.cpp

@@ -1573,7 +1573,8 @@ Error Profiler::start(Arguments &args, bool reset) {
   // Prepare JVMSupport for execution
   JVMSupport::initExecution(args, VM::jvmti(), VM::jni());
 
-
+  // Must precede the first updateSymbols(): it is what allows LibraryPatcher to
+  // start patching, and the hooks it installs assume a running profiler.
   LibraryPatcher::initialize();
 
   // Kernel symbols are useful only for perf_events without --all-user

ddprof-lib/src/test/cpp/libraryPatcher_ut.cpp

@@ -0,0 +1,170 @@
+/*
+ * Copyright 2026, Datadog, Inc.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <gtest/gtest.h>
+
+#if defined(__linux__)
+
+#include "codeCache.h"
+#include "libraryPatcher.h"
+
+#include <dlfcn.h>
+
+// LibraryPatcherTestAccessor — friend of LibraryPatcher, exposes the internals
+// the self-identification tests need.
+class LibraryPatcherTestAccessor {
+public:
+  static const void* selfAnchor() { return LibraryPatcher::self_anchor(); }
+
+  static bool isProfilerLibrary(CodeCache* lib) {
+    return LibraryPatcher::is_profiler_library(lib);
+  }
+
+  static void patchLibraryUnlocked(CodeCache* lib) {
+    LibraryPatcher::patch_library_unlocked(lib);
+  }
+
+  static int patchedCount() { return LibraryPatcher::_size; }
+
+  // Drops the patch table without writing the saved originals back, unlike
+  // unpatch_libraries(). Tests patch GOT slots that live in the test body's
+  // frame, so by the time a fixture could restore them the addresses are dead;
+  // forgetting the entries is what keeps one test from reaching into the next.
+  static void forgetPatchTable() { LibraryPatcher::_size = 0; }
+};
+
+namespace {
+
+// Stands in for the profiler's own library file after dd-trace-java's
+// LibraryLoader extracts it and unlinks it: the mapping is still live, but the
+// path no longer resolves, so realpath() returns nullptr.
+const char* const kUnlinkedPath =
+    "/tmp/ddprof_bits/pid_1/scratch/libjavaProfiler-dd-tmp-unlinked.so";
+
+// An address range that cannot overlap any real mapping of this binary.
+const void* const kForeignMin = (const void*)0x1000;
+const void* const kForeignMax = (const void*)0x2000;
+
+// Stands in for the real pthread_create that a library's GOT slot holds before
+// the profiler patches it.
+void* fake_original_pthread_create(void* arg) { return arg; }
+
+const void* anchorMin() {
+  return (const char*)LibraryPatcherTestAccessor::selfAnchor() - 0x1000;
+}
+
+const void* anchorMax() {
+  return (const char*)LibraryPatcherTestAccessor::selfAnchor() + 0x1000;
+}
+
+} // namespace
+
+// The regression test for the self-patching crash: a cache that covers our own
+// code must be recognised as ours even when its path cannot be resolved.
+//
+// Before the fix the only self-check was realpath(name) == profiler path. The
+// profiler's library is extracted to a temp file and unlinked once loaded, so
+// realpath() returned nullptr, the check reported "not self", and the profiler
+// patched its own pthread_create GOT slot. pthread_create_hook() calls
+// pthread_create() through that same slot, so it then called itself until the
+// thread stack was exhausted — SIGSEGV, and no hs_err, since reporting a crash
+// also needs stack.
+TEST(LibraryPatcherSelfCheck, RecognisesSelfWhenItsLibraryFileWasUnlinked) {
+  CodeCache lib(kUnlinkedPath, -1, anchorMin(), anchorMax());
+
+  EXPECT_TRUE(LibraryPatcherTestAccessor::isProfilerLibrary(&lib));
+}
+
+// The address range is what identifies us, not the mere fact that a path failed
+// to resolve: a library that is genuinely not ours stays patchable even if its
+// own path is unresolvable (deleted or replaced on upgrade, say).
+TEST(LibraryPatcherSelfCheck, DoesNotRecogniseForeignLibraryWithUnresolvablePath) {
+  CodeCache lib(kUnlinkedPath, -1, kForeignMin, kForeignMax);
+
+  EXPECT_FALSE(LibraryPatcherTestAccessor::isProfilerLibrary(&lib));
+}
+
+// Identity is decided by the mapping alone, so a library that is not ours stays
+// patchable whatever its path says — including a perfectly resolvable one. This
+// pins the contract against a future reintroduction of name matching, which is
+// what failed here in the first place.
+TEST(LibraryPatcherSelfCheck, DoesNotRecogniseUnrelatedResolvablePath) {
+  CodeCache lib("/proc/self/cmdline", -1, kForeignMin, kForeignMax);
+
+  EXPECT_FALSE(LibraryPatcherTestAccessor::isProfilerLibrary(&lib));
+}
+
+// A cache with no known address range (min/max default to
+// NO_MIN_ADDRESS/NO_MAX_ADDRESS, for which contains() is always false) is not
+// us. Naming our own library must not change that, now that the name plays no
+// part in the decision.
+TEST(LibraryPatcherSelfCheck, DoesNotRecogniseCacheWithUnknownAddressRange) {
+  Dl_info info;
+  ASSERT_NE(0, dladdr(LibraryPatcherTestAccessor::selfAnchor(), &info));
+
+  CodeCache lib(info.dli_fname);
+
+  EXPECT_FALSE(LibraryPatcherTestAccessor::isProfilerLibrary(&lib));
+}
+
+// The predicate dereferences nothing but the cache's bounds, so it is safe to
+// call at any point — including before the profiler has started, which is when
+// the patch sites used to compare against a not-yet-resolved profiler path.
+TEST(LibraryPatcherSelfCheck, TreatsNullLibraryAsForeign) {
+  EXPECT_FALSE(LibraryPatcherTestAccessor::isProfilerLibrary(nullptr));
+}
+
+// patch_library_unlocked() skips a cache it has already patched by comparing
+// CodeCache addresses, and successive tests tend to place their cache at the
+// same stack address. Start each test from an empty table so one cannot mask a
+// failure in the next.
+class LibraryPatcherPatching : public ::testing::Test {
+protected:
+  void SetUp() override { LibraryPatcherTestAccessor::forgetPatchTable(); }
+};
+
+// End to end through the patcher: given a cache that looks like the profiler's
+// own unlinked library and does expose a pthread_create import, the slot must be
+// left alone and nothing recorded in the patch table.
+TEST_F(LibraryPatcherPatching, LeavesItsOwnPthreadCreateSlotUntouched) {
+  void* const original = (void*)&fake_original_pthread_create;
+  void* slot = original;
+
+  CodeCache lib(kUnlinkedPath, -1, anchorMin(), anchorMax(), nullptr,
+                /* imports_patchable */ true);
+  lib.addImport(&slot, "pthread_create");
+  ASSERT_EQ(&slot, lib.findImport(im_pthread_create));
+
+  int before = LibraryPatcherTestAccessor::patchedCount();
+  LibraryPatcherTestAccessor::patchLibraryUnlocked(&lib);
+
+  EXPECT_EQ(original, slot);
+  EXPECT_EQ(before, LibraryPatcherTestAccessor::patchedCount());
+}
+
+// The counterpart, so the test above cannot pass by the patcher simply having
+// stopped working: an identical cache that is not ours does get its slot
+// redirected to the hook, and unpatching puts the original back.
+TEST_F(LibraryPatcherPatching, StillPatchesForeignPthreadCreateSlot) {
+  void* const original = (void*)&fake_original_pthread_create;
+  void* slot = original;
+
+  CodeCache lib(kUnlinkedPath, -1, kForeignMin, kForeignMax, nullptr,
+                /* imports_patchable */ true);
+  lib.addImport(&slot, "pthread_create");
+
+  int before = LibraryPatcherTestAccessor::patchedCount();
+  LibraryPatcherTestAccessor::patchLibraryUnlocked(&lib);
+
+  EXPECT_NE(original, slot);
+  EXPECT_EQ(before + 1, LibraryPatcherTestAccessor::patchedCount());
+
+  LibraryPatcher::unpatch_libraries();
+
+  EXPECT_EQ(original, slot);
+  EXPECT_EQ(0, LibraryPatcherTestAccessor::patchedCount());
+}
+
+#endif // __linux__