🍒 Prepare patch: backport 5 PR(s) to release/1.52._

.github/workflows/ci.yml

@@ -156,6 +156,47 @@ jobs:
         run: |
           ./gradlew spotlessCheck --no-daemon --parallel --build-cache --no-watch-fs
 
+  source-invariant-checks:
+    runs-on: ubuntu-22.04
+    needs: check-for-pr
+    if: needs.check-for-pr.outputs.skip != 'true'
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          fetch-depth: 0
+
+      - name: Setup Java
+        uses: actions/setup-java@b843ebf9b913146b38da2eaa7aeddf76074ae1ad # v6.1.0
+        with:
+          distribution: 'zulu'
+          java-version: '21'
+
+      - name: Setup OS
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y curl zip unzip binutils g++ python3
+
+      - name: Cache Gradle Wrapper Binaries
+        uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
+        with:
+          path: ~/.gradle/wrapper/dists
+          key: gradle-wrapper-${{ runner.os }}-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties') }}
+          restore-keys: |
+            gradle-wrapper-${{ runner.os }}-
+
+      - name: Cache Gradle User Home
+        uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
+        with:
+          path: ~/.gradle/caches
+          key: gradle-caches-${{ runner.os }}-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }}
+          restore-keys: |
+            gradle-caches-${{ runner.os }}-
+
+      - name: Source invariant checks
+        run: |
+          # Signal-reachability check (ELF debug objects + fixture self-test) and the utils/check-*.py source checks
+          ./gradlew :ddprof-lib:qualityChecks --no-daemon --build-cache --no-watch-fs
+
   check-javadoc:
     runs-on: ubuntu-22.04
     needs: check-for-pr

.github/workflows/codecheck.yml

@@ -29,7 +29,7 @@ jobs:
           sudo apt-get update
           sudo apt install -y clang clang-tools openjdk-11-jdk
       - name: Set up Python
-        uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
+        uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0
         with:
           python-version: 3.14
       - name: Install Python dependencies

AGENTS.md

@@ -343,6 +343,14 @@ exception list.
   helpers unless reused or genuinely hard to follow; "why" comments must reference concrete
   mechanisms (e.g. "ASAN's allocator lock is reentrant"), never speculative claims about
   HotSpot/glibc/ASAN internals — if unsure, describe the observable symptom.
+- **RAII for every acquire/release pair** (C/C++). A lock, a heap allocation, or a flag
+  that must be restored is released by a guard or owner destructor, not by a manual call
+  at each exit. Reuse `SharedLockGuard`, `ExclusiveLockGuard`, `OptionalSharedLockGuard`,
+  `MutexLocker`, `CriticalSection` before you add a type. A new owner type is non-copyable
+  and hands over ownership explicitly (the receiver sets the owner's pointer to `nullptr`).
+  A manual pair needs a comment that says why a guard scope cannot express it. In code that
+  shares a lock with signal handlers, end an owner that frees memory outside the exclusive
+  critical section, because its destructor frees at scope end.
 - Always challenge proposals with deep analysis when you have hard evidence.
 
 ## Work Tracking

ddprof-lib/build.gradle.kts

@@ -183,6 +183,93 @@ afterEvaluate {
   }
 }
 
