From 26ec669ce972f91c1d70b9c491a189313387ba3c Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Wed, 28 Oct 2026 15:57:34 +0100 Subject: [PATCH 1/9] Resolve mallinfo2 at runtime so malloc arena counters survive old build headers (PROF-16350) --- ddprof-lib/src/main/cpp/flightRecorder.cpp | 34 ++++----- ddprof-lib/src/main/cpp/flightRecorder.h | 8 +- ddprof-lib/src/main/cpp/mallocInfo.cpp | 86 ++++++++++++++++++++++ ddprof-lib/src/main/cpp/mallocInfo.h | 33 +++++++++ 4 files changed, 141 insertions(+), 20 deletions(-) create mode 100644 ddprof-lib/src/main/cpp/mallocInfo.cpp create mode 100644 ddprof-lib/src/main/cpp/mallocInfo.h diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index cb1acc0a0e..80a9938aac 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -8,15 +8,6 @@ #include #include -// mallinfo2() replaced the int-based mallinfo() in glibc 2.33; the older struct -// silently truncates past 2 GiB, so it is not a usable fallback for byte -// accounting. Absent on musl and macOS, where the arena counters stay zero. -#if defined(__linux__) && defined(__GLIBC__) && \ - (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 33)) - #include - #define DD_HAVE_MALLINFO2 1 -#endif - #include "buffers.h" #include "callTraceHashTable.h" #include "context.h" @@ -29,6 +20,7 @@ #include "jfrMetadata.h" #include "jniHelper.h" #include "jvmSupport.inline.h" +#include "mallocInfo.h" #include "os.h" #include "profiler.h" #include "signalSafety.h" @@ -2141,8 +2133,8 @@ void Recording::writeLogLevels(Buffer *buf) { // mallinfo2(), which then reports all zeros. Neither runtime exports // mallctl/tc_malloc_size, so the dlsym probe alone would miss them. // -// Not cached: called once per JFR chunk flush, so two dlsym() lookups are -// negligible. +// Not cached: called once per JFR chunk flush, so the dlsym() lookups (three +// counting the mallinfo2 resolution in mallocInfo.cpp) are negligible. bool Recording::glibcMallocActive() { #if defined(ASAN_ENABLED) || defined(TSAN_ENABLED) return false; @@ -2166,17 +2158,26 @@ bool Recording::glibcMallocActive() { // from the sampling signal handler. Once per JFR chunk is negligible. // // Left unset (rather than published as zero) when a replacement allocator is -// active: see glibcMallocActive() above. +// active: see glibcMallocActive() above. Also left unset when the runtime +// allocator does not provide mallinfo2() (glibc < 2.33, musl, macOS) -- the +// symbol is resolved at runtime, so a build made against old headers still +// picks it up from a newer runtime glibc (see mallocInfo.cpp). void Recording::updateMallocArenaStats() { -#ifdef DD_HAVE_MALLINFO2 if (!glibcMallocActive()) { return; } - struct mallinfo2 mi = mallinfo2(); - // arena: bytes obtained from the OS via brk, excluding mmap'd chunks + MallocArenaInfo mi; + if (!mallocArenaSnapshot(&mi)) { + return; + } + // arena: system memory held by glibc's arenas, summed per arena + // (glibc int_mallinfo: m->arena += av->system_mem) -- NOT just + // the main sbrk heap; non-main arenas live on their own mmap'd + // heaps, so arena growth tracks per-thread arenas too // uordblks: bytes currently handed out to callers // fordblks: bytes free but retained in the arenas -- the waste term - // keepcost: the trimmable top block, i.e. what malloc_trim could return; + // keepcost: the main arena's top chunk only (glibc sets it under + // `if (av == &main_arena)`), i.e. what malloc_trim could return; // separating it distinguishes trim-threshold policy from genuine // fragmentation, which trimming cannot reclaim // hblkhd: bytes in mmap'd chunks, which are returned to the OS on free @@ -2185,7 +2186,6 @@ void Recording::updateMallocArenaStats() { Counters::set(MALLOC_FREE_HELD_BYTES, (long long)mi.fordblks); Counters::set(MALLOC_TRIMMABLE_BYTES, (long long)mi.keepcost); Counters::set(MALLOC_MMAP_BYTES, (long long)mi.hblkhd); -#endif } void Recording::capturePostFlushNativeMem() { diff --git a/ddprof-lib/src/main/cpp/flightRecorder.h b/ddprof-lib/src/main/cpp/flightRecorder.h index 8b8de10a95..48283e29b5 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.h +++ b/ddprof-lib/src/main/cpp/flightRecorder.h @@ -444,9 +444,11 @@ class Recording { // updateMallocArenaStats() for why this matters. Exposed for // RecordingTestAccessor. static bool glibcMallocActive(); - // Process-wide malloc arena state (glibc mallinfo2). Flush path only -- - // takes every arena lock, so not async-signal-safe. No-op off glibc 2.33+, - // and also when glibcMallocActive() reports a replacement allocator. + // Process-wide malloc arena state (glibc mallinfo2, resolved at runtime -- + // see mallocInfo.h). Flush path only -- takes every arena lock, so not + // async-signal-safe. No-op when the runtime allocator does not provide + // mallinfo2() (glibc < 2.33, musl, macOS), and also when + // glibcMallocActive() reports a replacement allocator. void updateMallocArenaStats(); void writeNativeMem(Buffer *buf); diff --git a/ddprof-lib/src/main/cpp/mallocInfo.cpp b/ddprof-lib/src/main/cpp/mallocInfo.cpp new file mode 100644 index 0000000000..9b6d4d8361 --- /dev/null +++ b/ddprof-lib/src/main/cpp/mallocInfo.cpp @@ -0,0 +1,86 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "mallocInfo.h" +#include + +// mallinfo2() replaced the int-based mallinfo() in glibc 2.33. The release +// libraries are built against older glibc headers (the el8 image has 2.28), +// where declares neither mallinfo2() nor its struct -- so instead +// of gating the call on __GLIBC_MINOR__ at compile time, which compiled the +// arena counters out of every shipped build, the symbol is resolved at +// runtime with dlsym(). A dlsym string lookup adds no symbol-version +// reference, so the binary keeps loading on runtimes older than 2.33: there +// dlsym returns NULL and the counters stay unset, exactly as on musl and +// macOS where the symbol does not exist either. +// +// The mirror must match the real struct field for field: mallinfo2() is +// called through this type, and the return-by-value ABI (hidden sret pointer +// vs registers) is selected from it. Ten size_t fields, same order as the +// old struct mallinfo; verified against glibc 2.33+ . +struct Mallinfo2 { + size_t arena; + size_t ordblks; + size_t smblks; + size_t hblks; + size_t hblkhd; + size_t usmblks; + size_t fsmblks; + size_t uordblks; + size_t fordblks; + size_t keepcost; +}; + +#if defined(__GLIBC__) && defined(__GLIBC_PREREQ) + #if __GLIBC_PREREQ(2, 33) + #include + #include + // On builds whose headers do declare struct mallinfo2, the mirror is + // pinned to it field by field: every one of the ten offsets, not just the + // five mallocArenaSnapshot() copies out (arena, hblkhd, uordblks, + // fordblks, keepcost). On older headers -- including the el8 release + // image (glibc 2.28), the builds this path exists for -- struct mallinfo2 + // is not declared, so this pin cannot run and the mirror is the only + // definition. There the field mapping is cross-checked at runtime by + // mallocArenaStats_ut.cpp wherever the test runtime provides mallinfo2: + // glibc's accounting identity (arena == uordblks + fordblks, keepcost <= + // fordblks), a large mmapped chunk moving only hblkhd, signed in-use + // deltas across an allocation burst, and post-trim keepcost bounds. On + // runtimes older than 2.33 those tests skip. + static_assert(sizeof(Mallinfo2) == sizeof(struct mallinfo2), + "Mallinfo2 mirror does not match glibc's struct mallinfo2"); + #define DDPROF_PIN_MALLINFO2_FIELD(f) \ + static_assert(offsetof(Mallinfo2, f) == offsetof(struct mallinfo2, f), \ + "Mallinfo2::" #f " offset diverges from glibc's struct mallinfo2") + DDPROF_PIN_MALLINFO2_FIELD(arena); + DDPROF_PIN_MALLINFO2_FIELD(ordblks); + DDPROF_PIN_MALLINFO2_FIELD(smblks); + DDPROF_PIN_MALLINFO2_FIELD(hblks); + DDPROF_PIN_MALLINFO2_FIELD(hblkhd); + DDPROF_PIN_MALLINFO2_FIELD(usmblks); + DDPROF_PIN_MALLINFO2_FIELD(fsmblks); + DDPROF_PIN_MALLINFO2_FIELD(uordblks); + DDPROF_PIN_MALLINFO2_FIELD(fordblks); + DDPROF_PIN_MALLINFO2_FIELD(keepcost); + #undef DDPROF_PIN_MALLINFO2_FIELD + #endif +#endif + +bool mallocArenaSnapshot(MallocArenaInfo *out) { + typedef Mallinfo2 (*mallinfo2_fn)(); + // Not cached: called once per JFR chunk flush, so the dlsym() lookup is + // negligible (same trade-off as glibcMallocActive()). + mallinfo2_fn fn = (mallinfo2_fn)dlsym(RTLD_DEFAULT, "mallinfo2"); + if (fn == NULL) { + return false; + } + Mallinfo2 mi = fn(); + out->arena = mi.arena; + out->hblkhd = mi.hblkhd; + out->uordblks = mi.uordblks; + out->fordblks = mi.fordblks; + out->keepcost = mi.keepcost; + return true; +} diff --git a/ddprof-lib/src/main/cpp/mallocInfo.h b/ddprof-lib/src/main/cpp/mallocInfo.h new file mode 100644 index 0000000000..757c7c7f08 --- /dev/null +++ b/ddprof-lib/src/main/cpp/mallocInfo.h @@ -0,0 +1,33 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef MALLOC_INFO_H +#define MALLOC_INFO_H + +#include + +// The process-wide malloc arena figures the profiler publishes, in the units +// glibc's allocator accounting reports them (see mallocArenaSnapshot()). +struct MallocArenaInfo { + size_t arena; // system memory held by all arenas, summed per arena + size_t hblkhd; // bytes in mmap'd chunks + size_t uordblks; // bytes currently handed out to callers + size_t fordblks; // bytes free but retained inside the arenas + size_t keepcost; // the main arena's top chunk only +}; + +// Snapshot the process-wide glibc malloc arena state via mallinfo2(), resolved +// at runtime with dlsym() so a binary built against old glibc headers (the el8 +// image ships glibc 2.28 headers) still picks mallinfo2() up from a newer +// runtime glibc. Fills *out and returns true on success; returns false and +// leaves *out untouched when the runtime allocator does not provide +// mallinfo2() (glibc < 2.33 at runtime, musl, macOS) -- callers leave the +// arena counters unset, exactly as they already do on musl. +// +// NOT async-signal-safe: mallinfo2() walks every arena taking each arena lock. +// Flush path only -- must never be reached from the sampling signal handler. +bool mallocArenaSnapshot(MallocArenaInfo *out); + +#endif // MALLOC_INFO_H From 35fefc44ec5a2fa6acfc899976017dabe6458e74 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Wed, 28 Oct 2026 15:57:41 +0100 Subject: [PATCH 2/9] Run arena counter tests against the runtime-resolved mallinfo2 path; add functional counter test (PROF-16350) --- .../src/test/cpp/mallocArenaStats_ut.cpp | 196 +++++++++++++++--- .../nativemem/MallocArenaCountersTest.java | 178 ++++++++++++++++ 2 files changed, 349 insertions(+), 25 deletions(-) create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java diff --git a/ddprof-lib/src/test/cpp/mallocArenaStats_ut.cpp b/ddprof-lib/src/test/cpp/mallocArenaStats_ut.cpp index 043d3971d9..d5392a4601 100644 --- a/ddprof-lib/src/test/cpp/mallocArenaStats_ut.cpp +++ b/ddprof-lib/src/test/cpp/mallocArenaStats_ut.cpp @@ -5,15 +5,27 @@ #include #include +#include #include #include #include "flightRecorder.h" +#include "mallocInfo.h" -#if defined(__linux__) && defined(__GLIBC__) && \ - (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 33)) - #include - #define DD_HAVE_MALLINFO2 1 +// malloc_trim is a glibc extension: musl also defines __linux__, but its +// does not declare malloc_trim, so the guard must be __GLIBC__. +// __GLIBC__ comes from , included explicitly so the guard does not +// depend on another header pulling it in transitively. Gtest musl builds always +// pass -D__musl__ (GtestTaskBuilder), so the #error fires only when a Linux +// build has lost __GLIBC__ (e.g. the features.h include was removed or moved +// below the guard), instead of silently compiling trimArenas() to a no-op. +#if defined(__linux__) + #include // defines __GLIBC__ on glibc; present on musl too +#endif +#if defined(__GLIBC__) + #include // malloc_trim +#elif defined(__linux__) && !defined(__musl__) + #error "Linux libc is neither glibc (__GLIBC__) nor musl (-D__musl__); fix the malloc_trim guard" #endif // Friend of Recording (see flightRecorder.h), giving this test access to the @@ -70,11 +82,19 @@ TEST(GlibcMallocActive, ReportsActiveWithNoReplacementAllocatorLoaded) { // and, crucially, it is process-wide allocator behaviour rather than profiler // memory, which is why it is reported as a separate counter and not as an NM_* // category (see nativeMem.h). - -#ifdef DD_HAVE_MALLINFO2 +// +// The tests run against the same runtime-resolved mallinfo2 path the +// production code uses (mallocArenaSnapshot, see mallocInfo.h) -- they are +// never compiled out by header versions: on a runtime without mallinfo2 +// (glibc < 2.33, musl, macOS) they skip, mirroring the counters staying unset. static const size_t SBTABLE_SIZE = 6144; // sizeof(SBTable), measured static const int FLUSH_BLOCKS = 26450; // allocations in one observed flush +// Above glibc's dynamic mmap-threshold cap (DEFAULT_MMAP_THRESHOLD_MAX, 32 MiB +// on LP64), so it is always mmap-served and freeing it does not raise the +// threshold. Also well above anything the churn tests can leave free in the +// arena, so it cannot be carved from bins/top instead. +static const size_t MMAPPED_BLOCK = (size_t)256 << 20; // These premises are about glibc's arena. Under any other allocator the // churn below never reaches it, so there is nothing to measure. @@ -83,6 +103,70 @@ static const int FLUSH_BLOCKS = 26450; // allocations in one observed flush GTEST_SKIP() << "glibc malloc is not the allocator in force"; \ } +#define SKIP_UNLESS_MALLINFO2(out) \ + if (!mallocArenaSnapshot(out)) { \ + GTEST_SKIP() << "mallinfo2 not available at runtime; arena counters " \ + "stay unset on this platform"; \ + } + +// malloc_trim is a glibc extension and compiles only on glibc; elsewhere this +// returns false without trimming. It is only reached after +// SKIP_UNLESS_GLIBC_MALLOC and the mallinfo2 probe, i.e. on a glibc >= 2.33 +// runtime, so a false return there means the __GLIBC__ guard regressed. +static bool trimArenas() { +#if defined(__GLIBC__) + malloc_trim(0); + return true; +#else + return false; +#endif +} + +// Frees every block still held in the vector when the test body exits by any +// path (GTEST_SKIP / ASSERT_* early returns included), so a failing or skipped +// test does not leak 100+ MiB into the rest of the gtest process. Explicit +// free loops that must run at a measured point are followed by .clear(). +struct FreeAllOnExit { + explicit FreeAllOnExit(std::vector &v) : v_(v) {} + ~FreeAllOnExit() { + for (void *p : v_) free(p); + v_.clear(); + } + FreeAllOnExit(const FreeAllOnExit &) = delete; + FreeAllOnExit &operator=(const FreeAllOnExit &) = delete; + std::vector &v_; +}; + +// Pre-fills every field with 0xFF so a field the snapshot fails to copy reads +// SIZE_MAX instead of whatever was on the stack. +static bool sentinelSnapshot(MallocArenaInfo *out) { + memset(out, 0xFF, sizeof *out); + return mallocArenaSnapshot(out); +} + +// glibc int_mallinfo(): per arena, under that arena's lock, fordblks += avail, +// uordblks += system_mem - avail and arena += system_mem, so +// arena == uordblks + fordblks holds exactly within ONE snapshot, even with +// concurrent allocation. keepcost is chunksize(main_arena.top), which is part +// of the main arena's avail, so keepcost <= fordblks. Together these pin each +// copied field to its glibc meaning: a swapped or dropped copy of +// arena/uordblks/fordblks/keepcost breaks one of them. +static void expectMallinfoInvariants(const MallocArenaInfo &s, const char *where) { + EXPECT_EQ(s.arena, s.uordblks + s.fordblks) << where << ": arena != in_use + free_held"; + EXPECT_LE(s.keepcost, s.fordblks) << where << ": trimmable top exceeds free_held"; + EXPECT_GT(s.arena, 0u) << where; + EXPECT_GT(s.uordblks, 0u) << where; +} + +TEST(MallocArenaStats, SnapshotFieldsSatisfyGlibcAccountingIdentity) { + SKIP_UNLESS_GLIBC_MALLOC(); + MallocArenaInfo s; + if (!sentinelSnapshot(&s)) { + GTEST_SKIP() << "mallinfo2 not available at runtime"; + } + expectMallinfoInvariants(s, "idle"); +} + // Churn alone does NOT strand memory. 26,450 blocks allocated contiguously and // then all freed coalesce into one large free region at the arena top, which // glibc returns to the OS -- measured: arena 163 MB back down to 905 KB. So a @@ -96,29 +180,42 @@ TEST(MallocArenaStats, ContiguousChurnIsReturnedToTheOS) { for (void *p : warm) free(p); } - struct mallinfo2 before = mallinfo2(); + MallocArenaInfo before, peak, after; + SKIP_UNLESS_MALLINFO2(&before); std::vector blocks; blocks.reserve(FLUSH_BLOCKS); + FreeAllOnExit free_blocks(blocks); for (int i = 0; i < FLUSH_BLOCKS; i++) { void *p = malloc(SBTABLE_SIZE); + blocks.push_back(p); // owned before asserting, so a failure frees it ASSERT_NE(nullptr, p); *(volatile char *)p = 1; // touch it, as the real code would - blocks.push_back(p); } - struct mallinfo2 peak = mallinfo2(); + ASSERT_TRUE(sentinelSnapshot(&peak)); for (void *p : blocks) free(p); - struct mallinfo2 after = mallinfo2(); + blocks.clear(); // already freed; keep the guard from double-freeing + ASSERT_TRUE(sentinelSnapshot(&after)); - EXPECT_GE(peak.uordblks - before.uordblks, - (size_t)FLUSH_BLOCKS * SBTABLE_SIZE) + // The deltas are signed on purpose: an unsigned wrap let a uordblks<->fordblks + // copy swap pass. Under that swap the alloc-side delta becomes the fordblks + // change between before and peak -- at most a few hundred KiB (top pad plus + // fresh top), far below the ~155 MiB held -- and the free-side delta may even + // go negative if the trim threshold is raised. Do not "fix" the sign. + EXPECT_GE((long long)peak.uordblks - (long long)before.uordblks, + (long long)FLUSH_BLOCKS * (long long)SBTABLE_SIZE) << "in-use accounting did not reflect the held blocks"; + EXPECT_GE((long long)peak.uordblks - (long long)after.uordblks, + (long long)FLUSH_BLOCKS * (long long)SBTABLE_SIZE) + << "freeing the held blocks did not drop in-use accounting"; EXPECT_LT(after.uordblks, peak.uordblks); // The arena shrinks back: the coalesced free region is returned, so this // pattern leaves nothing meaningful in the free-but-held term. EXPECT_LT((long long)after.arena, (long long)peak.arena / 2) << "expected the coalesced top region to be returned to the OS"; + expectMallinfoInvariants(peak, "peak"); + expectMallinfoInvariants(after, "after"); printf("[ARENA] before: arena=%zu in_use=%zu free_held=%zu trimmable=%zu\n", before.arena, before.uordblks, before.fordblks, before.keepcost); @@ -140,41 +237,90 @@ TEST(MallocArenaStats, ContiguousChurnIsReturnedToTheOS) { // allocation *pattern* rather than of any one subsystem's allocations. TEST(MallocArenaStats, InterleavedSurvivorsStrandFreeChunksBeyondTrim) { SKIP_UNLESS_GLIBC_MALLOC(); + // Probe before allocating anything, so a skip cannot strand the survivors. + // Scratch only: the measurement must be taken after the churn is freed. + MallocArenaInfo probe; + SKIP_UNLESS_MALLINFO2(&probe); + std::vector survivors, churn; + FreeAllOnExit free_survivors(survivors); + FreeAllOnExit free_churn(churn); for (int i = 0; i < 20000; i++) { void *a = malloc(SBTABLE_SIZE); void *b = malloc(SBTABLE_SIZE); + survivors.push_back(a); // kept live, so it pins the region + churn.push_back(b); // freed below ASSERT_NE(nullptr, a); ASSERT_NE(nullptr, b); *(volatile char *)a = 1; *(volatile char *)b = 1; - survivors.push_back(a); // kept live, so it pins the region - churn.push_back(b); // freed below } for (void *p : churn) free(p); + churn.clear(); // already freed; keep the guard from double-freeing - struct mallinfo2 pre_trim = mallinfo2(); - malloc_trim(0); - struct mallinfo2 post_trim = mallinfo2(); + MallocArenaInfo pre_trim, post_trim; + ASSERT_TRUE(sentinelSnapshot(&pre_trim)); + ASSERT_TRUE(trimArenas()) + << "malloc_trim not compiled in although glibc malloc with mallinfo2 is " + "active; the __GLIBC__ guard regressed"; + ASSERT_TRUE(sentinelSnapshot(&post_trim)); EXPECT_GT(post_trim.fordblks, 0u) << "trim reclaimed everything; interleaving failed to strand chunks"; + // keepcost is the main arena's top chunk. malloc_trim(0) shrinks that top to + // about a page and never grows it, while the interleaved free chunks stay + // stranded in the bins (counted in fordblks, not in keepcost). + EXPECT_LE(post_trim.keepcost, pre_trim.keepcost) + << "malloc_trim grew the trimmable top"; + EXPECT_LT(post_trim.keepcost, post_trim.fordblks) + << "trimmable top must be a strict part of the stranded free_held"; + EXPECT_LT(post_trim.keepcost, (size_t)1 << 20) + << "malloc_trim left more than ~a page in the main arena's top"; + expectMallinfoInvariants(pre_trim, "pre_trim"); + expectMallinfoInvariants(post_trim, "post_trim"); + printf("[TRIM] free_held before trim: %.2f MiB, after trim: %.2f MiB " "(reclaimed %.2f MiB)\n", (double)pre_trim.fordblks / (1024 * 1024), (double)post_trim.fordblks / (1024 * 1024), (double)((long long)pre_trim.fordblks - (long long)post_trim.fordblks) / (1024 * 1024)); - - for (void *p : survivors) free(p); + printf("[TRIM] trimmable before trim: %zu, after trim: %zu\n", + pre_trim.keepcost, post_trim.keepcost); + // survivors are freed by free_survivors at scope exit, after every check. } -#else +// A request above the mmap-threshold cap is served by its own mapping: glibc +// accounts it in mp_.mmapped_mem (hblkhd) and nowhere in the arenas. The deltas +// are signed so an unsigned wrap can never satisfy >=; a dropped hblkhd copy +// reads the SIZE_MAX sentinel (-1 as long long) in every snapshot, delta 0. +TEST(MallocArenaStats, MmappedChunkIsReportedInHblkhdOnly) { + SKIP_UNLESS_GLIBC_MALLOC(); + MallocArenaInfo before, held, released; + if (!sentinelSnapshot(&before)) { + GTEST_SKIP() << "mallinfo2 not available at runtime"; + } + void *big = malloc(MMAPPED_BLOCK); + if (big == nullptr) { + GTEST_SKIP() << "could not allocate the mmapped test block"; + } + bool held_ok = sentinelSnapshot(&held); + free(big); // before any ASSERT, so no exit path leaks the mapping + ASSERT_TRUE(held_ok); + ASSERT_TRUE(sentinelSnapshot(&released)); -TEST(MallocArenaStats, SkippedWithoutMallinfo2) { - GTEST_SKIP() << "mallinfo2 requires glibc 2.33+; arena counters report zero " - "on this platform"; + EXPECT_GE((long long)held.hblkhd - (long long)before.hblkhd, + (long long)MMAPPED_BLOCK) + << "mmapped chunk not reflected in hblkhd"; + EXPECT_LT((long long)held.uordblks - (long long)before.uordblks, + (long long)(MMAPPED_BLOCK / 2)) + << "mmapped chunk leaked into in-use arena accounting"; + EXPECT_LT((long long)held.arena - (long long)before.arena, + (long long)(MMAPPED_BLOCK / 2)) + << "mmapped chunk leaked into arena system memory"; + EXPECT_GE((long long)held.hblkhd - (long long)released.hblkhd, + (long long)MMAPPED_BLOCK) + << "freeing the mmapped chunk did not lower hblkhd"; + expectMallinfoInvariants(held, "held"); } - -#endif diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java new file mode 100644 index 0000000000..d0b02c2b94 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java @@ -0,0 +1,178 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +package com.datadoghq.profiler.nativemem; + +import com.datadoghq.profiler.AbstractProfilerTest; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; + +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.util.Locale; +import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Functional check for the process-wide malloc arena counters + * ({@code malloc_arena_bytes}, {@code malloc_in_use_bytes} and siblings). + * + *

