From ea28f71e1031adc6fe77429be25819de3ec9bcb3 Mon Sep 17 00:00:00 2001
From: Roman Kennke <roman.kennke@datadoghq.com>
Date: Tue, 21 Jul 2026 13:03:29 +0200
Subject: [PATCH 1/8] fix(codecache): make memoryUsage() accurate and live
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CodeCache::memoryUsage() drove the CODECACHE_NATIVE_SIZE_BYTES counter
but both under-counted and went stale:

- The blob array was counted as _capacity * sizeof(CodeBlob*) — a
  pointer's worth — while the array holds CodeBlob (three pointers),
  a ~3x undercount.
- Symbol name strings were approximated as _count * sizeof(NativeFunc),
  a fixed 4 bytes each, ignoring the name length that dominates the
  actual allocation.
- The DWARF unwind table and build-id string were not counted at all.
- CodeCacheArray cached the sum at add() time, so it never reflected a
  library's later symbol growth.

Recompute both accurately on demand instead. CodeCache::memoryUsage() now
sums the full blob array, each symbol's real name-string allocation (via
new NativeFunc::allocSize()), its own name, the DWARF table, and the
build-id. CodeCacheArray::memoryUsage() sums the live per-library values
(the array is append-only, so iterating the published prefix is safe
alongside concurrent adds); the stale _used_memory cache is removed.
memoryUsage() is only called at dump time, so the O(symbols) recompute is
not on any hot path.

Tests: new codeCache_ut asserts usage tracks per-symbol name length, that
a longer name costs more than a shorter one, and that the blob array is
counted at full CodeBlob size rather than a pointer's.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 ddprof-lib/src/main/cpp/codeCache.cpp    | 22 +++++++++++
 ddprof-lib/src/main/cpp/codeCache.h      | 35 +++++++++++++----
 ddprof-lib/src/test/cpp/codeCache_ut.cpp | 49 ++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 7 deletions(-)
 create mode 100644 ddprof-lib/src/test/cpp/codeCache_ut.cpp

diff --git a/ddprof-lib/src/main/cpp/codeCache.cpp b/ddprof-lib/src/main/cpp/codeCache.cpp
index 5203d5c5d2..d460e5f6b0 100644
--- a/ddprof-lib/src/main/cpp/codeCache.cpp
+++ b/ddprof-lib/src/main/cpp/codeCache.cpp
@@ -144,6 +144,28 @@ CodeCache::~CodeCache() {
   free(_build_id);  // Free build-id memory
 }
 