+val debugObjDir: File = layout.buildDirectory.dir("obj/main/debug").get().asFile
+val signalReachabilityScript: File = rootProject.file("utils/check-signal-reachability.py")
+
+// The reachability checker reads ELF objects only (Linux); Mach-O objects are skipped.
+fun hasElfObjects(dir: File): Boolean {
+  val obj = dir.listFiles { f -> f.name.endsWith(".o") }?.firstOrNull() ?: return false
+  val magic = ByteArray(4)
+  return obj.inputStream().use { it.read(magic) } == 4 &&
+    magic.contentEquals(byteArrayOf(0x7f, 'E'.code.toByte(), 'L'.code.toByte(), 'F'.code.toByte()))
+}
+
+// Static async-signal-safety reachability check over the debug objects; see
+// utils/check-signal-reachability.py. Not wired into check/build: run
+// `./gradlew :ddprof-lib:checkSignalReachability` (fails fast), or via
+// qualityChecks. The checker reads ELF objects only, so the task is skipped
+// when the debug objects are absent or not ELF.
+tasks.register<Exec>("checkSignalReachability") {
+  group = "verification"
+  description = "Checks that no async-signal-unsafe function is reachable from the signal handlers and malloc hooks"
+  // Compile task exists for every active configuration; named lookup fails loudly on rename.
+  dependsOn(tasks.named("compileDebug"))
+
+  commandLine("python3", signalReachabilityScript.absolutePath, "--obj-dir", debugObjDir.absolutePath)
+
+  onlyIf("debug objects are ELF") { hasElfObjects(debugObjDir) }
+}
+
+// Quality checks: the signal-reachability check (ELF hosts only) and its
+// fixture self-test (Linux only), plus the source-level utils/check-*.py
+// scripts, each also runnable on its own. Not wired into check/build; CI runs
+// `./gradlew :ddprof-lib:qualityChecks` in the source-invariant-checks job of
+// .github/workflows/ci.yml. Runs every check, collects every failure, and
+// fails once at the end.
+interface QualityCheckServices {
+  @get:Inject val exec: ExecOperations
+}
+
+tasks.register("qualityChecks") {
+  group = "verification"
+  description = "Runs the signal-reachability check and the source-level quality checks in utils/"
+  // Depends on the debug objects, not on checkSignalReachability: that Exec
+  // task fails fast, which would stop the remaining checks from running.
+  dependsOn(tasks.named("compileDebug"))
+
+  val exec = objects.newInstance<QualityCheckServices>().exec
+  val root = rootProject.projectDir
+  val scripts = listOf(
+    "check-jvm-boundary-includes.py",
+    "check-doc-index.py",
+    "check-sigsetjmp-shape.py",
+    "check-tracked-artifacts.py",
+  ).map { File(root, "utils/$it") }
+
+  doLast {
+    val failed = mutableListOf<String>()
+    // Compile-and-assert the planted-violation fixture first: it pins the
+    // checker's most fragile assumptions (section symbols, ctor aliases, TLS,
+    // std-library attribution) and catches toolchain/objdump drift. The
+    // checker is ELF-only, so the self-test runs on Linux hosts only (macOS
+    // produces Mach-O fixture objects by design).
+    val onLinux = System.getProperty("os.name").lowercase().contains("linux")
+    if (onLinux) {
+      exec.exec {
+        commandLine("python3", signalReachabilityScript.absolutePath, "--self-test")
+        isIgnoreExitValue = true
+      }.exitValue.takeIf { it != 0 }?.let { failed += "self-test (check-signal-reachability.py --self-test)" }
+    }
+    if (hasElfObjects(debugObjDir)) {
+      exec.exec {
+        commandLine("python3", signalReachabilityScript.absolutePath, "--obj-dir", debugObjDir.absolutePath)
+        isIgnoreExitValue = true
+      }.exitValue.takeIf { it != 0 }?.let { failed += "check-signal-reachability.py" }
+    } else {
+      logger.lifecycle("qualityChecks: check-signal-reachability.py skipped (debug objects are not ELF)")
+    }
+    failed += scripts.filter { script ->
+      exec.exec {
+        commandLine("python3", script.absolutePath, "--root", root.absolutePath)
+        isIgnoreExitValue = true
+      }.exitValue != 0
+    }.map { it.name }
+    if (failed.isNotEmpty()) {
+      throw GradleException("quality checks failed: ${failed.joinToString { it }}")
+    }
+  }
+}
+
 // Add runBenchmarks task
 tasks.register("runBenchmarks") {
   dependsOn(":ddprof-lib:benchmarks:runBenchmark")

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

@@ -176,8 +176,8 @@ CodeCache::~CodeCache() {
 }
 
 long long CodeCache::memoryUsage() const {
-  // O(1): _memory_usage is updated incrementally by the constructor, add(),
-  // expand(), and setDwarfTable() -- the same pre-publication mutators that
+  // O(1): _memory_usage is updated incrementally by the constructor,
+  // add()/commitAdd(), and setDwarfTable() -- the same pre-publication mutators that
   // used to be re-summed here on every call. Safe to read lock-free once
   // published (Libraries::native_libs) because those mutators never run
   // afterwards (asserted via _published), same invariant the old per-call
@@ -191,55 +191,76 @@ long long CodeCache::memoryUsage() const {
   return _memory_usage;
 }
 
-void CodeCache::expand() {
-  // Must not run after publication: memoryUsage() reads _blobs lock-free from
-  // the dump thread and a concurrent realloc would free it underneath.
-  assert(!_published.load(std::memory_order_acquire) &&
-         "expand() on a published CodeCache races memoryUsage()");
-  CodeBlob *old_blobs = _blobs;
-  CodeBlob *new_blobs = new CodeBlob[_capacity * 2];
-
-  memcpy(new_blobs, old_blobs, _count * sizeof(CodeBlob));
-
-  // Delta is exactly the current (pre-doubling) capacity's worth of blobs.
-  _memory_usage += (long long)_capacity * sizeof(CodeBlob);
-  _capacity *= 2;
-  _blobs = new_blobs;
-  delete[] old_blobs;
-}
-
-void CodeCache::add(const void *start, int length, const char *name,
-                    bool update_bounds) {
+void CodeCache::prepareAdd(const char *name, PreparedAdd &prepared) const {
   // Symbols are added while parsing a library, before it is registered into a
   // CodeCacheArray. Adding after publication would race memoryUsage() (which
   // reads _blobs/_count lock-free at dump time). Unpublished standalone caches
   // (e.g. _runtime_stubs) are exempt — they are never published.
   assert(!_published.load(std::memory_order_acquire) &&
          "add() on a published CodeCache races memoryUsage()");
+  // A reused PreparedAdd would leak the allocations it still owns.
+  assert(prepared.name == nullptr && prepared.grown == nullptr);
   char *name_copy = NativeFunc::create(name, _lib_index);
-  _memory_usage += (long long)NativeFunc::allocSize(name);
+  prepared.name = name_copy;
+  prepared.name_size = (long long)NativeFunc::allocSize(name);
   // Measured off the block just handed back, while the pointer is in hand: the
   // gauge is published as a total at add() time, with no pointer left to probe.
-  _name_overhead += (long long)NativeFunc::nameOverhead(name_copy);
+  prepared.name_overhead = (long long)NativeFunc::nameOverhead(name_copy);
   // Replace non-printable characters
   for (char *s = name_copy; *s != 0; s++) {
     if (*s < ' ')
       *s = '?';
   }
 
+  prepared.capacity = _capacity;
+  prepared.grown = nullptr;
   if (_count >= _capacity) {
-    expand();
+    prepared.grown = new CodeBlob[_capacity * 2];
+    memcpy(prepared.grown, _blobs, _count * sizeof(CodeBlob));
+  }
+}
+
+CodeBlob *CodeCache::commitAdd(const void *start, int length,
+                               PreparedAdd &prepared,
+                               bool update_bounds) {
+  assert(!_published.load(std::memory_order_acquire) &&
+         "add() on a published CodeCache races memoryUsage()");
+  // A mutation between prepareAdd() and commitAdd() would make the grown copy
+  // stale or leave no free slot.
+  assert(prepared.capacity == _capacity &&
+         (prepared.grown != nullptr || _count < _capacity));
+  CodeBlob *retired = nullptr;
+  if (prepared.grown != nullptr) {
+    // Delta is exactly the current (pre-doubling) capacity's worth of blobs.
+    _memory_usage += (long long)_capacity * sizeof(CodeBlob);
+    _capacity *= 2;
+    retired = _blobs;
+    _blobs = prepared.grown;
   }
+  _memory_usage += prepared.name_size;
+  _name_overhead += prepared.name_overhead;
 
   const void *end = (const char *)start + length;
   _blobs[_count]._start = start;
   _blobs[_count]._end = end;
-  _blobs[_count]._name = name_copy;
+  _blobs[_count]._name = prepared.name;
   _count++;
+  // The cache owns both allocations now; the PreparedAdd destructor must not
+  // free them.
+  prepared.name = nullptr;
+  prepared.grown = nullptr;
 
   if (update_bounds) {
     updateBounds(start, end);
   }
+  return retired;
+}
+
+void CodeCache::add(const void *start, int length, const char *name,
+                    bool update_bounds) {
+  PreparedAdd prepared;
+  prepareAdd(name, prepared);
+  delete[] commitAdd(start, length, prepared, update_bounds);
 }
 
 void CodeCache::updateBounds(const void *start, const void *end) {

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

@@ -181,7 +181,7 @@ class CodeCache {
   CodeBlob *_blobs;
 
   // Running total of memoryUsage()'s formula, updated incrementally by the
-  // same pre-publication mutators (constructor, add(), expand(),
+  // same pre-publication mutators (constructor, add()/commitAdd(),
   // setDwarfTable()) instead of recomputed by iterating _blobs. Safe because
   // every contributing field is fixed before publication (see _published
   // below) -- there is no point after construction-and-population where this
@@ -204,17 +204,17 @@ class CodeCache {
 
   // Set once the cache is registered into a CodeCacheArray (see markPublished()).
   // After that, memoryUsage() may be read lock-free from another thread (dump),
-  // so the mutators that touch the fields it reads (add()/expand()/
-  // setDwarfTable()) must not run — asserted in debug. Standalone caches that
+  // so the mutators that touch the fields it reads (add()/prepareAdd()/
+  // commitAdd()/setDwarfTable()) must not run — asserted in debug. Standalone caches that
   // are never registered (e.g. JitCodeCache::_runtime_stubs, which is instead
-  // guarded by its own JitCodeCache::_stubs_lock) stay unpublished, so the
+  // guarded by its own JitCodeCache::_stubs_lock, with writers serialized by
+  // JitCodeCache::_stubs_write_lock) stay unpublished, so the
   // asserts never fire for them. Atomic with release/acquire so the flag is
   // observably true on any thread that reaches a published cache — the store is
   // sequenced before CodeCacheArray::add()'s RELEASE publish of the pointer, and
   // the assert loads it with acquire — without a formal data race.
   std::atomic<bool> _published;
 
-  void expand();
   void makeImportsPatchable();
   void saveImport(ImportId id, void** entry);
   void copyFrom(const CodeCache& other);
@@ -282,12 +282,47 @@ class CodeCache {
   void setImageFingerprint(u64 fingerprint) { _image_fingerprint = fingerprint; }
 
   // Mark this cache as published into a CodeCacheArray. Call before the array
-  // makes the pointer visible to readers; afterwards add()/expand()/
-  // setDwarfTable() must not be called (see _published).
+  // makes the pointer visible to readers; afterwards add()/prepareAdd()/
+  // commitAdd()/setDwarfTable() must not be called (see _published).
   void markPublished() { _published.store(true, std::memory_order_release); }
 
   void add(const void *start, int length, const char *name,
            bool update_bounds = false);
+
+  // Heap allocations that prepareAdd() makes for one later commitAdd().
+  // commitAdd() takes ownership of name and grown and sets them to nullptr.
+  // If no commitAdd() happens, the destructor frees them.
+  struct PreparedAdd {
+    char *name = nullptr;          // NativeFunc name copy, non-printables replaced
+    long long name_size = 0;       // NativeFunc::allocSize() of the name
+    long long name_overhead = 0;   // NativeFunc::nameOverhead() of the copy
+    CodeBlob *grown = nullptr;     // filled copy of _blobs at twice the capacity, or
+                                   // nullptr when the current array has a free slot
+    int capacity = 0;              // _capacity that this preparation is valid for
+
+    PreparedAdd() = default;
+    ~PreparedAdd() {
+      if (name != nullptr) {
+        NativeFunc::destroy(name);
+      }
+      delete[] grown;
+    }
+    // Non-copyable: a copy would free the same allocations twice.
+    PreparedAdd(const PreparedAdd &) = delete;
+    PreparedAdd &operator=(const PreparedAdd &) = delete;
+  };
+
+  // Two-phase add() for a cache whose readers run in signal handlers under a
+  // SpinLock (JitCodeCache::_runtime_stubs): prepareAdd() makes every heap
+  // allocation and can run outside the lock; commitAdd() only stores and
+  // copies pointers, so it is safe under the exclusive lock. commitAdd()
+  // returns the replaced blob array (or nullptr), which the caller must
+  // delete[] after it releases the lock. Between prepareAdd() and commitAdd()
+  // nothing else may mutate this cache; the caller serializes the pair
+  // against other writers. Pass a freshly constructed PreparedAdd.
+  void prepareAdd(const char *name, PreparedAdd &prepared) const;
+  CodeBlob *commitAdd(const void *start, int length, PreparedAdd &prepared,
+                      bool update_bounds = false);
   void updateBounds(const void *start, const void *end);
   void sort();
 
@@ -407,7 +442,7 @@ class CodeCacheArray {
     // so it stays consistent with the logical bytes above.
     NativeMem::recordOverhead(NM_NATIVE_SYMBOLS, lib->nameOverhead());
     // Mark published before the RELEASE store makes the pointer visible, so any
-    // later add()/expand()/setDwarfTable() on this cache trips the assert (its
+    // later add()/prepareAdd()/commitAdd()/setDwarfTable() on this cache trips the assert (its
     // _blobs would then be read lock-free by memoryUsage() at dump time).
     lib->markPublished();
     // Store pointer before publishing count. The RELEASE here pairs with

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

@@ -42,6 +42,7 @@
   X(DICTIONARY_ENDPOINTS_ARENA_WASTE_BYTES, "dictionary_endpoints_arena_waste_bytes") \
   X(DICTIONARY_CONTEXT_ARENA_WASTE_BYTES, "dictionary_context_arena_waste_bytes")     \
   X(DICTIONARY_DRAIN_TIMEOUTS, "dictionary_drain_timeouts")                    \
+  X(REFCOUNT_NESTED_OVERFLOW, "refcount_nested_overflow")                      \
   X(DICTIONARY_ID_CONFLICTS, "dictionary_id_conflicts")                        \
   X(CLASS_MAP_AT_CAPACITY, "class_map_at_capacity_drops")                      \
   X(CONTEXT_STORAGE_BYTES, "context_storage_bytes")                            \
@@ -135,6 +136,11 @@
   X(WALKVM_STUB_INFO_HIT, "walkvm_stub_info_hit")                            \
   X(WALKVM_STUB_INFO_FALLBACK, "walkvm_stub_info_fallback")                  \
   X(WALKVM_STUB_INFO_REGISTRATION_FAILED, "walkvm_stub_info_registration_failed") \
+  /* Runtime-stub lookups (JitCodeCache::findRuntimeStub/findRuntimeStubInfo)  \
+   * that returned "not found" without searching because a registration held  \
+   * the stubs lock exclusively; the stack walk continued with its             \
+   * stub-not-found fallback. */                                               \
+  X(STUB_LOOKUP_LOCK_UNAVAILABLE, "stub_lookup_lock_unavailable")            \
   X(WALKVM_FP_CHAIN_ATTEMPT, "walkvm_fp_chain_attempt")                      \
   X(WALKVM_FP_CHAIN_REACHED_CODEHEAP, "walkvm_fp_chain_reached_codeheap")    \
   X(WALKVM_ANCHOR_NOT_IN_JAVA,  "walkvm_anchor_not_in_java")                   \