These counters are produced from glibc's {@code mallinfo2()}, which the + * native code resolves at runtime with {@code dlsym(RTLD_DEFAULT, "mallinfo2")} + * (see {@code mallocInfo.cpp}). Resolving at runtime is what keeps the counters + * alive in binaries whose build headers predate glibc 2.33 -- the shipped + * release libraries are built against glibc 2.28 headers (el8), where a + * compile-time gate used to compile the whole path out and left every counter + * at zero. This test pins the observable end of that: on a runtime glibc that + * provides {@code mallinfo2()} (2.33+) a running JVM must always report + * non-zero arena and in-use figures, while on runtimes without it (older + * glibc, musl) the counters stay unset at zero. + * + *

The test is skipped, through a JUnit assumption, when glibc malloc is + * detectably not the allocator in force (the asan/tsan sanitizer test configs, + * an LD_PRELOAD'd jemalloc/tcmalloc), because the native code leaves the + * counters at zero there regardless of the glibc version. + */ +public class MallocArenaCountersTest extends AbstractProfilerTest { + + @Override + protected String getProfilerCommand() { + return "wall=~1ms"; + } + + @Test + public void shouldPublishNonZeroMallocArenaCountersWhenMallinfo2IsAvailable() throws Exception { + busyLoop(); + stopProfiler(); + + // The counters live in a fixed table, so they are always emitted -- + // -1 would mean the ProfilerCounter events never made it into the + // recording at all, which is a harness failure, not a counter state. + long arena = getRecordedCounterValue("malloc_arena_bytes"); + long inUse = getRecordedCounterValue("malloc_in_use_bytes"); + assertNotEquals(-1, arena, "malloc_arena_bytes not present in the recording"); + assertNotEquals(-1, inUse, "malloc_in_use_bytes not present in the recording"); + long trimmable = getRecordedCounterValue("malloc_trimmable_bytes"); + long mmapped = getRecordedCounterValue("malloc_mmap_bytes"); + assertNotEquals(-1, trimmable, "malloc_trimmable_bytes not present in the recording"); + assertNotEquals(-1, mmapped, "malloc_mmap_bytes not present in the recording"); + + // The native side leaves every arena counter at 0 when glibc malloc is not + // the allocator in force (Recording::glibcMallocActive(): sanitizer + // runtimes, LD_PRELOAD'd jemalloc/tcmalloc), so the glibc-version oracle + // below does not apply there. + String reason = nonGlibcAllocatorReason(); + Assumptions.assumeTrue(reason == null, + "glibc malloc not active (" + reason + "); arena counters legitimately stay 0"); + + int[] glibc = runtimeGlibcVersion(); + boolean mallinfo2Available = glibc != null && (glibc[0] > 2 || (glibc[0] == 2 && glibc[1] >= 33)); + if (!mallinfo2Available) { + // musl and pre-2.33 glibc runtimes: the counters stay unset (0), + // mirroring the dlsym lookup failing in the native code. + assertEquals(0, arena, "malloc_arena_bytes must stay unset without mallinfo2"); + assertEquals(0, inUse, "malloc_in_use_bytes must stay unset without mallinfo2"); + assertEquals(0, trimmable, "malloc_trimmable_bytes must stay unset without mallinfo2"); + assertEquals(0, mmapped, "malloc_mmap_bytes must stay unset without mallinfo2"); + return; + } + + // A running JVM has always handed out memory from glibc's arenas, so a + // zero here means the mallinfo2 path was disabled -- exactly the bug + // this guards against (compile-time gating compiled it out of the el8 + // release builds). Configurations where glibc malloc is detectably not + // in force were skipped above. trimmable/mmapped are not asserted + // positive: neither is guaranteed non-zero at flush time. + assertTrue(arena > 0, "malloc_arena_bytes must be non-zero on glibc " + + glibc[0] + "." + glibc[1] + ", was " + arena); + assertTrue(inUse > 0, "malloc_in_use_bytes must be non-zero on glibc " + + glibc[0] + "." + glibc[1] + ", was " + inUse); + } + + /** + * Returns why glibc malloc cannot be the allocator in force for this JVM, or + * {@code null} when nothing detectable says otherwise. Mirrors the cases in + * which the native glibcMallocActive() check disables the arena counters. + */ + private String nonGlibcAllocatorReason() { + String config = System.getProperty("ddprof_test.config", ""); + if ("asan".equals(config) || "tsan".equals(config)) { + return "sanitizer test config " + config; + } + if (isAsan()) { + return "ASAN_OPTIONS set"; + } + if (isTsan()) { + return "TSAN_OPTIONS set"; + } + String preload = System.getenv("LD_PRELOAD"); + if (preload != null) { + String p = preload.toLowerCase(Locale.ROOT); + for (String lib : new String[] {"jemalloc", "tcmalloc", "libasan", "libtsan", + "clang_rt.asan", "clang_rt.tsan"}) { + if (p.contains(lib)) { + return "LD_PRELOAD=" + preload; + } + } + } + return null; + } + + /** Keeps the sampler busy briefly so native buffers are populated for the counters to report. */ + private void busyLoop() { + long deadline = System.nanoTime() + 100_000_000L; // ~100ms + double sink = 0; + while (System.nanoTime() < deadline) { + for (int i = 0; i < 10_000; i++) { + sink += Math.sqrt(i); + } + } + // Guard against dead-code elimination of the busy loop. + assertTrue(!Double.isNaN(sink)); + } + + /** + * Detects the runtime glibc version by running {@code ldd --version} and + * taking the last {@code major.minor} pair from its first line (e.g. + * {@code ldd (Ubuntu GLIBC 2.35-0ubuntu3.10) 2.35} or + * {@code ldd (GNU libc) 2.17}). Returns {@code null} when the runtime is + * not glibc (musl prints a version without a GLIBC marker; the probe is + * not attempted at all on non-Linux platforms). stderr is merged into + * stdout (musl's ldd prints its banner to stderr), the wait is bounded, and + * the child is always destroyed, whatever the exit path. + */ + private static int[] runtimeGlibcVersion() throws Exception { + if (!System.getProperty("os.name", "").startsWith("Linux")) { + return null; + } + Process process = new ProcessBuilder("ldd", "--version").redirectErrorStream(true).start(); + String firstLine; + try { + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(process.getInputStream()))) { + firstLine = reader.readLine(); + } + process.waitFor(10, TimeUnit.SECONDS); // result ignored; finally reaps it either way + } finally { + process.destroyForcibly(); + } + if (firstLine == null) { + return null; + } + if (firstLine.contains("musl") || firstLine.contains("FreeBSD")) { + return null; + } + Matcher matcher = Pattern.compile("(\\d+)\\.(\\d+)").matcher(firstLine); + int[] version = null; + while (matcher.find()) { + version = new int[] {Integer.parseInt(matcher.group(1)), Integer.parseInt(matcher.group(2))}; + } + return version; + } +} From 1dd97c23e6842f73c6547050dbc0264f39a22efa Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Wed, 28 Oct 2026 16:54:01 +0100 Subject: [PATCH 3/9] Touch the mmapped test chunk so release builds cannot eliminate the allocation (PROF-16350) --- ddprof-lib/src/test/cpp/mallocArenaStats_ut.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ddprof-lib/src/test/cpp/mallocArenaStats_ut.cpp b/ddprof-lib/src/test/cpp/mallocArenaStats_ut.cpp index d5392a4601..b627c490ca 100644 --- a/ddprof-lib/src/test/cpp/mallocArenaStats_ut.cpp +++ b/ddprof-lib/src/test/cpp/mallocArenaStats_ut.cpp @@ -305,6 +305,11 @@ TEST(MallocArenaStats, MmappedChunkIsReportedInHblkhdOnly) { if (big == nullptr) { GTEST_SKIP() << "could not allocate the mmapped test block"; } + // Touch the chunk through a volatile pointer: an untouched malloc/free pair + // whose result never escapes is eliminated by the optimizer in release + // builds (-O2), so the allocation never happens and hblkhd never moves. + // The volatile write makes the allocation observable. + *(volatile char *)big = 1; bool held_ok = sentinelSnapshot(&held); free(big); // before any ASSERT, so no exit path leaks the mapping ASSERT_TRUE(held_ok); From c366afa1a8d3539d9b755508ea5cae6841ffbfa6 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 12 Nov 2026 10:37:52 +0100 Subject: [PATCH 4/9] Document malloc arena counters availability and fallback semantics (PROF-16350) --- doc/reference/ProfilerMemoryRequirements.md | 22 +++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/doc/reference/ProfilerMemoryRequirements.md b/doc/reference/ProfilerMemoryRequirements.md index 9dd378d795..d9339ae90c 100644 --- a/doc/reference/ProfilerMemoryRequirements.md +++ b/doc/reference/ProfilerMemoryRequirements.md @@ -576,6 +576,28 @@ jcmd VM.native_memory summary.diff - Profiler data structures (CallTraceStorage, StringDictionary, etc.) are not visible to NMT (libc `malloc`/raw `mmap`); the `native_mem_*` counters break these down by category +### Malloc Arena Counters + +The `malloc_*` JFR counters report the process-wide allocator state, not profiler +memory: `malloc_arena_bytes` (system memory held by the allocator's arenas), +`malloc_in_use_bytes` (bytes currently handed out to callers), +`malloc_free_held_bytes` (bytes free but retained in the arenas), +`malloc_trimmable_bytes` (the main arena's top chunk, i.e. what `malloc_trim` +could return), and `malloc_mmap_bytes` (bytes in mmap'd chunks, returned to the +OS on free). + +On glibc these come from `mallinfo2()`, resolved at runtime rather than at +compile time: a build made against old glibc headers (the el8 image has glibc +2.28) still populates the counters when the runtime glibc provides `mallinfo2` +(glibc >= 2.33). The counters stay unset — not published as zero — when the +runtime allocator does not provide `mallinfo2` (glibc < 2.33, musl, macOS), or +when a replacement allocator (tcmalloc, jemalloc) is loaded, because its +accounting model differs and the values would not be comparable. High +`malloc_free_held_bytes` with flat `malloc_in_use_bytes` indicates arena +fragmentation from interleaved allocation; `malloc_trimmable_bytes` separates +trim-threshold policy from genuine fragmentation, which trimming cannot +reclaim. + ### Class Count Metrics ```bash From 07fb43cb302f8e868b300d56772fbb3c51a3a6cf Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 12 Nov 2026 10:46:44 +0100 Subject: [PATCH 5/9] Apply spotless formatting to MallocArenaCountersTest (PROF-16350) --- .../nativemem/MallocArenaCountersTest.java | 298 +++++++++--------- 1 file changed, 154 insertions(+), 144 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java index d0b02c2b94..13258fee01 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java @@ -4,175 +4,185 @@ */ package com.datadoghq.profiler.nativemem; -import com.datadoghq.profiler.AbstractProfilerTest; -import org.junit.jupiter.api.Assumptions; -import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import com.datadoghq.profiler.AbstractProfilerTest; import java.io.BufferedReader; import java.io.InputStreamReader; import java.util.Locale; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; /** - * Functional check for the process-wide malloc arena counters - * ({@code malloc_arena_bytes}, {@code malloc_in_use_bytes} and siblings). + * Functional check for the process-wide malloc arena counters ({@code malloc_arena_bytes}, {@code + * malloc_in_use_bytes} and siblings). * - *