+long long CodeCache::memoryUsage() {
+  // Blob array. The previous formula used sizeof(CodeBlob*), undercounting the
+  // array roughly threefold (CodeBlob holds three pointers).
+  long long total = (long long)_capacity * sizeof(CodeBlob);
+
+  // This cache's own name, plus each symbol's variable-length name string. The
+  // previous formula approximated these as a fixed sizeof(NativeFunc), ignoring
+  // the name length that dominates the allocation.
+  total += (long long)NativeFunc::allocSize(_name);
+  for (int i = 0; i < _count; i++) {
+    total += (long long)NativeFunc::allocSize(_blobs[i]._name);
+  }
+
+  // DWARF unwind table and build-id string were not counted at all before.
+  total += (long long)_dwarf_table_length * sizeof(FrameDesc);
+  if (_build_id != nullptr) {
+    total += (long long)strlen(_build_id) + 1;
+  }
+
+  return total;
+}
+
 void CodeCache::expand() {
   CodeBlob *old_blobs = _blobs;
   CodeBlob *new_blobs = new CodeBlob[_capacity * 2];
diff --git a/ddprof-lib/src/main/cpp/codeCache.h b/ddprof-lib/src/main/cpp/codeCache.h
index 92b45bf472..d6ef7955a2 100644
--- a/ddprof-lib/src/main/cpp/codeCache.h
+++ b/ddprof-lib/src/main/cpp/codeCache.h
@@ -74,6 +74,15 @@ class NativeFunc {
   static char *create(const char *name, short lib_index);
   static void destroy(char *name);
 
+  // Size of the heap allocation backing a name string produced by create().
+  // Mirrors the size computed there so callers can account for it. 0 if null.
+  static size_t allocSize(const char *name) {
+    if (name == nullptr) {
+      return 0;
+    }
+    return align_up(sizeof(NativeFunc) + 1 + strlen(name), sizeof(NativeFunc *));
+  }
+
   static short libIndex(const char *name) {
     if (name == nullptr) {
       return -1;
@@ -251,9 +260,11 @@ class CodeCache {
   void setDwarfTable(FrameDesc *table, int length, const FrameDesc &default_frame = FrameDesc::default_frame);
   FrameDesc findFrameDesc(const void *pc);
 
-  long long memoryUsage() {
-    return _capacity * sizeof(CodeBlob *) + _count * sizeof(NativeFunc);
-  }
+  // Accurate live size of everything this CodeCache owns on the heap: the blob
+  // array, the per-symbol name strings (variable length), the DWARF unwind
+  // table, the build-id string, and this cache's own name. Recomputed on
+  // demand (called only at dump time), so it always reflects current contents.
+  long long memoryUsage();
 
   int count() { return _count; }
   CodeBlob* blob(int idx) {
@@ -266,11 +277,10 @@ class CodeCacheArray {
   CodeCache *_libs[MAX_NATIVE_LIBS];
   volatile int _reserved;       // next slot to reserve (CAS by writers)
   volatile int _count;          // published count (all indices < _count have non-NULL pointers)
-  volatile size_t _used_memory;
   bool _overflow_reported;
 
 public:
-  CodeCacheArray() : _reserved(0), _count(0), _used_memory(0), _overflow_reported(false) {
+  CodeCacheArray() : _reserved(0), _count(0), _overflow_reported(false) {
     memset(_libs, 0, MAX_NATIVE_LIBS * sizeof(CodeCache *));
   }
 
@@ -296,7 +306,6 @@ class CodeCacheArray {
     } while (!__atomic_compare_exchange_n(&_reserved, &slot, slot + 1,
                                           true, __ATOMIC_RELAXED, __ATOMIC_RELAXED));
     assert(__atomic_load_n(&_libs[slot], __ATOMIC_RELAXED) == nullptr);
-    __atomic_fetch_add(&_used_memory, lib->memoryUsage(), __ATOMIC_RELAXED);
     // Store pointer before publishing count. The RELEASE here pairs with
     // the ACQUIRE load in operator[]/at() and count().
     __atomic_store_n(&_libs[slot], lib, __ATOMIC_RELEASE);
@@ -316,8 +325,20 @@ class CodeCacheArray {
     return __atomic_load_n(&_libs[index], __ATOMIC_ACQUIRE);
   }
 
+  // Sum the live memory of all registered libraries. Recomputed on demand
+  // (called only at dump time) so it tracks each library's later growth,
+  // unlike a value cached at add() time. The array is append-only, so
+  // iterating the published prefix is safe alongside concurrent add()s.
   size_t memoryUsage() const {
-    return __atomic_load_n(&_used_memory, __ATOMIC_RELAXED);
+    size_t total = 0;
+    int n = count();
+    for (int i = 0; i < n; i++) {
+      CodeCache *lib = at(i);
+      if (lib != nullptr) {
+        total += (size_t)lib->memoryUsage();
+      }
+    }
+    return total;
   }
 };
 
diff --git a/ddprof-lib/src/test/cpp/codeCache_ut.cpp b/ddprof-lib/src/test/cpp/codeCache_ut.cpp
new file mode 100644
index 0000000000..039de04b0f
--- /dev/null
+++ b/ddprof-lib/src/test/cpp/codeCache_ut.cpp
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2026, Datadog, Inc.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <gtest/gtest.h>
+#include "codeCache.h"
+#include <cstring>
+
+// memoryUsage() must reflect the actual per-symbol name length, not a fixed
+// per-symbol constant: adding one symbol grows the reported usage by exactly
+// the allocation size of its name string.
+TEST(CodeCacheTest, MemoryUsageCountsSymbolNameLength) {
+    CodeCache cc("testlib");
+    long long base = cc.memoryUsage();
+
+    const char *name =
+        "a_long_symbol_name_well_beyond_sizeof_NativeFunc_padding_xxxxxxxxxxxx";
+    cc.add((const void *)0x1000, 16, name);
+
+    EXPECT_EQ(NativeFunc::allocSize(name), (size_t)(cc.memoryUsage() - base));
+}
+
+// A longer name costs more than a shorter one — i.e. length actually matters,
+// which the old fixed sizeof(NativeFunc) formula could not capture.
+TEST(CodeCacheTest, MemoryUsageGrowsWithNameLength) {
+    CodeCache shortc("lib");
+    CodeCache longc("lib");
+    long long short_base = shortc.memoryUsage();
+    long long long_base = longc.memoryUsage();
+
+    shortc.add((const void *)0x1000, 8, "f");
+    longc.add((const void *)0x1000, 8,
+              "an_extremely_long_symbol_name_padded_out_further_and_further_1234");
+
+    EXPECT_GT(longc.memoryUsage() - long_base, shortc.memoryUsage() - short_base);
+}
+
+// The blob array is counted at the full CodeBlob size, not a pointer's worth
+// (the previous formula used sizeof(CodeBlob*), undercounting ~3x). An empty
+// cache already accounts for capacity * sizeof(CodeBlob) plus its own name.
+TEST(CodeCacheTest, MemoryUsageCountsFullBlobArray) {
+    CodeCache cc("lib");
+    long long usage = cc.memoryUsage();
+    EXPECT_GT(usage, (long long)(INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob)));
+    // Sanity: a pointer-sized count would be far smaller than the real array.
+    EXPECT_GT((long long)(INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob)),
+              (long long)(INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob *)));
+}

From e9ad692cdbed851f358537c868948fe314a47886 Mon Sep 17 00:00:00 2001
From: Roman Kennke <roman.kennke@datadoghq.com>
Date: Tue, 21 Jul 2026 14:07:51 +0200
Subject: [PATCH 2/8] docs(codecache): describe current memoryUsage(), drop
 old-formula references

Comments explained what the previous formula did wrong rather than what
the code now counts. Reword to describe the current behavior only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 ddprof-lib/src/main/cpp/codeCache.cpp | 11 +++++------
 ddprof-lib/src/main/cpp/codeCache.h   | 15 ++++++++-------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/ddprof-lib/src/main/cpp/codeCache.cpp b/ddprof-lib/src/main/cpp/codeCache.cpp
index d460e5f6b0..5f36c0f64a 100644
--- a/ddprof-lib/src/main/cpp/codeCache.cpp
+++ b/ddprof-lib/src/main/cpp/codeCache.cpp
@@ -145,19 +145,18 @@ CodeCache::~CodeCache() {
 }
 
 long long CodeCache::memoryUsage() {
-  // Blob array. The previous formula used sizeof(CodeBlob*), undercounting the
-  // array roughly threefold (CodeBlob holds three pointers).
+  // The blob array: _capacity entries of CodeBlob.
   long long total = (long long)_capacity * sizeof(CodeBlob);
 
-  // This cache's own name, plus each symbol's variable-length name string. The
-  // previous formula approximated these as a fixed sizeof(NativeFunc), ignoring
-  // the name length that dominates the allocation.
+  // This cache's own name, plus each symbol's name string. Each is a
+  // variable-length allocation whose size depends on the name length
+  // (see NativeFunc::allocSize).
   total += (long long)NativeFunc::allocSize(_name);
   for (int i = 0; i < _count; i++) {
     total += (long long)NativeFunc::allocSize(_blobs[i]._name);
   }
 
-  // DWARF unwind table and build-id string were not counted at all before.
+  // The DWARF unwind table and the build-id string, when present.
   total += (long long)_dwarf_table_length * sizeof(FrameDesc);
   if (_build_id != nullptr) {
     total += (long long)strlen(_build_id) + 1;
diff --git a/ddprof-lib/src/main/cpp/codeCache.h b/ddprof-lib/src/main/cpp/codeCache.h
index d6ef7955a2..2b49cb41ee 100644
--- a/ddprof-lib/src/main/cpp/codeCache.h
+++ b/ddprof-lib/src/main/cpp/codeCache.h
@@ -260,10 +260,10 @@ class CodeCache {
   void setDwarfTable(FrameDesc *table, int length, const FrameDesc &default_frame = FrameDesc::default_frame);
   FrameDesc findFrameDesc(const void *pc);
 
-  // Accurate live size of everything this CodeCache owns on the heap: the blob
-  // array, the per-symbol name strings (variable length), the DWARF unwind
-  // table, the build-id string, and this cache's own name. Recomputed on
-  // demand (called only at dump time), so it always reflects current contents.
+  // Live size of everything this CodeCache owns on the heap: the blob array,
+  // the per-symbol name strings (variable length), the DWARF unwind table, the
+  // build-id string, and this cache's own name. Recomputed on demand (called
+  // only at dump time), so it reflects the current contents.
   long long memoryUsage();
 
   int count() { return _count; }
@@ -326,9 +326,10 @@ class CodeCacheArray {
   }
 
   // Sum the live memory of all registered libraries. Recomputed on demand
-  // (called only at dump time) so it tracks each library's later growth,
-  // unlike a value cached at add() time. The array is append-only, so
-  // iterating the published prefix is safe alongside concurrent add()s.
+  // (called only at dump time) so it reflects each library's current size,
+  // including symbols added after the library was registered. The array is
+  // append-only, so iterating the published prefix is safe alongside
+  // concurrent add()s.
   size_t memoryUsage() const {
     size_t total = 0;
     int n = count();

From d68cae23a2cfed7e3158a11641676698f6fd373f Mon Sep 17 00:00:00 2001
From: Roman Kennke <roman.kennke@datadoghq.com>
Date: Tue, 21 Jul 2026 15:56:45 +0200
Subject: [PATCH 3/8] fix(codecache): don't read build-id in memoryUsage()
 (data race); review nits
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address PR review feedback:

- Drop the build-id from memoryUsage(). The background library refresher
  (Libraries::updateBuildIds) calls setBuildId() — which frees and
  replaces _build_id — on already-published caches under _build_id_lock,
  which dump does not hold. The strlen(_build_id) added here could race
  that free/replace (use-after-free / torn read). Build-id is negligible
  (~tens of bytes per library) next to the symbol tables, so excluding it
  keeps the read lock-free at no meaningful accuracy cost. The blob-name
  loop and dwarf-length read are safe: blobs are fixed once a library is
  published (same read the symbolication fast path does) and the dwarf
  read touches only the length scalar.
- Mark CodeCache::memoryUsage() const (it does not mutate state).
- Test: use reinterpret_cast for the integer-to-pointer args and drop the
  unused <cstring> include.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 ddprof-lib/src/main/cpp/codeCache.cpp    | 18 ++++++++++++------
 ddprof-lib/src/main/cpp/codeCache.h      | 13 ++++++++-----
 ddprof-lib/src/test/cpp/codeCache_ut.cpp | 17 ++++++++---------
 3 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/ddprof-lib/src/main/cpp/codeCache.cpp b/ddprof-lib/src/main/cpp/codeCache.cpp
index 5f36c0f64a..a18d8c2e68 100644
--- a/ddprof-lib/src/main/cpp/codeCache.cpp
+++ b/ddprof-lib/src/main/cpp/codeCache.cpp
@@ -144,23 +144,29 @@ CodeCache::~CodeCache() {
   free(_build_id);  // Free build-id memory
 }
 
-long long CodeCache::memoryUsage() {
+long long CodeCache::memoryUsage() const {
   // The blob array: _capacity entries of CodeBlob.
   long long total = (long long)_capacity * sizeof(CodeBlob);
 
   // This cache's own name, plus each symbol's name string. Each is a
   // variable-length allocation whose size depends on the name length
-  // (see NativeFunc::allocSize).
+  // (see NativeFunc::allocSize). Both the name pointers and the array are fixed
+  // once the library is published (the same read the symbolication fast path
+  // does), so this is safe to read at dump time without extra synchronization.
   total += (long long)NativeFunc::allocSize(_name);
   for (int i = 0; i < _count; i++) {
     total += (long long)NativeFunc::allocSize(_blobs[i]._name);
   }
 
-  // The DWARF unwind table and the build-id string, when present.
+  // The DWARF unwind table, when present (length only — no pointer deref).
   total += (long long)_dwarf_table_length * sizeof(FrameDesc);
-  if (_build_id != nullptr) {
-    total += (long long)strlen(_build_id) + 1;
-  }
+
+  // The build-id string is intentionally NOT counted here: the background
+  // library refresher (Libraries::updateBuildIds) frees and replaces _build_id
+  // on already-published caches under _build_id_lock, which dump does not hold,
+  // so dereferencing it here would race. It is negligible (~tens of bytes per
+  // library) next to the symbol tables, so excluding it costs no meaningful
+  // accuracy while keeping this read lock-free.
 
   return total;
 }
diff --git a/ddprof-lib/src/main/cpp/codeCache.h b/ddprof-lib/src/main/cpp/codeCache.h
index 2b49cb41ee..bfa65c6601 100644
--- a/ddprof-lib/src/main/cpp/codeCache.h
+++ b/ddprof-lib/src/main/cpp/codeCache.h
@@ -260,11 +260,14 @@ class CodeCache {
   void setDwarfTable(FrameDesc *table, int length, const FrameDesc &default_frame = FrameDesc::default_frame);
   FrameDesc findFrameDesc(const void *pc);
 
-  // Live size of everything this CodeCache owns on the heap: the blob array,
-  // the per-symbol name strings (variable length), the DWARF unwind table, the
-  // build-id string, and this cache's own name. Recomputed on demand (called
-  // only at dump time), so it reflects the current contents.
-  long long memoryUsage();
+  // Live size of what this CodeCache owns on the heap: the blob array, the
+  // per-symbol name strings (variable length), this cache's own name, and the
+  // DWARF unwind table. Recomputed on demand (called only at dump time), so it
+  // reflects the current contents. The build-id string is deliberately excluded
+  // — it is mutated by the background refresher and negligible in size (see the
+  // definition). Const and lock-free: reads only fields that are stable once the
+  // library is published.
+  long long memoryUsage() const;
 
   int count() { return _count; }
   CodeBlob* blob(int idx) {
diff --git a/ddprof-lib/src/test/cpp/codeCache_ut.cpp b/ddprof-lib/src/test/cpp/codeCache_ut.cpp
index 039de04b0f..a679645573 100644
--- a/ddprof-lib/src/test/cpp/codeCache_ut.cpp
+++ b/ddprof-lib/src/test/cpp/codeCache_ut.cpp
@@ -5,7 +5,6 @@
 
 #include <gtest/gtest.h>
 #include "codeCache.h"
-#include <cstring>
 
 // memoryUsage() must reflect the actual per-symbol name length, not a fixed
 // per-symbol constant: adding one symbol grows the reported usage by exactly
@@ -16,29 +15,29 @@ TEST(CodeCacheTest, MemoryUsageCountsSymbolNameLength) {
 
     const char *name =
         "a_long_symbol_name_well_beyond_sizeof_NativeFunc_padding_xxxxxxxxxxxx";
-    cc.add((const void *)0x1000, 16, name);
+    cc.add(reinterpret_cast<const void *>(0x1000), 16, name);
 
     EXPECT_EQ(NativeFunc::allocSize(name), (size_t)(cc.memoryUsage() - base));
 }
 
-// A longer name costs more than a shorter one — i.e. length actually matters,
-// which the old fixed sizeof(NativeFunc) formula could not capture.
+// A longer name costs more than a shorter one — the reported size scales with
+// the actual symbol-name length.
 TEST(CodeCacheTest, MemoryUsageGrowsWithNameLength) {
     CodeCache shortc("lib");
     CodeCache longc("lib");
     long long short_base = shortc.memoryUsage();
     long long long_base = longc.memoryUsage();
 
-    shortc.add((const void *)0x1000, 8, "f");
-    longc.add((const void *)0x1000, 8,
+    shortc.add(reinterpret_cast<const void *>(0x1000), 8, "f");
+    longc.add(reinterpret_cast<const void *>(0x1000), 8,
               "an_extremely_long_symbol_name_padded_out_further_and_further_1234");
 
     EXPECT_GT(longc.memoryUsage() - long_base, shortc.memoryUsage() - short_base);
 }
 
-// The blob array is counted at the full CodeBlob size, not a pointer's worth
-// (the previous formula used sizeof(CodeBlob*), undercounting ~3x). An empty
-// cache already accounts for capacity * sizeof(CodeBlob) plus its own name.
+// The blob array is counted at the full CodeBlob size (a CodeBlob is several
+// pointers wide): an empty cache already accounts for
+// capacity * sizeof(CodeBlob) plus its own name.
 TEST(CodeCacheTest, MemoryUsageCountsFullBlobArray) {
     CodeCache cc("lib");
     long long usage = cc.memoryUsage();

From 7d4494950cf4bd642fd77e523b30554714d821f0 Mon Sep 17 00:00:00 2001
From: Roman Kennke <roman.kennke@datadoghq.com>
Date: Wed, 22 Jul 2026 15:12:56 +0200
Subject: [PATCH 4/8] harden(codecache): enforce/scope the
 published-immutability invariant
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address the Sphinx review on memoryUsage()'s lock-free read:

- Make memoryUsage()'s lock-free safety an enforced precondition. Add a
  _published flag, set by CodeCacheArray::add() before the pointer is
  published, and assert !_published in add()/expand()/setDwarfTable() —
  the mutators that touch the _blobs array and _dwarf_table_length that
  memoryUsage() reads from the dump thread. Standalone caches that are
  never registered (e.g. Libraries::_runtime_stubs, which is continuously
  mutated under its own lock) stay unpublished and may be mutated freely.
- Scope the safety comment accordingly: the lock-free read is valid only
  for array-registered caches, and must not be used on _runtime_stubs
  without that cache's lock.
- NativeFunc::create() now calls allocSize() instead of recomputing the
  size formula, so memoryUsage() and the allocation can't drift.
- Reword CodeCacheArray::memoryUsage() to drop the incorrect
  "reflects symbols added after registration" claim (adds happen
  pre-publish); it reflects each library's fully-populated state at dump.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 ddprof-lib/src/main/cpp/codeCache.cpp | 29 +++++++++++++++++++++++----
 ddprof-lib/src/main/cpp/codeCache.h   | 29 ++++++++++++++++++++++-----
 2 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/ddprof-lib/src/main/cpp/codeCache.cpp b/ddprof-lib/src/main/cpp/codeCache.cpp
index a18d8c2e68..1e014380ec 100644
--- a/ddprof-lib/src/main/cpp/codeCache.cpp
+++ b/ddprof-lib/src/main/cpp/codeCache.cpp
@@ -9,13 +9,14 @@
 #include "os.h"
 #include "safeAccess.h"
 
+#include <cassert>
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/mman.h>
 
 char *NativeFunc::create(const char *name, short lib_index) {
-  size_t size = align_up(sizeof(NativeFunc) + 1 + strlen(name), sizeof(NativeFunc*));
+  size_t size = allocSize(name);
   NativeFunc *f = (NativeFunc *)aligned_alloc(sizeof(NativeFunc*), size);
   f->_lib_index = lib_index;
   f->_mark = 0;
@@ -69,6 +70,8 @@ CodeCache::CodeCache(const char *name, short lib_index,
   _capacity = INITIAL_CODE_CACHE_CAPACITY;
   _count = 0;
   _blobs = new CodeBlob[_capacity];
+
+  _published = false;
 }
 
 void CodeCache::copyFrom(const CodeCache& other) {
@@ -113,6 +116,9 @@ void CodeCache::copyFrom(const CodeCache& other) {
   _count = other._count;
   _blobs = new CodeBlob[_capacity];
   memcpy(_blobs, other._blobs, _count * sizeof(CodeBlob));
+
+  // A copy is a fresh, not-yet-registered cache.
+  _published = false;
 }
 
 CodeCache::CodeCache(const CodeCache &other) {
@@ -150,9 +156,13 @@ long long CodeCache::memoryUsage() const {
 
   // This cache's own name, plus each symbol's name string. Each is a
   // variable-length allocation whose size depends on the name length
-  // (see NativeFunc::allocSize). Both the name pointers and the array are fixed
-  // once the library is published (the same read the symbolication fast path
-  // does), so this is safe to read at dump time without extra synchronization.
+  // (see NativeFunc::allocSize). The lock-free read here is safe only for caches
+  // registered in a CodeCacheArray (Libraries::native_libs): their _blobs array
+  // and name pointers are fixed once published (add()/expand()/setDwarfTable()
+  // run only pre-publish — asserted via _published), which is the same read the
+  // symbolication fast path relies on. It must NOT be called on a continuously
+  // mutated, unpublished cache such as Libraries::_runtime_stubs without holding
+  // that cache's lock, since a concurrent expand() would free _blobs underneath.
   total += (long long)NativeFunc::allocSize(_name);
   for (int i = 0; i < _count; i++) {
     total += (long long)NativeFunc::allocSize(_blobs[i]._name);
@@ -172,6 +182,9 @@ long long CodeCache::memoryUsage() const {
 }
 
 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 && "expand() on a published CodeCache races memoryUsage()");
   CodeBlob *old_blobs = _blobs;
   CodeBlob *new_blobs = new CodeBlob[_capacity * 2];
 
@@ -184,6 +197,11 @@ void CodeCache::expand() {
 
 void CodeCache::add(const void *start, int length, const char *name,
                     bool update_bounds) {
+  // 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 && "add() on a published CodeCache races memoryUsage()");
   char *name_copy = NativeFunc::create(name, _lib_index);
   // Replace non-printable characters
   for (char *s = name_copy; *s != 0; s++) {
@@ -441,6 +459,9 @@ void CodeCache::makeImportsPatchable() {
 }
 
 void CodeCache::setDwarfTable(FrameDesc *table, int length, const FrameDesc &default_frame) {
+  // Set during library parsing, before publication. memoryUsage() reads
+  // _dwarf_table_length lock-free at dump time, so this must not run afterwards.
+  assert(!_published && "setDwarfTable() on a published CodeCache races memoryUsage()");
   _dwarf_table = table;
   _dwarf_table_length = length;
   _default_frame = &default_frame;
diff --git a/ddprof-lib/src/main/cpp/codeCache.h b/ddprof-lib/src/main/cpp/codeCache.h
index bfa65c6601..cbf850c1f8 100644
--- a/ddprof-lib/src/main/cpp/codeCache.h
+++ b/ddprof-lib/src/main/cpp/codeCache.h
@@ -75,7 +75,8 @@ class NativeFunc {
   static void destroy(char *name);
 
   // Size of the heap allocation backing a name string produced by create().
-  // Mirrors the size computed there so callers can account for it. 0 if null.
+  // This is the single source of truth for that size: create() allocates
+  // exactly allocSize(name) bytes, and memoryUsage() accounts for it. 0 if null.
   static size_t allocSize(const char *name) {
     if (name == nullptr) {
       return 0;
@@ -162,6 +163,14 @@ class CodeCache {
   int _count;
   CodeBlob *_blobs;
 
+  // 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
+  // are never published (e.g. Libraries::_runtime_stubs) stay unpublished and
+  // may be mutated freely.
+  bool _published;
+
   void expand();
   void makeImportsPatchable();
   void saveImport(ImportId id, void** entry);
@@ -218,6 +227,11 @@ class CodeCache {
   void setBuildId(const char* build_id, size_t build_id_len);
   void setLoadBias(uintptr_t load_bias) { _load_bias = load_bias; }
 
+  // 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).
+  void markPublished() { _published = true; }
+
   void add(const void *start, int length, const char *name,
            bool update_bounds = false);
   void updateBounds(const void *start, const void *end);
@@ -309,6 +323,10 @@ class CodeCacheArray {
     } while (!__atomic_compare_exchange_n(&_reserved, &slot, slot + 1,
                                           true, __ATOMIC_RELAXED, __ATOMIC_RELAXED));
     assert(__atomic_load_n(&_libs[slot], __ATOMIC_RELAXED) == nullptr);
+    // Mark published before the RELEASE store makes the pointer visible, so any
+    // later add()/expand()/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
     // the ACQUIRE load in operator[]/at() and count().
     __atomic_store_n(&_libs[slot], lib, __ATOMIC_RELEASE);
@@ -329,10 +347,11 @@ class CodeCacheArray {
   }
 
   // Sum the live memory of all registered libraries. Recomputed on demand
-  // (called only at dump time) so it reflects each library's current size,
-  // including symbols added after the library was registered. The array is
-  // append-only, so iterating the published prefix is safe alongside
-  // concurrent add()s.
+  // (called only at dump time) so it reflects each library's fully-populated
+  // state at that point — the accurate per-library formula rather than a value
+  // cached at add() time. (Libraries are populated before being registered and
+  // not grown afterwards.) The array is append-only, so iterating the published
+  // prefix is safe alongside concurrent add()s.
   size_t memoryUsage() const {
     size_t total = 0;
     int n = count();

From cc9e523733d608cd6079c4214719745cba7f4be2 Mon Sep 17 00:00:00 2001
From: Roman Kennke <roman.kennke@datadoghq.com>
Date: Wed, 22 Jul 2026 15:12:58 +0200
Subject: [PATCH 5/8] test(codecache): cover array aggregation, DWARF term,
 exact blob-array size
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address the Sphinx review's test-coverage gaps:

- ArrayMemoryUsageSumsLibraries: exercises CodeCacheArray::memoryUsage()
  (the aggregation production consumes via Profiler::dump()), which no
  test covered — guards the loop and accumulation.
- MemoryUsageCountsDwarfTable: populates a DWARF table via setDwarfTable()
  so the length * sizeof(FrameDesc) term is non-zero and a regression in
  it is detectable.
- MemoryUsageCountsFullBlobArray: tightened from EXPECT_GT to an exact
  EXPECT_EQ (blob array + own name) so over-counting would fail.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 ddprof-lib/src/test/cpp/codeCache_ut.cpp | 51 ++++++++++++++++++++----
 1 file changed, 43 insertions(+), 8 deletions(-)

diff --git a/ddprof-lib/src/test/cpp/codeCache_ut.cpp b/ddprof-lib/src/test/cpp/codeCache_ut.cpp
index a679645573..4910c918ec 100644
--- a/ddprof-lib/src/test/cpp/codeCache_ut.cpp
+++ b/ddprof-lib/src/test/cpp/codeCache_ut.cpp
@@ -35,14 +35,49 @@ TEST(CodeCacheTest, MemoryUsageGrowsWithNameLength) {
     EXPECT_GT(longc.memoryUsage() - long_base, shortc.memoryUsage() - short_base);
 }
 
-// The blob array is counted at the full CodeBlob size (a CodeBlob is several
-// pointers wide): an empty cache already accounts for
-// capacity * sizeof(CodeBlob) plus its own name.
+// A freshly-constructed, empty cache reports exactly the blob array (counted at
+// the full CodeBlob size, not a pointer's worth) plus its own name — nothing
+// more. An exact check so over-counting or an inflated constant would fail.
 TEST(CodeCacheTest, MemoryUsageCountsFullBlobArray) {
     CodeCache cc("lib");
-    long long usage = cc.memoryUsage();
-    EXPECT_GT(usage, (long long)(INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob)));
-    // Sanity: a pointer-sized count would be far smaller than the real array.
-    EXPECT_GT((long long)(INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob)),
-              (long long)(INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob *)));
+    EXPECT_EQ((long long)INITIAL_CODE_CACHE_CAPACITY * sizeof(CodeBlob)
+                  + (long long)NativeFunc::allocSize("lib"),
+              cc.memoryUsage());
+}
+
+// The DWARF unwind table contributes length * sizeof(FrameDesc). Populate it so
+// a regression (e.g. * flipped to /) in that term is caught; the table is
+// zero-length in the other tests.
+TEST(CodeCacheTest, MemoryUsageCountsDwarfTable) {
+    CodeCache cc("lib");
+    long long base = cc.memoryUsage();
+
+    const int entries = 7;
+    // setDwarfTable() takes ownership; ~CodeCache frees it. Contents are
+    // irrelevant here — memoryUsage() reads only the length.
+    FrameDesc *table = (FrameDesc *)malloc(entries * sizeof(FrameDesc));
+    cc.setDwarfTable(table, entries);
+
+    EXPECT_EQ((long long)entries * sizeof(FrameDesc), cc.memoryUsage() - base);
+}
+
+// CodeCacheArray::memoryUsage() is the aggregation production actually consumes
+// (Profiler::dump()). Verify it sums the per-library values — guards the loop
+// bound and the accumulation against regressions the per-instance tests miss.
+TEST(CodeCacheTest, ArrayMemoryUsageSumsLibraries) {
+    CodeCacheArray arr;
+    CodeCache *a = new CodeCache("liba");
+    CodeCache *b = new CodeCache("libb");
+    // Populate before registering: add() must run pre-publication.
+    a->add(reinterpret_cast<const void *>(0x1000), 16, "alpha_symbol");
+    b->add(reinterpret_cast<const void *>(0x2000), 16, "beta_symbol_longer_name");
+
+    long long expected = (long long)a->memoryUsage() + (long long)b->memoryUsage();
+    ASSERT_TRUE(arr.add(a));
+    ASSERT_TRUE(arr.add(b));
+    EXPECT_EQ(expected, (long long)arr.memoryUsage());
+
+    // CodeCacheArray does not own the entries.
+    delete a;
+    delete b;
 }

From 90523b953b3e9eb20a712295cc8fe75d54760e70 Mon Sep 17 00:00:00 2001
From: Roman Kennke <roman.kennke@datadoghq.com>
Date: Wed, 22 Jul 2026 15:25:58 +0200
Subject: [PATCH 6/8] docs(codecache): reference JitCodeCache::_runtime_stubs,
 not Libraries::

The comments on the lock-free memoryUsage() read named
Libraries::_runtime_stubs (a vestigial, unused member) as the example of
a continuously-mutated unpublished cache. The actual one is
JitCodeCache::_runtime_stubs, mutated under JitCodeCache::_stubs_lock
(add() under lock(), findRuntimeStub() under lockShared()). Fix the
references.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 ddprof-lib/src/main/cpp/codeCache.cpp | 5 +++--
 ddprof-lib/src/main/cpp/codeCache.h   | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/ddprof-lib/src/main/cpp/codeCache.cpp b/ddprof-lib/src/main/cpp/codeCache.cpp
index 1e014380ec..52dff98843 100644
--- a/ddprof-lib/src/main/cpp/codeCache.cpp
+++ b/ddprof-lib/src/main/cpp/codeCache.cpp
@@ -161,8 +161,9 @@ long long CodeCache::memoryUsage() const {
   // and name pointers are fixed once published (add()/expand()/setDwarfTable()
   // run only pre-publish — asserted via _published), which is the same read the
   // symbolication fast path relies on. It must NOT be called on a continuously
-  // mutated, unpublished cache such as Libraries::_runtime_stubs without holding
-  // that cache's lock, since a concurrent expand() would free _blobs underneath.
+  // mutated, unpublished cache such as JitCodeCache::_runtime_stubs without
+  // holding JitCodeCache::_stubs_lock (shared), since a concurrent add()/expand()
+  // would free _blobs underneath the reader.
   total += (long long)NativeFunc::allocSize(_name);
   for (int i = 0; i < _count; i++) {
     total += (long long)NativeFunc::allocSize(_blobs[i]._name);
diff --git a/ddprof-lib/src/main/cpp/codeCache.h b/ddprof-lib/src/main/cpp/codeCache.h
index cbf850c1f8..ecd9f7c6b6 100644
--- a/ddprof-lib/src/main/cpp/codeCache.h
+++ b/ddprof-lib/src/main/cpp/codeCache.h
@@ -167,8 +167,9 @@ class CodeCache {
   // 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
-  // are never published (e.g. Libraries::_runtime_stubs) stay unpublished and
-  // may be mutated freely.
+  // are never registered (e.g. JitCodeCache::_runtime_stubs, which is instead
+  // guarded by its own JitCodeCache::_stubs_lock) stay unpublished, so the
+  // asserts never fire for them.
   bool _published;
 
   void expand();

From 906d445e4023ae71f8e31e2aa13c9af14d4598a5 Mon Sep 17 00:00:00 2001
From: Roman Kennke <roman.kennke@datadoghq.com>
Date: Wed, 22 Jul 2026 17:06:20 +0200
Subject: [PATCH 7/8] harden(codecache): make _published atomic
 (release/acquire)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addressing review feedback on visibility: _published was a plain bool
read/written across threads (set by CodeCacheArray::add(), read by the
add()/expand()/setDwarfTable() asserts). Make it std::atomic<bool> with a
release store in markPublished() and acquire loads in the asserts. The
store is sequenced before add()'s RELEASE publish of the pointer, so any
thread that reaches a published cache observes _published == true — now
without a formal data race and TSan-clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 ddprof-lib/src/main/cpp/codeCache.cpp | 13 ++++++++-----
 ddprof-lib/src/main/cpp/codeCache.h   | 10 +++++++---
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/ddprof-lib/src/main/cpp/codeCache.cpp b/ddprof-lib/src/main/cpp/codeCache.cpp
index 52dff98843..9105ed0831 100644
--- a/ddprof-lib/src/main/cpp/codeCache.cpp
+++ b/ddprof-lib/src/main/cpp/codeCache.cpp
@@ -71,7 +71,7 @@ CodeCache::CodeCache(const char *name, short lib_index,
   _count = 0;
   _blobs = new CodeBlob[_capacity];
 
-  _published = false;
+  _published.store(false, std::memory_order_relaxed);
 }
 
 void CodeCache::copyFrom(const CodeCache& other) {
@@ -118,7 +118,7 @@ void CodeCache::copyFrom(const CodeCache& other) {
   memcpy(_blobs, other._blobs, _count * sizeof(CodeBlob));
 
   // A copy is a fresh, not-yet-registered cache.
-  _published = false;
+  _published.store(false, std::memory_order_relaxed);
 }
 
 CodeCache::CodeCache(const CodeCache &other) {
@@ -185,7 +185,8 @@ long long CodeCache::memoryUsage() const {
 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 && "expand() on a published CodeCache races memoryUsage()");
+  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];
 
@@ -202,7 +203,8 @@ void CodeCache::add(const void *start, int length, const char *name,
   // 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 && "add() on a published CodeCache races memoryUsage()");
+  assert(!_published.load(std::memory_order_acquire) &&
+         "add() on a published CodeCache races memoryUsage()");
   char *name_copy = NativeFunc::create(name, _lib_index);
   // Replace non-printable characters
   for (char *s = name_copy; *s != 0; s++) {
@@ -462,7 +464,8 @@ void CodeCache::makeImportsPatchable() {
 void CodeCache::setDwarfTable(FrameDesc *table, int length, const FrameDesc &default_frame) {
   // Set during library parsing, before publication. memoryUsage() reads
   // _dwarf_table_length lock-free at dump time, so this must not run afterwards.
-  assert(!_published && "setDwarfTable() on a published CodeCache races memoryUsage()");
+  assert(!_published.load(std::memory_order_acquire) &&
+         "setDwarfTable() on a published CodeCache races memoryUsage()");
   _dwarf_table = table;
   _dwarf_table_length = length;
   _default_frame = &default_frame;
diff --git a/ddprof-lib/src/main/cpp/codeCache.h b/ddprof-lib/src/main/cpp/codeCache.h
index ecd9f7c6b6..ab52916863 100644
--- a/ddprof-lib/src/main/cpp/codeCache.h
+++ b/ddprof-lib/src/main/cpp/codeCache.h
@@ -12,6 +12,7 @@
 #include "dwarf.h"
 #include "utils.h"
 
+#include <atomic>
 #include <jvmti.h>
 #include <stdlib.h>
 #include <string.h>
@@ -169,8 +170,11 @@ class CodeCache {
   // 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
-  // asserts never fire for them.
-  bool _published;
+  // 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();
@@ -231,7 +235,7 @@ class CodeCache {
   // 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).
-  void markPublished() { _published = true; }
+  void markPublished() { _published.store(true, std::memory_order_release); }
 
   void add(const void *start, int length, const char *name,
            bool update_bounds = false);

From 9800ad34e86f24d383d968166f335333306e8d8e Mon Sep 17 00:00:00 2001
From: Roman Kennke <roman.kennke@datadoghq.com>
Date: Wed, 22 Jul 2026 17:14:10 +0200
Subject: [PATCH 8/8] fix(codecache): deep-copy blob name strings in copyFrom()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

copyFrom() memcpy'd the CodeBlob array, which copied each blob's _name
*pointer* — so a copied CodeCache shared name allocations with the
original and both destructors freed them (double-free / use-after-free).
Give the copy its own name strings via NativeFunc::create(), matching how
build-id and the DWARF table are already deep-copied.

(CodeCache is only ever used by pointer today, so the copy path is
currently unexercised — this hardens it rather than fixing an observed
crash.) Adds CopyIsDeepAndDoesNotDoubleFree, which copies a populated
cache and destroys both, aborting on the old shallow-copy behavior.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 ddprof-lib/src/main/cpp/codeCache.cpp    |  8 ++++++++
 ddprof-lib/src/test/cpp/codeCache_ut.cpp | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/ddprof-lib/src/main/cpp/codeCache.cpp b/ddprof-lib/src/main/cpp/codeCache.cpp
index 9105ed0831..ab96ee112d 100644
--- a/ddprof-lib/src/main/cpp/codeCache.cpp
+++ b/ddprof-lib/src/main/cpp/codeCache.cpp
@@ -116,6 +116,14 @@ void CodeCache::copyFrom(const CodeCache& other) {
   _count = other._count;
   _blobs = new CodeBlob[_capacity];
   memcpy(_blobs, other._blobs, _count * sizeof(CodeBlob));
+  // The memcpy above copied each CodeBlob's _name *pointer*, so without this the
+  // two caches would share name allocations and both destructors would free
+  // them (double-free / use-after-free). Give this copy its own name strings.
+  for (int i = 0; i < _count; i++) {
+    if (_blobs[i]._name != nullptr) {
+      _blobs[i]._name = NativeFunc::create(other._blobs[i]._name, _lib_index);
+    }
+  }
 
   // A copy is a fresh, not-yet-registered cache.
   _published.store(false, std::memory_order_relaxed);
diff --git a/ddprof-lib/src/test/cpp/codeCache_ut.cpp b/ddprof-lib/src/test/cpp/codeCache_ut.cpp
index 4910c918ec..1183473316 100644
--- a/ddprof-lib/src/test/cpp/codeCache_ut.cpp
+++ b/ddprof-lib/src/test/cpp/codeCache_ut.cpp
@@ -61,6 +61,24 @@ TEST(CodeCacheTest, MemoryUsageCountsDwarfTable) {
     EXPECT_EQ((long long)entries * sizeof(FrameDesc), cc.memoryUsage() - base);
 }
 
+// Copying a CodeCache must deep-copy each blob's name string. With the previous
+// shallow memcpy the copy shared the originals' _name pointers, so destroying
+// both double-freed them. Here both copies are populated, sized equally, and
+// destroyed — a double-free would abort (or trip ASan).
+TEST(CodeCacheTest, CopyIsDeepAndDoesNotDoubleFree) {
+    CodeCache src("lib");
+    src.add(reinterpret_cast<const void *>(0x1000), 16, "sym_one");
+    src.add(reinterpret_cast<const void *>(0x2000), 16, "sym_two_longer_name");
+    long long expected = src.memoryUsage();
+
+    {
+        CodeCache copy(src);
+        EXPECT_EQ(expected, copy.memoryUsage());
+    }  // copy destroyed — must not free src's name strings
+
+    EXPECT_EQ(expected, src.memoryUsage());  // src's names still intact
+}  // src destroyed — no double free
+
 // CodeCacheArray::memoryUsage() is the aggregation production actually consumes
 // (Profiler::dump()). Verify it sums the per-library values — guards the loop
 // bound and the accumulation against regressions the per-instance tests miss.