These counters are produced from glibc's {@code mallinfo2()}, which the - * native code resolves at runtime with {@code dlsym(RTLD_DEFAULT, "mallinfo2")} - * (see {@code mallocInfo.cpp}). Resolving at runtime is what keeps the counters - * alive in binaries whose build headers predate glibc 2.33 -- the shipped - * release libraries are built against glibc 2.28 headers (el8), where a - * compile-time gate used to compile the whole path out and left every counter - * at zero. This test pins the observable end of that: on a runtime glibc that - * provides {@code mallinfo2()} (2.33+) a running JVM must always report - * non-zero arena and in-use figures, while on runtimes without it (older + *

These counters are produced from glibc's {@code mallinfo2()}, which the native code resolves + * at runtime with {@code dlsym(RTLD_DEFAULT, "mallinfo2")} (see {@code mallocInfo.cpp}). Resolving + * at runtime is what keeps the counters alive in binaries whose build headers predate glibc 2.33 -- + * the shipped release libraries are built against glibc 2.28 headers (el8), where a compile-time + * gate used to compile the whole path out and left every counter at zero. This test pins the + * observable end of that: on a runtime glibc that provides {@code mallinfo2()} (2.33+) a running + * JVM must always report non-zero arena and in-use figures, while on runtimes without it (older * glibc, musl) the counters stay unset at zero. * - *

The test is skipped, through a JUnit assumption, when glibc malloc is - * detectably not the allocator in force (the asan/tsan sanitizer test configs, - * an LD_PRELOAD'd jemalloc/tcmalloc), because the native code leaves the - * counters at zero there regardless of the glibc version. + *

The test is skipped, through a JUnit assumption, when glibc malloc is detectably not the + * allocator in force (the asan/tsan sanitizer test configs, an LD_PRELOAD'd jemalloc/tcmalloc), + * because the native code leaves the counters at zero there regardless of the glibc version. */ public class MallocArenaCountersTest extends AbstractProfilerTest { - @Override - protected String getProfilerCommand() { - return "wall=~1ms"; - } - - @Test - public void shouldPublishNonZeroMallocArenaCountersWhenMallinfo2IsAvailable() throws Exception { - busyLoop(); - stopProfiler(); + @Override + protected String getProfilerCommand() { + return "wall=~1ms"; + } - // The counters live in a fixed table, so they are always emitted -- - // -1 would mean the ProfilerCounter events never made it into the - // recording at all, which is a harness failure, not a counter state. - long arena = getRecordedCounterValue("malloc_arena_bytes"); - long inUse = getRecordedCounterValue("malloc_in_use_bytes"); - assertNotEquals(-1, arena, "malloc_arena_bytes not present in the recording"); - assertNotEquals(-1, inUse, "malloc_in_use_bytes not present in the recording"); - long trimmable = getRecordedCounterValue("malloc_trimmable_bytes"); - long mmapped = getRecordedCounterValue("malloc_mmap_bytes"); - assertNotEquals(-1, trimmable, "malloc_trimmable_bytes not present in the recording"); - assertNotEquals(-1, mmapped, "malloc_mmap_bytes not present in the recording"); + @Test + public void shouldPublishNonZeroMallocArenaCountersWhenMallinfo2IsAvailable() throws Exception { + busyLoop(); + stopProfiler(); - // The native side leaves every arena counter at 0 when glibc malloc is not - // the allocator in force (Recording::glibcMallocActive(): sanitizer - // runtimes, LD_PRELOAD'd jemalloc/tcmalloc), so the glibc-version oracle - // below does not apply there. - String reason = nonGlibcAllocatorReason(); - Assumptions.assumeTrue(reason == null, - "glibc malloc not active (" + reason + "); arena counters legitimately stay 0"); + // The counters live in a fixed table, so they are always emitted -- + // -1 would mean the ProfilerCounter events never made it into the + // recording at all, which is a harness failure, not a counter state. + long arena = getRecordedCounterValue("malloc_arena_bytes"); + long inUse = getRecordedCounterValue("malloc_in_use_bytes"); + assertNotEquals(-1, arena, "malloc_arena_bytes not present in the recording"); + assertNotEquals(-1, inUse, "malloc_in_use_bytes not present in the recording"); + long trimmable = getRecordedCounterValue("malloc_trimmable_bytes"); + long mmapped = getRecordedCounterValue("malloc_mmap_bytes"); + assertNotEquals(-1, trimmable, "malloc_trimmable_bytes not present in the recording"); + assertNotEquals(-1, mmapped, "malloc_mmap_bytes not present in the recording"); - int[] glibc = runtimeGlibcVersion(); - boolean mallinfo2Available = glibc != null && (glibc[0] > 2 || (glibc[0] == 2 && glibc[1] >= 33)); - if (!mallinfo2Available) { - // musl and pre-2.33 glibc runtimes: the counters stay unset (0), - // mirroring the dlsym lookup failing in the native code. - assertEquals(0, arena, "malloc_arena_bytes must stay unset without mallinfo2"); - assertEquals(0, inUse, "malloc_in_use_bytes must stay unset without mallinfo2"); - assertEquals(0, trimmable, "malloc_trimmable_bytes must stay unset without mallinfo2"); - assertEquals(0, mmapped, "malloc_mmap_bytes must stay unset without mallinfo2"); - return; - } + // The native side leaves every arena counter at 0 when glibc malloc is not + // the allocator in force (Recording::glibcMallocActive(): sanitizer + // runtimes, LD_PRELOAD'd jemalloc/tcmalloc), so the glibc-version oracle + // below does not apply there. + String reason = nonGlibcAllocatorReason(); + Assumptions.assumeTrue( + reason == null, + "glibc malloc not active (" + reason + "); arena counters legitimately stay 0"); - // A running JVM has always handed out memory from glibc's arenas, so a - // zero here means the mallinfo2 path was disabled -- exactly the bug - // this guards against (compile-time gating compiled it out of the el8 - // release builds). Configurations where glibc malloc is detectably not - // in force were skipped above. trimmable/mmapped are not asserted - // positive: neither is guaranteed non-zero at flush time. - assertTrue(arena > 0, "malloc_arena_bytes must be non-zero on glibc " - + glibc[0] + "." + glibc[1] + ", was " + arena); - assertTrue(inUse > 0, "malloc_in_use_bytes must be non-zero on glibc " - + glibc[0] + "." + glibc[1] + ", was " + inUse); + int[] glibc = runtimeGlibcVersion(); + boolean mallinfo2Available = + glibc != null && (glibc[0] > 2 || (glibc[0] == 2 && glibc[1] >= 33)); + if (!mallinfo2Available) { + // musl and pre-2.33 glibc runtimes: the counters stay unset (0), + // mirroring the dlsym lookup failing in the native code. + assertEquals(0, arena, "malloc_arena_bytes must stay unset without mallinfo2"); + assertEquals(0, inUse, "malloc_in_use_bytes must stay unset without mallinfo2"); + assertEquals(0, trimmable, "malloc_trimmable_bytes must stay unset without mallinfo2"); + assertEquals(0, mmapped, "malloc_mmap_bytes must stay unset without mallinfo2"); + return; } - /** - * Returns why glibc malloc cannot be the allocator in force for this JVM, or - * {@code null} when nothing detectable says otherwise. Mirrors the cases in - * which the native glibcMallocActive() check disables the arena counters. - */ - private String nonGlibcAllocatorReason() { - String config = System.getProperty("ddprof_test.config", ""); - if ("asan".equals(config) || "tsan".equals(config)) { - return "sanitizer test config " + config; - } - if (isAsan()) { - return "ASAN_OPTIONS set"; - } - if (isTsan()) { - return "TSAN_OPTIONS set"; - } - String preload = System.getenv("LD_PRELOAD"); - if (preload != null) { - String p = preload.toLowerCase(Locale.ROOT); - for (String lib : new String[] {"jemalloc", "tcmalloc", "libasan", "libtsan", - "clang_rt.asan", "clang_rt.tsan"}) { - if (p.contains(lib)) { - return "LD_PRELOAD=" + preload; - } - } + // A running JVM has always handed out memory from glibc's arenas, so a + // zero here means the mallinfo2 path was disabled -- exactly the bug + // this guards against (compile-time gating compiled it out of the el8 + // release builds). Configurations where glibc malloc is detectably not + // in force were skipped above. trimmable/mmapped are not asserted + // positive: neither is guaranteed non-zero at flush time. + assertTrue( + arena > 0, + "malloc_arena_bytes must be non-zero on glibc " + + glibc[0] + + "." + + glibc[1] + + ", was " + + arena); + assertTrue( + inUse > 0, + "malloc_in_use_bytes must be non-zero on glibc " + + glibc[0] + + "." + + glibc[1] + + ", was " + + inUse); + } + + /** + * Returns why glibc malloc cannot be the allocator in force for this JVM, or {@code null} when + * nothing detectable says otherwise. Mirrors the cases in which the native glibcMallocActive() + * check disables the arena counters. + */ + private String nonGlibcAllocatorReason() { + String config = System.getProperty("ddprof_test.config", ""); + if ("asan".equals(config) || "tsan".equals(config)) { + return "sanitizer test config " + config; + } + if (isAsan()) { + return "ASAN_OPTIONS set"; + } + if (isTsan()) { + return "TSAN_OPTIONS set"; + } + String preload = System.getenv("LD_PRELOAD"); + if (preload != null) { + String p = preload.toLowerCase(Locale.ROOT); + for (String lib : + new String[] { + "jemalloc", "tcmalloc", "libasan", "libtsan", "clang_rt.asan", "clang_rt.tsan" + }) { + if (p.contains(lib)) { + return "LD_PRELOAD=" + preload; } - return null; + } } + return null; + } - /** Keeps the sampler busy briefly so native buffers are populated for the counters to report. */ - private void busyLoop() { - long deadline = System.nanoTime() + 100_000_000L; // ~100ms - double sink = 0; - while (System.nanoTime() < deadline) { - for (int i = 0; i < 10_000; i++) { - sink += Math.sqrt(i); - } - } - // Guard against dead-code elimination of the busy loop. - assertTrue(!Double.isNaN(sink)); + /** Keeps the sampler busy briefly so native buffers are populated for the counters to report. */ + private void busyLoop() { + long deadline = System.nanoTime() + 100_000_000L; // ~100ms + double sink = 0; + while (System.nanoTime() < deadline) { + for (int i = 0; i < 10_000; i++) { + sink += Math.sqrt(i); + } } + // Guard against dead-code elimination of the busy loop. + assertTrue(!Double.isNaN(sink)); + } - /** - * Detects the runtime glibc version by running {@code ldd --version} and - * taking the last {@code major.minor} pair from its first line (e.g. - * {@code ldd (Ubuntu GLIBC 2.35-0ubuntu3.10) 2.35} or - * {@code ldd (GNU libc) 2.17}). Returns {@code null} when the runtime is - * not glibc (musl prints a version without a GLIBC marker; the probe is - * not attempted at all on non-Linux platforms). stderr is merged into - * stdout (musl's ldd prints its banner to stderr), the wait is bounded, and - * the child is always destroyed, whatever the exit path. - */ - private static int[] runtimeGlibcVersion() throws Exception { - if (!System.getProperty("os.name", "").startsWith("Linux")) { - return null; - } - Process process = new ProcessBuilder("ldd", "--version").redirectErrorStream(true).start(); - String firstLine; - try { - try (BufferedReader reader = new BufferedReader( - new InputStreamReader(process.getInputStream()))) { - firstLine = reader.readLine(); - } - process.waitFor(10, TimeUnit.SECONDS); // result ignored; finally reaps it either way - } finally { - process.destroyForcibly(); - } - if (firstLine == null) { - return null; - } - if (firstLine.contains("musl") || firstLine.contains("FreeBSD")) { - return null; - } - Matcher matcher = Pattern.compile("(\\d+)\\.(\\d+)").matcher(firstLine); - int[] version = null; - while (matcher.find()) { - version = new int[] {Integer.parseInt(matcher.group(1)), Integer.parseInt(matcher.group(2))}; - } - return version; + /** + * Detects the runtime glibc version by running {@code ldd --version} and taking the last {@code + * major.minor} pair from its first line (e.g. {@code ldd (Ubuntu GLIBC 2.35-0ubuntu3.10) 2.35} or + * {@code ldd (GNU libc) 2.17}). Returns {@code null} when the runtime is not glibc (musl prints a + * version without a GLIBC marker; the probe is not attempted at all on non-Linux platforms). + * stderr is merged into stdout (musl's ldd prints its banner to stderr), the wait is bounded, and + * the child is always destroyed, whatever the exit path. + */ + private static int[] runtimeGlibcVersion() throws Exception { + if (!System.getProperty("os.name", "").startsWith("Linux")) { + return null; + } + Process process = new ProcessBuilder("ldd", "--version").redirectErrorStream(true).start(); + String firstLine; + try { + try (BufferedReader reader = + new BufferedReader(new InputStreamReader(process.getInputStream()))) { + firstLine = reader.readLine(); + } + process.waitFor(10, TimeUnit.SECONDS); // result ignored; finally reaps it either way + } finally { + process.destroyForcibly(); + } + if (firstLine == null) { + return null; + } + if (firstLine.contains("musl") || firstLine.contains("FreeBSD")) { + return null; + } + Matcher matcher = Pattern.compile("(\\d+)\\.(\\d+)").matcher(firstLine); + int[] version = null; + while (matcher.find()) { + version = new int[] {Integer.parseInt(matcher.group(1)), Integer.parseInt(matcher.group(2))}; } + return version; + } } From efb2a4e5e21506e8ae5aa46489ea1c0cd51a2c7f Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 12 Nov 2026 10:56:23 +0100 Subject: [PATCH 6/9] Anchor ldd version parse to the trailing token and bound the output read (PROF-16350) --- .../nativemem/MallocArenaCountersTest.java | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java index 13258fee01..a46be93141 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java @@ -150,12 +150,15 @@ private void busyLoop() { } /** - * Detects the runtime glibc version by running {@code ldd --version} and taking the last {@code - * major.minor} pair from its first line (e.g. {@code ldd (Ubuntu GLIBC 2.35-0ubuntu3.10) 2.35} or - * {@code ldd (GNU libc) 2.17}). Returns {@code null} when the runtime is not glibc (musl prints a - * version without a GLIBC marker; the probe is not attempted at all on non-Linux platforms). - * stderr is merged into stdout (musl's ldd prints its banner to stderr), the wait is bounded, and - * the child is always destroyed, whatever the exit path. + * Detects the runtime glibc version by running {@code ldd --version} and taking the trailing + * {@code major.minor} token from its first line (e.g. {@code ldd (Ubuntu GLIBC 2.35-0ubuntu3.10) + * 2.35} or {@code ldd (GNU libc) 2.17}). The trailing token is required: distro package versions + * elsewhere in the banner also contain {@code major.minor} pairs (Ubuntu's {@code + * 2.31-0ubuntu9.16} ends with {@code 9.16}), and picking one of those misclassifies the glibc + * 2.33 gate. Returns {@code null} when the runtime is not glibc (musl prints a version without a + * GLIBC marker; the probe is not attempted at all on non-Linux platforms). stderr is merged into + * stdout (musl's ldd prints its banner to stderr), the child is reaped under a deadline before + * anything is read, and it is always destroyed, whatever the exit path. */ private static int[] runtimeGlibcVersion() throws Exception { if (!System.getProperty("os.name", "").startsWith("Linux")) { @@ -164,11 +167,16 @@ private static int[] runtimeGlibcVersion() throws Exception { Process process = new ProcessBuilder("ldd", "--version").redirectErrorStream(true).start(); String firstLine; try { + // Wait first, then read: the exit deadline also bounds the read. ldd's output is a few + // lines, far below the pipe buffer, so it cannot block the child on a full pipe while + // we wait. Reading first would block unboundedly on a child that prints nothing. + if (!process.waitFor(10, TimeUnit.SECONDS)) { + return null; + } try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()))) { firstLine = reader.readLine(); } - process.waitFor(10, TimeUnit.SECONDS); // result ignored; finally reaps it either way } finally { process.destroyForcibly(); } @@ -178,11 +186,10 @@ private static int[] runtimeGlibcVersion() throws Exception { if (firstLine.contains("musl") || firstLine.contains("FreeBSD")) { return null; } - Matcher matcher = Pattern.compile("(\\d+)\\.(\\d+)").matcher(firstLine); - int[] version = null; - while (matcher.find()) { - version = new int[] {Integer.parseInt(matcher.group(1)), Integer.parseInt(matcher.group(2))}; + Matcher matcher = Pattern.compile("\\b(\\d+)\\.(\\d+)\\s*$").matcher(firstLine); + if (!matcher.find()) { + return null; } - return version; + return new int[] {Integer.parseInt(matcher.group(1)), Integer.parseInt(matcher.group(2))}; } } From f027b26536423f8956747906f1b1df722a3dead0 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 12 Nov 2026 15:34:34 +0100 Subject: [PATCH 7/9] Quarantine LineNumberTableCopyTest.RejectsUnmappedSource on glibc-tsan cells (PROF-16621) --- ddprof-test/quarantine.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ddprof-test/quarantine.txt b/ddprof-test/quarantine.txt index e2988f8d0f..f5214efb94 100644 --- a/ddprof-test/quarantine.txt +++ b/ddprof-test/quarantine.txt @@ -102,3 +102,10 @@ com.datadoghq.profiler.referencechains.ReferenceChainTrackingTest.shouldReconstr com.datadoghq.profiler.referencechains.ThreadLocalLeakReferenceChainTest.shouldCorrelateThreadLocalHeldLeakChain | PROF-16305 | 2026-10-23 | 2027-01-21 | *-slow | Child reports [tl-correlation-tag-out-of-pool]; 3 failing jobs in nightly run 39893765385 com.datadoghq.profiler.referencechains.LeakTagCorrelationReferenceChainTest.shouldCorrelateLeakChainTargetTagWithLiveObjectLeakTag | PROF-16305 | 2026-10-23 | 2027-01-21 | *-slow | Child reports [correlation-tag-out-of-pool]; 1 failing job in nightly run 39893765385 + +# PROF-16621 -- TSan timing flake in the line-number-table fault-recovery test: +# failed attempt 1 in glibc-tsan-amd64 (GitLab job 2243650462, PR #918 pipeline, +# head efb2a4e5e) and passed the full-suite retry with an identical binary. +# Scoped to the gtest tsan cells so a failure in the regular suite, in asan, or +# on another arch still gates. +LineNumberTableCopyTest.RejectsUnmappedSource | PROF-16621 | 2026-11-12 | 2027-02-10 | glibc-tsan-* | Failed on attempt 1 and passed the full-suite retry (job 2243650462); fault-recovery timing under TSan instrumentation, unrelated to the triggering change set From 64a395a864fd10298f628809c9074f15f3346107 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 12 Nov 2026 15:44:57 +0100 Subject: [PATCH 8/9] Pin the mirror include explicitly, cache the dlsym lookups, skip on inconclusive ldd probes (PROF-16350) --- ddprof-lib/src/main/cpp/flightRecorder.cpp | 16 +++++-- ddprof-lib/src/main/cpp/mallocInfo.cpp | 28 +++++++++-- .../nativemem/MallocArenaCountersTest.java | 48 +++++++++++++++---- 3 files changed, 73 insertions(+), 19 deletions(-) diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 80a9938aac..0bcac12f3d 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -2133,14 +2133,22 @@ void Recording::writeLogLevels(Buffer *buf) { // mallinfo2(), which then reports all zeros. Neither runtime exports // mallctl/tc_malloc_size, so the dlsym probe alone would miss them. // -// Not cached: called once per JFR chunk flush, so the dlsym() lookups (three -// counting the mallinfo2 resolution in mallocInfo.cpp) are negligible. +// Resolved once: the lookup result cannot change after process start, and the +// loader lock is not worth taking on every chunk flush. The sentinel makes the +// first call racily idempotent -- dlsym() is thread-safe and all threads store +// the same verdict -- so no lock is needed. Flush path only, never signal +// context, so plain statics are legal here. bool Recording::glibcMallocActive() { #if defined(ASAN_ENABLED) || defined(TSAN_ENABLED) return false; #else - return dlsym(RTLD_DEFAULT, "mallctl") == nullptr && - dlsym(RTLD_DEFAULT, "tc_malloc_size") == nullptr; + static int cached = 0; // 0 = unresolved, 1 = active, 2 = inactive + if (cached == 0) { + bool active = dlsym(RTLD_DEFAULT, "mallctl") == nullptr && + dlsym(RTLD_DEFAULT, "tc_malloc_size") == nullptr; + cached = active ? 1 : 2; + } + return cached == 1; #endif } diff --git a/ddprof-lib/src/main/cpp/mallocInfo.cpp b/ddprof-lib/src/main/cpp/mallocInfo.cpp index 9b6d4d8361..2660c92216 100644 --- a/ddprof-lib/src/main/cpp/mallocInfo.cpp +++ b/ddprof-lib/src/main/cpp/mallocInfo.cpp @@ -5,6 +5,14 @@ #include "mallocInfo.h" #include +#if defined(__linux__) +// The glibc guard below must see __GLIBC_PREREQ here, not via a transitive +// include: if ever stopped pulling in , the guard would +// silently turn false and the mirror pins would stop compiling on every +// glibc build. exists on musl too, so the include is unconditional +// on Linux. +#include +#endif // mallinfo2() replaced the int-based mallinfo() in glibc 2.33. The release // libraries are built against older glibc headers (the el8 image has 2.28), @@ -70,13 +78,23 @@ struct Mallinfo2 { bool mallocArenaSnapshot(MallocArenaInfo *out) { typedef Mallinfo2 (*mallinfo2_fn)(); - // Not cached: called once per JFR chunk flush, so the dlsym() lookup is - // negligible (same trade-off as glibcMallocActive()). - mallinfo2_fn fn = (mallinfo2_fn)dlsym(RTLD_DEFAULT, "mallinfo2"); - if (fn == NULL) { + // Resolved once. The RTLD_DEFAULT lookup result cannot change after libc is + // loaded, and the loader lock is not worth taking on every chunk flush while + // application threads may be inside dlopen(). The pair is written racily but + // idempotently: dlsym() is thread-safe and every thread stores the same + // value, so no lock is needed; a thread that races the very first lookup can + // only see NULL for one flush (counters unset for that chunk, self-healing). + // Flush path only, never signal context, so plain statics are legal here. + static mallinfo2_fn cached_fn = NULL; + static bool resolved = false; + if (!resolved) { + cached_fn = (mallinfo2_fn)dlsym(RTLD_DEFAULT, "mallinfo2"); + resolved = true; + } + if (cached_fn == NULL) { return false; } - Mallinfo2 mi = fn(); + Mallinfo2 mi = cached_fn(); out->arena = mi.arena; out->hblkhd = mi.hblkhd; out->uordblks = mi.uordblks; diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java index a46be93141..b1d1de0fe4 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/nativemem/MallocArenaCountersTest.java @@ -10,6 +10,7 @@ import com.datadoghq.profiler.AbstractProfilerTest; import java.io.BufferedReader; +import java.io.IOException; import java.io.InputStreamReader; import java.util.Locale; import java.util.concurrent.TimeUnit; @@ -155,41 +156,68 @@ private void busyLoop() { * 2.35} or {@code ldd (GNU libc) 2.17}). The trailing token is required: distro package versions * elsewhere in the banner also contain {@code major.minor} pairs (Ubuntu's {@code * 2.31-0ubuntu9.16} ends with {@code 9.16}), and picking one of those misclassifies the glibc - * 2.33 gate. Returns {@code null} when the runtime is not glibc (musl prints a version without a - * GLIBC marker; the probe is not attempted at all on non-Linux platforms). stderr is merged into - * stdout (musl's ldd prints its banner to stderr), the child is reaped under a deadline before - * anything is read, and it is always destroyed, whatever the exit path. + * 2.33 gate. Returns {@code null} only when the runtime is genuinely not glibc (musl prints a + * version without a GLIBC marker; the probe is not attempted at all on non-Linux platforms). A + * probe failure (ldd missing, timeout, interrupted, unparsable banner) is inconclusive, not a + * negative: it skips the test via an assumption so it can never assert the counters-are-unset + * branch against a modern glibc by mistake. The oracle assumes the JVM was loaded against the + * system glibc -- the case in every supported test image; a bundled-sysroot environment is + * unsupported. stderr is merged into stdout (musl's ldd prints its banner to stderr), the child + * is reaped under a deadline before anything is read, and it is always destroyed, whatever the + * exit path. */ - private static int[] runtimeGlibcVersion() throws Exception { + private static int[] runtimeGlibcVersion() { if (!System.getProperty("os.name", "").startsWith("Linux")) { return null; } - Process process = new ProcessBuilder("ldd", "--version").redirectErrorStream(true).start(); + Process process; + try { + process = new ProcessBuilder("ldd", "--version").redirectErrorStream(true).start(); + } catch (IOException e) { + // A missing ldd makes the probe inconclusive, not negative: skipping is the only + // outcome that can never mask a counter regression behind a wrong assertion. + return inconclusive("ldd could not be started", e); + } String firstLine; try { // Wait first, then read: the exit deadline also bounds the read. ldd's output is a few // lines, far below the pipe buffer, so it cannot block the child on a full pipe while // we wait. Reading first would block unboundedly on a child that prints nothing. if (!process.waitFor(10, TimeUnit.SECONDS)) { - return null; + return inconclusive("ldd did not exit within the deadline", null); } try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()))) { firstLine = reader.readLine(); } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return inconclusive("ldd probe interrupted", e); + } catch (IOException e) { + return inconclusive("ldd output could not be read", e); } finally { process.destroyForcibly(); } if (firstLine == null) { - return null; + return inconclusive("ldd produced no output", null); } if (firstLine.contains("musl") || firstLine.contains("FreeBSD")) { return null; } - Matcher matcher = Pattern.compile("\\b(\\d+)\\.(\\d+)\\s*$").matcher(firstLine); + // The full-token boundary rejects three-component trailing versions such as a + // pre-release "2.38.9000": "38.9000" there is preceded by a dot, not a token boundary, + // so an ambiguous banner is inconclusive instead of parsing to a wrong version. + Matcher matcher = Pattern.compile("(?:^|\\s)(\\d+)\\.(\\d+)\\s*$").matcher(firstLine); if (!matcher.find()) { - return null; + return inconclusive("ldd banner has no trailing major.minor token: " + firstLine, null); } return new int[] {Integer.parseInt(matcher.group(1)), Integer.parseInt(matcher.group(2))}; } + + /** Fails the assumption, skipping the test: the probe outcome is unknown, not negative. */ + private static int[] inconclusive(String reason, Exception cause) { + String message = reason + (cause == null ? "" : ": " + cause); + Assumptions.assumeTrue(false, message); + return null; // unreachable: assumeTrue(false) always aborts + } } From 51338065e8bb275b09275db29a65220ada980d3c Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 12 Nov 2026 15:53:34 +0100 Subject: [PATCH 9/9] Publish the cached dlsym results with acquire/release instead of plain statics (PROF-16350) --- ddprof-lib/src/main/cpp/flightRecorder.cpp | 16 ++++++++------- ddprof-lib/src/main/cpp/mallocInfo.cpp | 24 ++++++++++++---------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 0bcac12f3d..d3826685db 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -2134,21 +2134,23 @@ void Recording::writeLogLevels(Buffer *buf) { // mallctl/tc_malloc_size, so the dlsym probe alone would miss them. // // Resolved once: the lookup result cannot change after process start, and the -// loader lock is not worth taking on every chunk flush. The sentinel makes the -// first call racily idempotent -- dlsym() is thread-safe and all threads store -// the same verdict -- so no lock is needed. Flush path only, never signal -// context, so plain statics are legal here. +// loader lock is not worth taking on every chunk flush. The sentinel is zero- +// initialized (no dynamic-init guard) and read/written with acquire/release, +// so a racing first caller can only repeat the same lookup. Flush path only, +// never signal context. bool Recording::glibcMallocActive() { #if defined(ASAN_ENABLED) || defined(TSAN_ENABLED) return false; #else static int cached = 0; // 0 = unresolved, 1 = active, 2 = inactive - if (cached == 0) { + int state = __atomic_load_n(&cached, __ATOMIC_ACQUIRE); + if (state == 0) { bool active = dlsym(RTLD_DEFAULT, "mallctl") == nullptr && dlsym(RTLD_DEFAULT, "tc_malloc_size") == nullptr; - cached = active ? 1 : 2; + state = active ? 1 : 2; + __atomic_store_n(&cached, state, __ATOMIC_RELEASE); } - return cached == 1; + return state == 1; #endif } diff --git a/ddprof-lib/src/main/cpp/mallocInfo.cpp b/ddprof-lib/src/main/cpp/mallocInfo.cpp index 2660c92216..1f5578b028 100644 --- a/ddprof-lib/src/main/cpp/mallocInfo.cpp +++ b/ddprof-lib/src/main/cpp/mallocInfo.cpp @@ -80,21 +80,23 @@ bool mallocArenaSnapshot(MallocArenaInfo *out) { typedef Mallinfo2 (*mallinfo2_fn)(); // Resolved once. The RTLD_DEFAULT lookup result cannot change after libc is // loaded, and the loader lock is not worth taking on every chunk flush while - // application threads may be inside dlopen(). The pair is written racily but - // idempotently: dlsym() is thread-safe and every thread stores the same - // value, so no lock is needed; a thread that races the very first lookup can - // only see NULL for one flush (counters unset for that chunk, self-healing). - // Flush path only, never signal context, so plain statics are legal here. + // application threads may be inside dlopen(). The statics are zero- + // initialized (no dynamic-init guard), and the handoff is published with + // release/acquire: dlsym() is thread-safe, so racing first callers compute + // the same value; a concurrent reader can only repeat the lookup once. + // Flush path only, never signal context. static mallinfo2_fn cached_fn = NULL; - static bool resolved = false; - if (!resolved) { - cached_fn = (mallinfo2_fn)dlsym(RTLD_DEFAULT, "mallinfo2"); - resolved = true; + static int resolved = 0; + if (__atomic_load_n(&resolved, __ATOMIC_ACQUIRE) == 0) { + __atomic_store_n(&cached_fn, (mallinfo2_fn)dlsym(RTLD_DEFAULT, "mallinfo2"), + __ATOMIC_RELEASE); + __atomic_store_n(&resolved, 1, __ATOMIC_RELEASE); } - if (cached_fn == NULL) { + mallinfo2_fn fn = __atomic_load_n(&cached_fn, __ATOMIC_ACQUIRE); + if (fn == NULL) { return false; } - Mallinfo2 mi = cached_fn(); + Mallinfo2 mi = fn(); out->arena = mi.arena; out->hblkhd = mi.hblkhd; out->uordblks = mi.uordblks;