diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4cc37a483..2665254670 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -156,6 +156,47 @@ jobs: run: | ./gradlew spotlessCheck --no-daemon --parallel --build-cache --no-watch-fs + source-invariant-checks: + runs-on: ubuntu-22.04 + needs: check-for-pr + if: needs.check-for-pr.outputs.skip != 'true' + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + + - name: Setup Java + uses: actions/setup-java@b843ebf9b913146b38da2eaa7aeddf76074ae1ad # v6.1.0 + with: + distribution: 'zulu' + java-version: '21' + + - name: Setup OS + run: | + sudo apt-get update + sudo apt-get install -y curl zip unzip binutils g++ python3 + + - name: Cache Gradle Wrapper Binaries + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ~/.gradle/wrapper/dists + key: gradle-wrapper-${{ runner.os }}-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties') }} + restore-keys: | + gradle-wrapper-${{ runner.os }}- + + - name: Cache Gradle User Home + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ~/.gradle/caches + key: gradle-caches-${{ runner.os }}-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} + restore-keys: | + gradle-caches-${{ runner.os }}- + + - name: Source invariant checks + run: | + # Signal-reachability check (ELF debug objects + fixture self-test) and the utils/check-*.py source checks + ./gradlew :ddprof-lib:qualityChecks --no-daemon --build-cache --no-watch-fs + check-javadoc: runs-on: ubuntu-22.04 needs: check-for-pr diff --git a/.github/workflows/codecheck.yml b/.github/workflows/codecheck.yml index a79d173712..cac89fda20 100644 --- a/.github/workflows/codecheck.yml +++ b/.github/workflows/codecheck.yml @@ -29,7 +29,7 @@ jobs: sudo apt-get update sudo apt install -y clang clang-tools openjdk-11-jdk - name: Set up Python - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0 with: python-version: 3.14 - name: Install Python dependencies diff --git a/AGENTS.md b/AGENTS.md index 03931677da..a574e785fa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -343,6 +343,14 @@ exception list. helpers unless reused or genuinely hard to follow; "why" comments must reference concrete mechanisms (e.g. "ASAN's allocator lock is reentrant"), never speculative claims about HotSpot/glibc/ASAN internals — if unsure, describe the observable symptom. +- **RAII for every acquire/release pair** (C/C++). A lock, a heap allocation, or a flag + that must be restored is released by a guard or owner destructor, not by a manual call + at each exit. Reuse `SharedLockGuard`, `ExclusiveLockGuard`, `OptionalSharedLockGuard`, + `MutexLocker`, `CriticalSection` before you add a type. A new owner type is non-copyable + and hands over ownership explicitly (the receiver sets the owner's pointer to `nullptr`). + A manual pair needs a comment that says why a guard scope cannot express it. In code that + shares a lock with signal handlers, end an owner that frees memory outside the exclusive + critical section, because its destructor frees at scope end. - Always challenge proposals with deep analysis when you have hard evidence. ## Work Tracking diff --git a/ddprof-lib/build.gradle.kts b/ddprof-lib/build.gradle.kts index ff194cccf6..134431336a 100644 --- a/ddprof-lib/build.gradle.kts +++ b/ddprof-lib/build.gradle.kts @@ -183,6 +183,93 @@ afterEvaluate { } } +val debugObjDir: File = layout.buildDirectory.dir("obj/main/debug").get().asFile +val signalReachabilityScript: File = rootProject.file("utils/check-signal-reachability.py") + +// The reachability checker reads ELF objects only (Linux); Mach-O objects are skipped. +fun hasElfObjects(dir: File): Boolean { + val obj = dir.listFiles { f -> f.name.endsWith(".o") }?.firstOrNull() ?: return false + val magic = ByteArray(4) + return obj.inputStream().use { it.read(magic) } == 4 && + magic.contentEquals(byteArrayOf(0x7f, 'E'.code.toByte(), 'L'.code.toByte(), 'F'.code.toByte())) +} + +// Static async-signal-safety reachability check over the debug objects; see +// utils/check-signal-reachability.py. Not wired into check/build: run +// `./gradlew :ddprof-lib:checkSignalReachability` (fails fast), or via +// qualityChecks. The checker reads ELF objects only, so the task is skipped +// when the debug objects are absent or not ELF. +tasks.register("checkSignalReachability") { + group = "verification" + description = "Checks that no async-signal-unsafe function is reachable from the signal handlers and malloc hooks" + // Compile task exists for every active configuration; named lookup fails loudly on rename. + dependsOn(tasks.named("compileDebug")) + + commandLine("python3", signalReachabilityScript.absolutePath, "--obj-dir", debugObjDir.absolutePath) + + onlyIf("debug objects are ELF") { hasElfObjects(debugObjDir) } +} + +// Quality checks: the signal-reachability check (ELF hosts only) and its +// fixture self-test (Linux only), plus the source-level utils/check-*.py +// scripts, each also runnable on its own. Not wired into check/build; CI runs +// `./gradlew :ddprof-lib:qualityChecks` in the source-invariant-checks job of +// .github/workflows/ci.yml. Runs every check, collects every failure, and +// fails once at the end. +interface QualityCheckServices { + @get:Inject val exec: ExecOperations +} + +tasks.register("qualityChecks") { + group = "verification" + description = "Runs the signal-reachability check and the source-level quality checks in utils/" + // Depends on the debug objects, not on checkSignalReachability: that Exec + // task fails fast, which would stop the remaining checks from running. + dependsOn(tasks.named("compileDebug")) + + val exec = objects.newInstance().exec + val root = rootProject.projectDir + val scripts = listOf( + "check-jvm-boundary-includes.py", + "check-doc-index.py", + "check-sigsetjmp-shape.py", + "check-tracked-artifacts.py", + ).map { File(root, "utils/$it") } + + doLast { + val failed = mutableListOf() + // Compile-and-assert the planted-violation fixture first: it pins the + // checker's most fragile assumptions (section symbols, ctor aliases, TLS, + // std-library attribution) and catches toolchain/objdump drift. The + // checker is ELF-only, so the self-test runs on Linux hosts only (macOS + // produces Mach-O fixture objects by design). + val onLinux = System.getProperty("os.name").lowercase().contains("linux") + if (onLinux) { + exec.exec { + commandLine("python3", signalReachabilityScript.absolutePath, "--self-test") + isIgnoreExitValue = true + }.exitValue.takeIf { it != 0 }?.let { failed += "self-test (check-signal-reachability.py --self-test)" } + } + if (hasElfObjects(debugObjDir)) { + exec.exec { + commandLine("python3", signalReachabilityScript.absolutePath, "--obj-dir", debugObjDir.absolutePath) + isIgnoreExitValue = true + }.exitValue.takeIf { it != 0 }?.let { failed += "check-signal-reachability.py" } + } else { + logger.lifecycle("qualityChecks: check-signal-reachability.py skipped (debug objects are not ELF)") + } + failed += scripts.filter { script -> + exec.exec { + commandLine("python3", script.absolutePath, "--root", root.absolutePath) + isIgnoreExitValue = true + }.exitValue != 0 + }.map { it.name } + if (failed.isNotEmpty()) { + throw GradleException("quality checks failed: ${failed.joinToString { it }}") + } + } +} + // Add runBenchmarks task tasks.register("runBenchmarks") { dependsOn(":ddprof-lib:benchmarks:runBenchmark") diff --git a/ddprof-lib/src/main/cpp/codeCache.cpp b/ddprof-lib/src/main/cpp/codeCache.cpp index 943e7481ff..4845f053f7 100644 --- a/ddprof-lib/src/main/cpp/codeCache.cpp +++ b/ddprof-lib/src/main/cpp/codeCache.cpp @@ -176,8 +176,8 @@ CodeCache::~CodeCache() { } long long CodeCache::memoryUsage() const { - // O(1): _memory_usage is updated incrementally by the constructor, add(), - // expand(), and setDwarfTable() -- the same pre-publication mutators that + // O(1): _memory_usage is updated incrementally by the constructor, + // add()/commitAdd(), and setDwarfTable() -- the same pre-publication mutators that // used to be re-summed here on every call. Safe to read lock-free once // published (Libraries::native_libs) because those mutators never run // afterwards (asserted via _published), same invariant the old per-call @@ -191,55 +191,76 @@ long long CodeCache::memoryUsage() const { return _memory_usage; } -void CodeCache::expand() { - // Must not run after publication: memoryUsage() reads _blobs lock-free from - // the dump thread and a concurrent realloc would free it underneath. - assert(!_published.load(std::memory_order_acquire) && - "expand() on a published CodeCache races memoryUsage()"); - CodeBlob *old_blobs = _blobs; - CodeBlob *new_blobs = new CodeBlob[_capacity * 2]; - - memcpy(new_blobs, old_blobs, _count * sizeof(CodeBlob)); - - // Delta is exactly the current (pre-doubling) capacity's worth of blobs. - _memory_usage += (long long)_capacity * sizeof(CodeBlob); - _capacity *= 2; - _blobs = new_blobs; - delete[] old_blobs; -} - -void CodeCache::add(const void *start, int length, const char *name, - bool update_bounds) { +void CodeCache::prepareAdd(const char *name, PreparedAdd &prepared) const { // Symbols are added while parsing a library, before it is registered into a // CodeCacheArray. Adding after publication would race memoryUsage() (which // reads _blobs/_count lock-free at dump time). Unpublished standalone caches // (e.g. _runtime_stubs) are exempt — they are never published. assert(!_published.load(std::memory_order_acquire) && "add() on a published CodeCache races memoryUsage()"); + // A reused PreparedAdd would leak the allocations it still owns. + assert(prepared.name == nullptr && prepared.grown == nullptr); char *name_copy = NativeFunc::create(name, _lib_index); - _memory_usage += (long long)NativeFunc::allocSize(name); + prepared.name = name_copy; + prepared.name_size = (long long)NativeFunc::allocSize(name); // Measured off the block just handed back, while the pointer is in hand: the // gauge is published as a total at add() time, with no pointer left to probe. - _name_overhead += (long long)NativeFunc::nameOverhead(name_copy); + prepared.name_overhead = (long long)NativeFunc::nameOverhead(name_copy); // Replace non-printable characters for (char *s = name_copy; *s != 0; s++) { if (*s < ' ') *s = '?'; } + prepared.capacity = _capacity; + prepared.grown = nullptr; if (_count >= _capacity) { - expand(); + prepared.grown = new CodeBlob[_capacity * 2]; + memcpy(prepared.grown, _blobs, _count * sizeof(CodeBlob)); + } +} + +CodeBlob *CodeCache::commitAdd(const void *start, int length, + PreparedAdd &prepared, + bool update_bounds) { + assert(!_published.load(std::memory_order_acquire) && + "add() on a published CodeCache races memoryUsage()"); + // A mutation between prepareAdd() and commitAdd() would make the grown copy + // stale or leave no free slot. + assert(prepared.capacity == _capacity && + (prepared.grown != nullptr || _count < _capacity)); + CodeBlob *retired = nullptr; + if (prepared.grown != nullptr) { + // Delta is exactly the current (pre-doubling) capacity's worth of blobs. + _memory_usage += (long long)_capacity * sizeof(CodeBlob); + _capacity *= 2; + retired = _blobs; + _blobs = prepared.grown; } + _memory_usage += prepared.name_size; + _name_overhead += prepared.name_overhead; const void *end = (const char *)start + length; _blobs[_count]._start = start; _blobs[_count]._end = end; - _blobs[_count]._name = name_copy; + _blobs[_count]._name = prepared.name; _count++; + // The cache owns both allocations now; the PreparedAdd destructor must not + // free them. + prepared.name = nullptr; + prepared.grown = nullptr; if (update_bounds) { updateBounds(start, end); } + return retired; +} + +void CodeCache::add(const void *start, int length, const char *name, + bool update_bounds) { + PreparedAdd prepared; + prepareAdd(name, prepared); + delete[] commitAdd(start, length, prepared, update_bounds); } void CodeCache::updateBounds(const void *start, const void *end) { diff --git a/ddprof-lib/src/main/cpp/codeCache.h b/ddprof-lib/src/main/cpp/codeCache.h index 951cfc610a..6ced816af8 100644 --- a/ddprof-lib/src/main/cpp/codeCache.h +++ b/ddprof-lib/src/main/cpp/codeCache.h @@ -181,7 +181,7 @@ class CodeCache { CodeBlob *_blobs; // Running total of memoryUsage()'s formula, updated incrementally by the - // same pre-publication mutators (constructor, add(), expand(), + // same pre-publication mutators (constructor, add()/commitAdd(), // setDwarfTable()) instead of recomputed by iterating _blobs. Safe because // every contributing field is fixed before publication (see _published // below) -- there is no point after construction-and-population where this @@ -204,17 +204,17 @@ class CodeCache { // Set once the cache is registered into a CodeCacheArray (see markPublished()). // After that, memoryUsage() may be read lock-free from another thread (dump), - // so the mutators that touch the fields it reads (add()/expand()/ - // setDwarfTable()) must not run — asserted in debug. Standalone caches that + // so the mutators that touch the fields it reads (add()/prepareAdd()/ + // commitAdd()/setDwarfTable()) must not run — asserted in debug. Standalone caches that // are never registered (e.g. JitCodeCache::_runtime_stubs, which is instead - // guarded by its own JitCodeCache::_stubs_lock) stay unpublished, so the + // guarded by its own JitCodeCache::_stubs_lock, with writers serialized by + // JitCodeCache::_stubs_write_lock) stay unpublished, so the // asserts never fire for them. Atomic with release/acquire so the flag is // observably true on any thread that reaches a published cache — the store is // sequenced before CodeCacheArray::add()'s RELEASE publish of the pointer, and // the assert loads it with acquire — without a formal data race. std::atomic _published; - void expand(); void makeImportsPatchable(); void saveImport(ImportId id, void** entry); void copyFrom(const CodeCache& other); @@ -282,12 +282,47 @@ class CodeCache { void setImageFingerprint(u64 fingerprint) { _image_fingerprint = fingerprint; } // Mark this cache as published into a CodeCacheArray. Call before the array - // makes the pointer visible to readers; afterwards add()/expand()/ - // setDwarfTable() must not be called (see _published). + // makes the pointer visible to readers; afterwards add()/prepareAdd()/ + // commitAdd()/setDwarfTable() must not be called (see _published). void markPublished() { _published.store(true, std::memory_order_release); } void add(const void *start, int length, const char *name, bool update_bounds = false); + + // Heap allocations that prepareAdd() makes for one later commitAdd(). + // commitAdd() takes ownership of name and grown and sets them to nullptr. + // If no commitAdd() happens, the destructor frees them. + struct PreparedAdd { + char *name = nullptr; // NativeFunc name copy, non-printables replaced + long long name_size = 0; // NativeFunc::allocSize() of the name + long long name_overhead = 0; // NativeFunc::nameOverhead() of the copy + CodeBlob *grown = nullptr; // filled copy of _blobs at twice the capacity, or + // nullptr when the current array has a free slot + int capacity = 0; // _capacity that this preparation is valid for + + PreparedAdd() = default; + ~PreparedAdd() { + if (name != nullptr) { + NativeFunc::destroy(name); + } + delete[] grown; + } + // Non-copyable: a copy would free the same allocations twice. + PreparedAdd(const PreparedAdd &) = delete; + PreparedAdd &operator=(const PreparedAdd &) = delete; + }; + + // Two-phase add() for a cache whose readers run in signal handlers under a + // SpinLock (JitCodeCache::_runtime_stubs): prepareAdd() makes every heap + // allocation and can run outside the lock; commitAdd() only stores and + // copies pointers, so it is safe under the exclusive lock. commitAdd() + // returns the replaced blob array (or nullptr), which the caller must + // delete[] after it releases the lock. Between prepareAdd() and commitAdd() + // nothing else may mutate this cache; the caller serializes the pair + // against other writers. Pass a freshly constructed PreparedAdd. + void prepareAdd(const char *name, PreparedAdd &prepared) const; + CodeBlob *commitAdd(const void *start, int length, PreparedAdd &prepared, + bool update_bounds = false); void updateBounds(const void *start, const void *end); void sort(); @@ -407,7 +442,7 @@ class CodeCacheArray { // so it stays consistent with the logical bytes above. NativeMem::recordOverhead(NM_NATIVE_SYMBOLS, lib->nameOverhead()); // Mark published before the RELEASE store makes the pointer visible, so any - // later add()/expand()/setDwarfTable() on this cache trips the assert (its + // later add()/prepareAdd()/commitAdd()/setDwarfTable() on this cache trips the assert (its // _blobs would then be read lock-free by memoryUsage() at dump time). lib->markPublished(); // Store pointer before publishing count. The RELEASE here pairs with diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index 950ce1fb49..ad58859eb3 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -42,6 +42,7 @@ X(DICTIONARY_ENDPOINTS_ARENA_WASTE_BYTES, "dictionary_endpoints_arena_waste_bytes") \ X(DICTIONARY_CONTEXT_ARENA_WASTE_BYTES, "dictionary_context_arena_waste_bytes") \ X(DICTIONARY_DRAIN_TIMEOUTS, "dictionary_drain_timeouts") \ + X(REFCOUNT_NESTED_OVERFLOW, "refcount_nested_overflow") \ X(DICTIONARY_ID_CONFLICTS, "dictionary_id_conflicts") \ X(CLASS_MAP_AT_CAPACITY, "class_map_at_capacity_drops") \ X(CONTEXT_STORAGE_BYTES, "context_storage_bytes") \ @@ -135,6 +136,11 @@ X(WALKVM_STUB_INFO_HIT, "walkvm_stub_info_hit") \ X(WALKVM_STUB_INFO_FALLBACK, "walkvm_stub_info_fallback") \ X(WALKVM_STUB_INFO_REGISTRATION_FAILED, "walkvm_stub_info_registration_failed") \ + /* Runtime-stub lookups (JitCodeCache::findRuntimeStub/findRuntimeStubInfo) \ + * that returned "not found" without searching because a registration held \ + * the stubs lock exclusively; the stack walk continued with its \ + * stub-not-found fallback. */ \ + X(STUB_LOOKUP_LOCK_UNAVAILABLE, "stub_lookup_lock_unavailable") \ X(WALKVM_FP_CHAIN_ATTEMPT, "walkvm_fp_chain_attempt") \ X(WALKVM_FP_CHAIN_REACHED_CODEHEAP, "walkvm_fp_chain_reached_codeheap") \ X(WALKVM_ANCHOR_NOT_IN_JAVA, "walkvm_anchor_not_in_java") \ diff --git a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp index 52bf894354..92598afdc9 100644 --- a/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp @@ -761,9 +761,9 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex } } - // One shared-lock acquisition returns the stub view plus the - // precomputed aarch64 unwind info (null on x64): the signal - // handler still takes _stubs_lock exactly once. + // At most one bounded shared-lock attempt returns the stub view + // plus the precomputed aarch64 unwind info (null on x64). When + // the lookup fails, the nmethod's own start and name are used. const void* start; const char* name; const StubUnwindInfo* stub_info = nullptr; @@ -773,9 +773,10 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex name = nm->name(); } #else - CodeBlob* stub = JitCodeCache::findRuntimeStub(walk_pc.raw()); - start = stub != NULL ? stub->_start : nm->code(); - name = stub != NULL ? stub->_name : nm->name(); + if (!JitCodeCache::findRuntimeStub(walk_pc.raw(), &start, &name)) { + start = nm->code(); + name = nm->name(); + } #endif if (details) { @@ -1170,10 +1171,7 @@ int HotspotSupport::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, #ifdef __aarch64__ if (JitCodeCache::findRuntimeStubInfo((const void *)frame.pc(), &stub_start, &stub_name, &stub_info)) { #else - CodeBlob *stub = JitCodeCache::findRuntimeStub((const void *)frame.pc()); - if (stub != NULL) { - stub_start = stub->_start; - stub_name = stub->_name; + if (JitCodeCache::findRuntimeStub((const void *)frame.pc(), &stub_start, &stub_name)) { #endif if (cstack != CSTACK_NO) { max_depth -= makeFrame(trace.frames++, BCI_NATIVE_FRAME, stub_name); diff --git a/ddprof-lib/src/main/cpp/hotspot/jitCodeCache.cpp b/ddprof-lib/src/main/cpp/hotspot/jitCodeCache.cpp index df75e6cf0b..c314e5cc3c 100644 --- a/ddprof-lib/src/main/cpp/hotspot/jitCodeCache.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/jitCodeCache.cpp @@ -8,6 +8,7 @@ #include #include +#include "counters.h" #include "hotspot/vmStructs.h" #include "threadLocalData.h" #ifdef __aarch64__ @@ -15,6 +16,7 @@ #endif SpinLock JitCodeCache::_stubs_lock; +Mutex JitCodeCache::_stubs_write_lock; CodeCache JitCodeCache::_runtime_stubs("[stubs]"); std::atomic JitCodeCache::_call_stub_begin = { nullptr }; std::atomic JitCodeCache::_call_stub_end = { nullptr }; @@ -24,11 +26,12 @@ std::atomic JitCodeCache::_call_stub_end = { nullptr }; // signal-handler lookup is a binary search. Inserts happen under the // exclusive _stubs_lock and each entry's info pointer is fixed at // registration time, so a reader holding the shared lock sees a stable -// snapshot: the same lock serializes the growing realloc with the scan, and -// the full-barrier CAS in SpinLock publishes the new contents. Info objects -// are malloc'ed once at DynamicCodeGenerated time and never freed -- a signal -// handler may still be reading one -- matching the lifetime of the stub name -// strings. +// snapshot. A full array is replaced by a larger copy, allocated and filled +// before the exclusive lock and swapped in under it; the old array is freed +// only after the unlock, when no reader can still hold it. The full-barrier +// CAS in SpinLock publishes the new contents. Published info objects are +// never freed -- a signal handler may still be reading one -- matching the +// lifetime of the stub name strings. StubUnwindEntry* JitCodeCache::_stub_unwind = nullptr; int JitCodeCache::_stub_unwind_count = 0; int JitCodeCache::_stub_unwind_capacity = 0; @@ -61,10 +64,10 @@ void JNICALL JitCodeCache::DynamicCodeGenerated(jvmtiEnv *jvmti, const char *nam const void *address, jint length) { ProfiledThread::initCurrentThreadSignalSafe(); #ifdef __aarch64__ - // Analyze before taking the lock: the classifier decodes every instruction - // and allocates, which must not happen while sampling threads spin on the - // shared lock. Publication is ordered by the exclusive lock below -- a - // reader that finds the stub always finds its info. + // Analyze before taking either lock: the classifier decodes every + // instruction and allocates, but reads only the stub code. Publication is + // ordered by the exclusive lock below -- a reader that finds the stub always + // finds its info. StubUnwindInfo* stub_info = nullptr; if (shouldAnalyzeStub(name)) { // Only the adapters blob is known to pack several entry points behind @@ -76,14 +79,23 @@ void JNICALL JitCodeCache::DynamicCodeGenerated(jvmtiEnv *jvmti, const char *nam : WALKVM_STUB_INFO_UNCLASSIFIED); } #endif - _stubs_lock.lock(); - _runtime_stubs.add(address, length, name, true); + // Registrations can arrive concurrently from several threads. The writer + // mutex orders them, so the state read while preparing below stays valid + // until the commit. Signal handlers never take it. + MutexLocker writer(_stubs_write_lock); + // Every heap allocation and free happens outside the exclusive lock. The + // bounded readers already prevent a deadlock: while the lock is held + // exclusively, a stub lookup returns false and the stack walk uses its + // fallback unwind. Keeping the allocator out of the exclusive section keeps + // that window short: the registering thread cannot block in the allocator + // while it holds _stubs_lock. + CodeCache::PreparedAdd prepared; + _runtime_stubs.prepareAdd(name, prepared); #ifdef __aarch64__ // A re-registration of the same range (never observed for HotSpot runtime // stubs, but DynamicCodeGenerated makes no promises) keeps the previously // registered info: the old object cannot be freed because a signal handler // may still hold a pointer to it. - bool duplicate = false; int lo = 0, hi = _stub_unwind_count; while (lo < hi) { int mid = (lo + hi) / 2; @@ -93,38 +105,62 @@ void JNICALL JitCodeCache::DynamicCodeGenerated(jvmtiEnv *jvmti, const char *nam hi = mid; } } - if (lo < _stub_unwind_count && _stub_unwind[lo]._start == address) { - duplicate = true; - } - if (!duplicate && _stub_unwind_count == _stub_unwind_capacity) { - int new_capacity = _stub_unwind_capacity == 0 ? 64 : _stub_unwind_capacity * 2; - StubUnwindEntry* grown = - (StubUnwindEntry*)realloc(_stub_unwind, new_capacity * sizeof(StubUnwindEntry)); - if (grown != nullptr) { - _stub_unwind = grown; - _stub_unwind_capacity = new_capacity; - } - } - if (!duplicate && _stub_unwind_count < _stub_unwind_capacity) { - // Sorted insert: shift the tail right to keep the lookup binary-searchable. - memmove(&_stub_unwind[lo + 1], &_stub_unwind[lo], - (_stub_unwind_count - lo) * sizeof(StubUnwindEntry)); - _stub_unwind[lo]._start = address; - _stub_unwind[lo]._info = stub_info; - _stub_unwind_count++; - } else { - if (duplicate) { - // The freshly analyzed info is redundant; the registered one stays. - free(stub_info); + bool insert = !(lo < _stub_unwind_count && _stub_unwind[lo]._start == address); + StubUnwindEntry* grown_unwind = nullptr; + int grown_capacity = 0; + if (insert && _stub_unwind_count == _stub_unwind_capacity) { + grown_capacity = _stub_unwind_capacity == 0 ? 64 : _stub_unwind_capacity * 2; + grown_unwind = (StubUnwindEntry*)malloc(grown_capacity * sizeof(StubUnwindEntry)); + if (grown_unwind != nullptr) { + // _stub_unwind is null with zero count on the first growth; memcpy + // requires a valid source even for zero bytes. + if (_stub_unwind_count > 0) { + memcpy(grown_unwind, _stub_unwind, _stub_unwind_count * sizeof(StubUnwindEntry)); + } } else { // Capacity could not be grown: this stub (and every later one) loses // precomputed unwinding silently unless the outcome is counted. Counters::increment(WALKVM_STUB_INFO_REGISTRATION_FAILED); + insert = false; } + } + if (!insert) { + // Either a duplicate (the registered info stays) or a failed growth. The + // info was never published, so no reader can hold it. + free(stub_info); stub_info = nullptr; } + StubUnwindEntry* retired_unwind = nullptr; +#endif + + CodeBlob* retired_blobs; + { + // The scope ends before the frees below: nothing in it allocates or frees. + ExclusiveLockGuard publish(&_stubs_lock); + retired_blobs = _runtime_stubs.commitAdd(address, length, prepared, true); +#ifdef __aarch64__ + if (grown_unwind != nullptr) { + retired_unwind = _stub_unwind; + _stub_unwind = grown_unwind; + _stub_unwind_capacity = grown_capacity; + } + if (insert) { + // Sorted insert: shift the tail right to keep the lookup binary-searchable. + memmove(&_stub_unwind[lo + 1], &_stub_unwind[lo], + (_stub_unwind_count - lo) * sizeof(StubUnwindEntry)); + _stub_unwind[lo]._start = address; + _stub_unwind[lo]._info = stub_info; + _stub_unwind_count++; + } +#endif + } + + // Readers take the shared lock for every access, so after the unlock no + // reader can still use the replaced arrays. + delete[] retired_blobs; +#ifdef __aarch64__ + free(retired_unwind); #endif - _stubs_lock.unlock(); if (name[0] == 'I' && strcmp(name, "Interpreter") == 0) { CodeHeap::setInterpreterStart(address); @@ -138,22 +174,41 @@ void JNICALL JitCodeCache::DynamicCodeGenerated(jvmtiEnv *jvmti, const char *nam CodeHeap::updateBounds(address, (const char *)address + length); } -CodeBlob* JitCodeCache::findRuntimeStub(const void *address) { - CodeBlob *stub = nullptr; - _stubs_lock.lockShared(); - if (_runtime_stubs.contains(address)) { - stub = _runtime_stubs.findBlobByAddress(address); +bool JitCodeCache::findRuntimeStub(const void *address, const void **start, + const char **name) { + // Fails only while a writer holds the lock: the caller is a signal handler, + // and the writer may be the interrupted thread itself. Competing readers do + // not make it fail, because each failed CAS means another reader got in. + OptionalSharedLockGuard guard(&_stubs_lock, SpinLock::NO_SPIN_BUDGET); + if (!guard.ownsLock()) { + Counters::increment(STUB_LOOKUP_LOCK_UNAVAILABLE); + return false; } - _stubs_lock.unlockShared(); - return stub; + if (!_runtime_stubs.contains(address)) { + return false; + } + CodeBlob *blob = _runtime_stubs.findBlobByAddress(address); + if (blob == nullptr) { + return false; + } + // Copy out under the lock: a registration may replace the blob array as + // soon as the lock is released. + *start = blob->_start; + *name = blob->_name; + return true; } #ifdef __aarch64__ bool JitCodeCache::findRuntimeStubInfo(const void* address, const void** start, const char** name, const StubUnwindInfo** info) { - bool found = false; *info = nullptr; - _stubs_lock.lockShared(); + // Same lock acquisition as findRuntimeStub(). + OptionalSharedLockGuard guard(&_stubs_lock, SpinLock::NO_SPIN_BUDGET); + if (!guard.ownsLock()) { + Counters::increment(STUB_LOOKUP_LOCK_UNAVAILABLE); + return false; + } + bool found = false; if (_runtime_stubs.contains(address)) { CodeBlob* blob = _runtime_stubs.findBlobByAddress(address); if (blob != nullptr) { @@ -176,14 +231,11 @@ bool JitCodeCache::findRuntimeStubInfo(const void* address, const void** start, } } } - _stubs_lock.unlockShared(); return found; } #endif long long JitCodeCache::runtimeStubsMemoryUsage() { - _stubs_lock.lockShared(); - long long usage = _runtime_stubs.memoryUsage(); - _stubs_lock.unlockShared(); - return usage; + SharedLockGuard guard(&_stubs_lock); + return _runtime_stubs.memoryUsage(); } diff --git a/ddprof-lib/src/main/cpp/hotspot/jitCodeCache.h b/ddprof-lib/src/main/cpp/hotspot/jitCodeCache.h index c2e02b221b..fb9dd2ea9b 100644 --- a/ddprof-lib/src/main/cpp/hotspot/jitCodeCache.h +++ b/ddprof-lib/src/main/cpp/hotspot/jitCodeCache.h @@ -9,6 +9,7 @@ #include #include "codeCache.h" +#include "mutex.h" #include "spinLock.h" #include "hotspot/vmStructs.h" @@ -19,10 +20,11 @@ // hotspot/stubUnwindInfo.h). struct StubUnwindInfo; -// Registry entry: one per registered runtime stub, appended under the exclusive -// _stubs_lock, never removed or reordered (CodeCache never removes blobs -// either). _info is fixed at registration time and the pointed-to object is -// never freed, so readers see a stable snapshot under the shared lock. +// Registry entry: one per registered runtime stub, inserted in start-address +// order under the exclusive _stubs_lock and never removed (CodeCache never +// removes blobs either). _info is fixed at registration time and the +// pointed-to object is never freed, so readers see a stable snapshot under the +// shared lock. The array holding the entries is replaced when it grows. struct StubUnwindEntry { const void* _start; const StubUnwindInfo* _info; @@ -32,7 +34,14 @@ struct StubUnwindEntry { // The class tracks JIT-compiled code and ranges class JitCodeCache { private: + friend class JitCodeCacheTestAccessor; + + // Readers (signal handlers) take it shared and give up while a writer + // holds it; writers take it exclusively only to publish what they + // prepared outside it. static SpinLock _stubs_lock; + // Orders DynamicCodeGenerated() writers; never taken in a signal handler. + static Mutex _stubs_write_lock; static CodeCache _runtime_stubs; static std::atomic _call_stub_begin; static std::atomic _call_stub_end; @@ -58,24 +67,34 @@ class JitCodeCache { address < stub_end; } - static CodeBlob* findRuntimeStub(const void *address); + // Finds the runtime stub containing address and copies its start and name + // out under the shared _stubs_lock (the names are never freed). Returns + // false when address is not inside any stub, or without searching while a + // registration holds the lock exclusively (counted as + // STUB_LOOKUP_LOCK_UNAVAILABLE). Safe to call from a signal handler. On a + // false result the stack walk unwinds the frame without the stub start + // and, on aarch64, with the legacy heuristics instead of the precomputed + // StubUnwindInfo. + static bool findRuntimeStub(const void *address, const void **start, + const char **name); #ifdef __aarch64__ // Finds the runtime stub containing address and returns a consistent view // of its start/name plus the precomputed StubUnwindInfo (NULL when the stub // was not classified -- including excluded names such as call_stub -- or // when address is not inside any stub; returns false in that case). - // Takes _stubs_lock shared exactly like findRuntimeStub(), which the - // calling signal-handler paths already do, so no new lock acquisitions are - // introduced. Copies everything out before releasing the lock; the info - // objects themselves are never freed (same lifetime as the stub names). + // Takes _stubs_lock shared like findRuntimeStub() and also returns false + // while a registration holds the lock exclusively. + // Copies everything out before releasing the lock; the info objects + // themselves are never freed (same lifetime as the stub names). static bool findRuntimeStubInfo(const void* address, const void** start, const char** name, const StubUnwindInfo** info); #endif // Heap usage of the runtime-stubs code cache, via CodeCache::memoryUsage(). // Read under the shared stubs lock because DynamicCodeGenerated() mutates - // _runtime_stubs (add()/expand()) concurrently. + // _runtime_stubs (commitAdd()) concurrently. Not for signal handlers: it + // waits without bound. static long long runtimeStubsMemoryUsage(); static bool isJitCode(const void* pc) { return CodeHeap::contains(pc); diff --git a/ddprof-lib/src/main/cpp/refCountGuard.cpp b/ddprof-lib/src/main/cpp/refCountGuard.cpp index a86fd26869..379d85d09f 100644 --- a/ddprof-lib/src/main/cpp/refCountGuard.cpp +++ b/ddprof-lib/src/main/cpp/refCountGuard.cpp @@ -18,11 +18,6 @@ RefCountSlot RefCountGuard::refcount_slots[RefCountGuard::MAX_THREADS]; int RefCountGuard::slot_owners[RefCountGuard::MAX_THREADS]; -// One-time warning latch: emit at most one Log::warn per process when -// reentrant nesting exceeds NESTED_DEPTH and the scanner can no longer see -// every nested resource on the slot. -static std::atomic s_nested_overflow_warned{false}; - int RefCountGuard::getThreadRefCountSlot() { ProfiledThread* thrd = ProfiledThread::current(); int tid = thrd != nullptr ? thrd->tid() : OS::threadId(); @@ -88,12 +83,13 @@ RefCountGuard::RefCountGuard(void* resource) : _active(true), _is_reentrant(fals __atomic_store_n(&refcount_slots[_my_slot].nested[idx], resource, __ATOMIC_SEQ_CST); } else { // Reentrant nesting deeper than NESTED_DEPTH; this guard's resource - // is invisible to the scanner. Latch a single warning per process. - bool expected = false; - if (s_nested_overflow_warned.compare_exchange_strong(expected, true, std::memory_order_relaxed)) { - Log::warn("RefCountGuard reentrancy depth %u exceeds NESTED_DEPTH=%d; scanner may miss nested resources", - static_cast(prev_count) + 1, NESTED_DEPTH); - } + // is invisible to the scanner. Counted, not logged: the guard + // constructor runs in signal handlers, where stdio is not + // async-signal-safe (PROF-16314). The counter is written to + // each JFR chunk as a datadog.ProfilerCounter event and exposed via + // JavaProfiler.getDebugCounters(); it advances once per overflowing + // guard. + Counters::increment(REFCOUNT_NESTED_OVERFLOW, 1); } } else { // Non-reentrant (count was 0): store pointer first so the scanner skips diff --git a/ddprof-lib/src/main/cpp/refCountGuard.h b/ddprof-lib/src/main/cpp/refCountGuard.h index 1b1aef8501..13a69081f7 100644 --- a/ddprof-lib/src/main/cpp/refCountGuard.h +++ b/ddprof-lib/src/main/cpp/refCountGuard.h @@ -34,8 +34,10 @@ * it no matter how many nested guards start or end during the scan. (When a * nested guard moved the outer resource out of active_ptr and back, a scan * could read each location while the resource was in the other one.) - * nested is sized to NESTED_DEPTH; deeper nesting emits a one-time warning and - * that guard's resource is invisible to the scanner (rare: it requires + * nested is sized to NESTED_DEPTH; deeper nesting increments the + * REFCOUNT_NESTED_OVERFLOW counter (async-signal-safe; the guard constructor + * runs in signal handlers, so it must not log) and that guard's resource is + * invisible to the scanner (rare: it requires * NESTED_DEPTH+1 nested signal deliveries on the same thread). The unrecorded * guard is the innermost one, the handler running at that moment; recording * it by overwriting an entry would move a protected resource again. diff --git a/ddprof-lib/src/main/cpp/spinLock.h b/ddprof-lib/src/main/cpp/spinLock.h index d35e40e45b..a62117431c 100644 --- a/ddprof-lib/src/main/cpp/spinLock.h +++ b/ddprof-lib/src/main/cpp/spinLock.h @@ -1,5 +1,6 @@ /* * Copyright 2017 Andrei Pangin + * Copyright 2026, Datadog, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,6 +58,10 @@ class alignas(DEFAULT_CACHE_LINE_SIZE) SpinLock { // Bounds the number of CAS-retry iterations under reader contention; // does NOT bound wall-clock latency (CAS stall time is hardware-dependent). static constexpr int DEFAULT_SHARED_SPIN_BUDGET = 256; + // Passed as max_spins to OptionalSharedLockGuard: fail only while an + // exclusive lock is held, and keep retrying under reader contention + // (tryLockShared() without a budget). + static constexpr int NO_SPIN_BUDGET = -1; bool tryLockShared() { // Spins while no exclusive lock is held and the CAS to acquire a shared @@ -123,15 +128,19 @@ class SharedLockGuard { // Acquires a shared lock with a bounded CAS-retry budget. Returns without // acquiring (ownsLock() == false) when an exclusive lock is observed or -// the spin budget is exhausted under reader contention. Safe to use in -// signal-handler paths. +// the spin budget is exhausted under reader contention. With +// SpinLock::NO_SPIN_BUDGET it fails only when an exclusive lock is observed. +// Safe to use in signal-handler paths. class OptionalSharedLockGuard { SpinLock* _lock; public: explicit OptionalSharedLockGuard( SpinLock* lock, int max_spins = SpinLock::DEFAULT_SHARED_SPIN_BUDGET) : _lock(lock) { - if (!_lock->tryLockShared(max_spins)) { + bool acquired = max_spins == SpinLock::NO_SPIN_BUDGET + ? _lock->tryLockShared() + : _lock->tryLockShared(max_spins); + if (!acquired) { _lock = nullptr; } } diff --git a/ddprof-lib/src/test/cpp/codeCache_ut.cpp b/ddprof-lib/src/test/cpp/codeCache_ut.cpp index 1183473316..dd6806be53 100644 --- a/ddprof-lib/src/test/cpp/codeCache_ut.cpp +++ b/ddprof-lib/src/test/cpp/codeCache_ut.cpp @@ -6,6 +6,8 @@ #include #include "codeCache.h" +#include + // 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. @@ -99,3 +101,75 @@ TEST(CodeCacheTest, ArrayMemoryUsageSumsLibraries) { delete a; delete b; } + +// The two-phase add used under JitCodeCache's stubs lock: prepareAdd() hands +// over a grown array only when the cache is full, commitAdd() returns the +// replaced array exactly then, and the result matches add()'s documented +// accounting (blob array at the grown capacity plus each name allocation). +TEST(CodeCacheTest, TwoPhaseAddGrowsOnlyWhenFull) { + CodeCache cc("lib"); + long long names = 0; + char name[32]; + const int ADDS = INITIAL_CODE_CACHE_CAPACITY + 1; + for (int i = 0; i < ADDS; i++) { + snprintf(name, sizeof(name), "sym_%d", i); + CodeCache::PreparedAdd prepared; + cc.prepareAdd(name, prepared); + bool full = i == INITIAL_CODE_CACHE_CAPACITY; + EXPECT_EQ(full, prepared.grown != nullptr) << i; + CodeBlob *retired = cc.commitAdd(reinterpret_cast(0x1000 + i * 16), + 16, prepared); + EXPECT_EQ(full, retired != nullptr) << i; + delete[] retired; + names += (long long)NativeFunc::allocSize(name); + } + EXPECT_EQ((long long)INITIAL_CODE_CACHE_CAPACITY * 2 * sizeof(CodeBlob) + + (long long)NativeFunc::allocSize("lib") + names, + cc.memoryUsage()); + for (int i = 0; i < ADDS; i++) { + snprintf(name, sizeof(name), "sym_%d", i); + EXPECT_EQ(reinterpret_cast(0x1000 + i * 16), cc.findSymbol(name)) << i; + } +} + +// prepareAdd() keeps add()'s replacement of non-printable name characters. +TEST(CodeCacheTest, TwoPhaseAddReplacesNonPrintableName) { + CodeCache cc("lib"); + CodeCache::PreparedAdd prepared; + cc.prepareAdd("a\x01" "b", prepared); + delete[] cc.commitAdd(reinterpret_cast(0x1000), 16, prepared); + EXPECT_EQ(reinterpret_cast(0x1000), cc.findSymbol("a?b")); +} + +// A PreparedAdd that is never committed frees its allocations and leaves the +// cache unchanged, including when it holds a grown blob array. LeakSanitizer +// (ASan builds) reports the allocations if the destructor does not free them. +TEST(CodeCacheTest, UncommittedPrepareFreesAndLeavesCacheUnchanged) { + CodeCache cc("lib"); + char name[32]; + for (int i = 0; i < INITIAL_CODE_CACHE_CAPACITY; i++) { + snprintf(name, sizeof(name), "sym_%d", i); + cc.add(reinterpret_cast(0x1000 + i * 16), 16, name); + } + long long usage = cc.memoryUsage(); + { + CodeCache::PreparedAdd prepared; + cc.prepareAdd("never_committed", prepared); + EXPECT_NE(nullptr, prepared.name); + EXPECT_NE(nullptr, prepared.grown); + } + EXPECT_EQ(usage, cc.memoryUsage()); + EXPECT_EQ(nullptr, cc.findSymbol("never_committed")); +} + +// commitAdd() takes ownership: the PreparedAdd no longer refers to the name or +// the grown array, so its destructor cannot free storage the cache uses. +TEST(CodeCacheTest, CommitAddTakesOwnership) { + CodeCache cc("lib"); + CodeCache::PreparedAdd prepared; + cc.prepareAdd("owned", prepared); + delete[] cc.commitAdd(reinterpret_cast(0x1000), 16, prepared); + EXPECT_EQ(nullptr, prepared.name); + EXPECT_EQ(nullptr, prepared.grown); + EXPECT_EQ(reinterpret_cast(0x1000), cc.findSymbol("owned")); +} diff --git a/ddprof-lib/src/test/cpp/jitCodeCache_ut.cpp b/ddprof-lib/src/test/cpp/jitCodeCache_ut.cpp new file mode 100644 index 0000000000..079cc51031 --- /dev/null +++ b/ddprof-lib/src/test/cpp/jitCodeCache_ut.cpp @@ -0,0 +1,227 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "counters.h" +#include "hotspot/jitCodeCache.h" +#ifdef __aarch64__ +#include "hotspot/stubUnwindInfo.h" +#endif + +class JitCodeCacheTestAccessor { +public: + static SpinLock& stubsLock() { return JitCodeCache::_stubs_lock; } +}; + +namespace { + +// JitCodeCache state is process-wide and stubs are never removed, so every +// test registers into its own disjoint slice of this buffer. The buffer is +// static so registered addresses stay valid; on aarch64 the registration +// decodes it, so it is filled with NOPs. Stub names avoid "Interpreter" and +// "call_stub", which change other global state. +const int CODE_WORDS = 16384; +alignas(64) uint32_t g_code[CODE_WORDS]; + +const uint32_t* codeAt(int word) { + static bool filled = [] { + for (int i = 0; i < CODE_WORDS; i++) { + g_code[i] = 0xd503201f; // aarch64 NOP + } + return true; + }(); + (void)filled; + return &g_code[word]; +} + +void registerStub(const void* start, int length, const char* name) { + JitCodeCache::DynamicCodeGenerated(nullptr, name, start, length); +} + +// Runs body on a helper thread and reports whether it finished within the +// timeout. Joins in every case, so a body that waits on a lock held by the +// caller must be released by the caller before the join (see release). +template +bool finishesWithin(std::chrono::milliseconds timeout, Body body, Release release) { + std::atomic done{false}; + std::thread t([&] { + body(); + done.store(true, std::memory_order_release); + }); + auto deadline = std::chrono::steady_clock::now() + timeout; + while (!done.load(std::memory_order_acquire) && + std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + bool finished = done.load(std::memory_order_acquire); + release(); + t.join(); + return finished; +} + +} // namespace + +// A signal-handler lookup must not wait for a registration that holds +// _stubs_lock exclusively: the lookup returns "not found" at once, counts the +// miss, and finds the stub again after the unlock. +TEST(JitCodeCacheTest, FindRuntimeStubDoesNotWaitForExclusiveHolder) { + const void* code = codeAt(0); + registerStub(code, 64, "jcc_test_busy"); + + const void* start = nullptr; + const char* name = nullptr; + ASSERT_TRUE(JitCodeCache::findRuntimeStub(code, &start, &name)); + EXPECT_EQ(code, start); + EXPECT_STREQ("jcc_test_busy", name); + + SpinLock& lock = JitCodeCacheTestAccessor::stubsLock(); + long long before = Counters::getCounter(STUB_LOOKUP_LOCK_UNAVAILABLE); + bool found = true; + lock.lock(); + bool finished = finishesWithin( + std::chrono::seconds(5), + [&] { found = JitCodeCache::findRuntimeStub(code, &start, &name); }, + [&] { lock.unlock(); }); + ASSERT_TRUE(finished) << "findRuntimeStub waited for the exclusive holder"; + EXPECT_FALSE(found); + EXPECT_EQ(1, Counters::getCounter(STUB_LOOKUP_LOCK_UNAVAILABLE) - before); + + EXPECT_TRUE(JitCodeCache::findRuntimeStub(code, &start, &name)); +} + +#ifdef __aarch64__ +TEST(JitCodeCacheTest, FindRuntimeStubInfoDoesNotWaitForExclusiveHolder) { + const void* code = codeAt(64); + registerStub(code, 64, "jcc_test_busy_info"); + + const void* start = nullptr; + const char* name = nullptr; + const StubUnwindInfo* info = nullptr; + ASSERT_TRUE(JitCodeCache::findRuntimeStubInfo(code, &start, &name, &info)); + EXPECT_EQ(code, start); + EXPECT_STREQ("jcc_test_busy_info", name); + + SpinLock& lock = JitCodeCacheTestAccessor::stubsLock(); + long long before = Counters::getCounter(STUB_LOOKUP_LOCK_UNAVAILABLE); + bool found = true; + lock.lock(); + bool finished = finishesWithin( + std::chrono::seconds(5), + [&] { found = JitCodeCache::findRuntimeStubInfo(code, &start, &name, &info); }, + [&] { lock.unlock(); }); + ASSERT_TRUE(finished) << "findRuntimeStubInfo waited for the exclusive holder"; + EXPECT_FALSE(found); + EXPECT_EQ(nullptr, info); + EXPECT_EQ(1, Counters::getCounter(STUB_LOOKUP_LOCK_UNAVAILABLE) - before); + + EXPECT_TRUE(JitCodeCache::findRuntimeStubInfo(code, &start, &name, &info)); +} + +// A second registration of the same start keeps the first unwind info. +TEST(JitCodeCacheTest, DuplicateRegistrationKeepsFirstInfo) { + const void* code = codeAt(128); + registerStub(code, 64, "jcc_test_dup"); + const void* start = nullptr; + const char* name = nullptr; + const StubUnwindInfo* first = nullptr; + ASSERT_TRUE(JitCodeCache::findRuntimeStubInfo(code, &start, &name, &first)); + ASSERT_NE(nullptr, first); + + registerStub(code, 64, "jcc_test_dup_again"); + const StubUnwindInfo* second = nullptr; + ASSERT_TRUE(JitCodeCache::findRuntimeStubInfo(code, &start, &name, &second)); + EXPECT_EQ(first, second); +} +#endif + +// More registrations than the initial blob capacity (and, on aarch64, the +// initial unwind-registry capacity) force both arrays to be replaced; every +// stub must stay findable with its own start, name and unwind info. +TEST(JitCodeCacheTest, RegistrationsAcrossGrowthStayFindable) { + const int STUBS = INITIAL_CODE_CACHE_CAPACITY + 100; + const int WORDS_PER_STUB = 2; + const int BASE = 256; + ASSERT_LE(BASE + STUBS * WORDS_PER_STUB, CODE_WORDS); + char name[32]; + for (int i = 0; i < STUBS; i++) { + snprintf(name, sizeof(name), "jcc_grow_%d", i); + registerStub(codeAt(BASE + i * WORDS_PER_STUB), WORDS_PER_STUB * 4, name); + } + for (int i = 0; i < STUBS; i++) { + const void* code = codeAt(BASE + i * WORDS_PER_STUB); + snprintf(name, sizeof(name), "jcc_grow_%d", i); + const void* start = nullptr; + const char* found_name = nullptr; + ASSERT_TRUE(JitCodeCache::findRuntimeStub(code, &start, &found_name)) << i; + EXPECT_EQ(code, start) << i; + EXPECT_STREQ(name, found_name) << i; +#ifdef __aarch64__ + const StubUnwindInfo* info = nullptr; + ASSERT_TRUE(JitCodeCache::findRuntimeStubInfo(code, &start, &found_name, &info)) << i; + EXPECT_NE(nullptr, info) << i; +#endif + } +} + +// Lookups racing registrations that replace the blob array. Readers copy the +// stub fields out under the lock, so none can read a replaced array; under +// ASan a read of a freed array fails the test. Readers pause between lookups: +// SpinLock::lock() only succeeds when no reader holds the lock, so readers +// that only yield can delay the writers for seconds under ASan. +TEST(JitCodeCacheTest, ConcurrentRegistrationAndLookup) { + const int WRITERS = 2; + const int STUBS_PER_WRITER = 600; + const int WORDS_PER_STUB = 2; + const int BASE = 4096; + ASSERT_LE(BASE + WRITERS * STUBS_PER_WRITER * WORDS_PER_STUB, CODE_WORDS); + + std::atomic writers_left{WRITERS}; + std::vector threads; + for (int w = 0; w < WRITERS; w++) { + threads.emplace_back([&, w] { + char name[32]; + for (int i = 0; i < STUBS_PER_WRITER; i++) { + int slot = w * STUBS_PER_WRITER + i; + snprintf(name, sizeof(name), "jcc_conc_%d", slot); + registerStub(codeAt(BASE + slot * WORDS_PER_STUB), WORDS_PER_STUB * 4, name); + } + writers_left.fetch_sub(1); + }); + } + for (int r = 0; r < 4; r++) { + threads.emplace_back([&, r] { + int slot = r; + while (writers_left.load() > 0) { + const void* code = codeAt(BASE + slot * WORDS_PER_STUB); + const void* start = nullptr; + const char* name = nullptr; + if (JitCodeCache::findRuntimeStub(code, &start, &name)) { + EXPECT_EQ(code, start); + EXPECT_EQ(0, strncmp(name, "jcc_conc_", 9)); + } + slot = (slot + 7) % (WRITERS * STUBS_PER_WRITER); + std::this_thread::sleep_for(std::chrono::microseconds(50)); + } + }); + } + for (auto& t : threads) { + t.join(); + } + for (int slot = 0; slot < WRITERS * STUBS_PER_WRITER; slot++) { + const void* start = nullptr; + const char* name = nullptr; + EXPECT_TRUE(JitCodeCache::findRuntimeStub(codeAt(BASE + slot * WORDS_PER_STUB), + &start, &name)) << slot; + } +} diff --git a/ddprof-lib/src/test/cpp/spinLock_ut.cpp b/ddprof-lib/src/test/cpp/spinLock_ut.cpp index 5b7ebceeb7..d9cb6a436c 100644 --- a/ddprof-lib/src/test/cpp/spinLock_ut.cpp +++ b/ddprof-lib/src/test/cpp/spinLock_ut.cpp @@ -19,6 +19,7 @@ #include "../../main/cpp/gtest_crash_handler.h" #include #include +#include static constexpr char SPINLOCK_TEST_NAME[] = "SpinLockTest"; @@ -114,3 +115,51 @@ TEST_F(SpinLockTest, TryLockShared_Free_ReturnsTrue) { EXPECT_TRUE(lock.tryLockShared()); lock.unlockShared(); } + +// NO_SPIN_BUDGET: an exclusive holder still makes the guard fail at once. +TEST_F(SpinLockTest, OptionalGuard_NoSpinBudget_ExclusiveHeld_NotOwned) { + lock.lock(); + { + OptionalSharedLockGuard g(&lock, SpinLock::NO_SPIN_BUDGET); + EXPECT_FALSE(g.ownsLock()); + } + lock.unlock(); +} + +// NO_SPIN_BUDGET: on a free lock the guard acquires and releases on destruction. +TEST_F(SpinLockTest, OptionalGuard_NoSpinBudget_Free_OwnedAndReleased) { + { + OptionalSharedLockGuard g(&lock, SpinLock::NO_SPIN_BUDGET); + EXPECT_TRUE(g.ownsLock()); + EXPECT_FALSE(lock.tryLock()); + } + EXPECT_TRUE(lock.tryLock()); + lock.unlock(); +} + +// NO_SPIN_BUDGET: reader contention alone never makes the guard fail. Readers +// on other threads race the CAS continuously; every acquisition must succeed. +TEST_F(SpinLockTest, OptionalGuard_NoSpinBudget_ReaderContention_AlwaysOwned) { + std::atomic stop{false}; + std::vector contenders; + for (int t = 0; t < 4; t++) { + contenders.emplace_back([&] { + while (!stop.load(std::memory_order_relaxed)) { + lock.lockShared(); + lock.unlockShared(); + } + }); + } + int failures = 0; + for (int i = 0; i < 100000; ++i) { + OptionalSharedLockGuard g(&lock, SpinLock::NO_SPIN_BUDGET); + if (!g.ownsLock()) { + failures++; + } + } + stop.store(true, std::memory_order_relaxed); + for (auto& t : contenders) { + t.join(); + } + EXPECT_EQ(0, failures); +} diff --git a/doc/architecture/JvmSupportLayer.md b/doc/architecture/JvmSupportLayer.md index 09c8cb47ca..8a0ecccac3 100644 --- a/doc/architecture/JvmSupportLayer.md +++ b/doc/architecture/JvmSupportLayer.md @@ -235,7 +235,7 @@ checked on a Zing JVM. Zing is also currently disabled in the CI JDK matrix ## Known boundary exceptions -In these shared files, outside the abstraction files, `grep '#include "(hotspot|j9|zing)/'` +In these shared files, outside the abstraction files, `grep -E '#include ["<](hotspot|j9|zing)/'` finds direct includes of implementation headers: | File | Includes | diff --git a/doc/architecture/RefCountGuard.md b/doc/architecture/RefCountGuard.md index 16c8862f8e..02eab4a8de 100644 --- a/doc/architecture/RefCountGuard.md +++ b/doc/architecture/RefCountGuard.md @@ -98,7 +98,7 @@ sequenceDiagram alt "prev_count - 1 fits in NESTED_DEPTH" Inner->>Slot: "store nested[prev_count - 1] := resource (seq_cst)" else "depth exceeds NESTED_DEPTH" - Inner->>Slot: "Log warn once per process; this resource invisible to scanner" + Inner->>Slot: "increment REFCOUNT_NESTED_OVERFLOW counter; this resource invisible to scanner" end ``` @@ -107,15 +107,25 @@ resource. After construction the slot contains: - `count == prev_count + 1` - `active_ptr == ` the root guard's resource - `nested[0..min(prev_count, NESTED_DEPTH)-1]` holding the nested guards' - resources; a guard nested deeper than that is not recorded and triggers the - one-time overflow warning latched by `s_nested_overflow_warned`. That - unrecorded guard is the innermost one - the handler running at that moment - + resources; a guard nested deeper than that is not recorded and increments + the `REFCOUNT_NESTED_OVERFLOW` counter (`refcount_nested_overflow`) once per + overflowing guard. It is a counter rather than a log line because the + constructor runs in signal handlers, where logging is not + async-signal-safe. That unrecorded guard is the innermost one - the handler + running at that moment - whereas the earlier displacing protocol lost track of a suspended outer one. Either way a drain can then free a resource the thread uses once it continues, and it takes `NESTED_DEPTH + 1` nested signal deliveries on one thread. Recording the innermost guard by overwriting an entry would move a protected resource again, which is what this protocol avoids. +The overflow counter is written with the other profiler counters as a +`datadog.ProfilerCounter` event in every JFR chunk +(`Recording::writeCounters()`), and is readable from Java through +`JavaProfiler.getDebugCounters()` under `refcount_nested_overflow`. +`Profiler::resetRecordingState()` zeroes it (`Counters::reset()`), so it counts +overflows per recording. + The `nested[]` store is `SEQ_CST`. The caller re-checks its resource right after constructing the guard (`CallTraceStorage::put()` reloads `_active_storage`, `StringDictionary` reloads `_accepting`), while a drainer @@ -231,13 +241,14 @@ that drain, with no waiting. | "Every resource on a reentrant chain is visible to the scanner up to NESTED_DEPTH" | `nested[prev_count-1]` write in ctor; conditional clear in dtor | | "No deadlock between drain and signal handler" | All scans are bounded (100 spins, then up to 5000 × 100 µs sleeps); a timeout is reported to the caller, and `waitForRefCountToClear()` also counts, logs and (DEBUG, non-gtest) aborts | | "Slot can be reclaimed after drain returns" | `slot_owners[i] = 0` is written only by the non-reentrant teardown path — destructor or move-assignment overwriting an active guard — and only after `count` has been decremented to 0 | -| "Nesting beyond NESTED_DEPTH is observable" | One-time `Log::warn` latched via `s_nested_overflow_warned` | +| "Nesting beyond NESTED_DEPTH is observable" | `REFCOUNT_NESTED_OVERFLOW` counter incremented by each overflowing guard; reported as a `datadog.ProfilerCounter` JFR event and via `JavaProfiler.getDebugCounters()` | --- ## Files - `ddprof-lib/src/main/cpp/refCountGuard.h` — class declaration, `RefCountSlot` layout, `NESTED_DEPTH`. -- `ddprof-lib/src/main/cpp/refCountGuard.cpp` — implementation, drain loop, overflow warning. +- `ddprof-lib/src/main/cpp/refCountGuard.cpp` — implementation, drain loop, nested-overflow counter. +- `ddprof-lib/src/main/cpp/counters.h` — `REFCOUNT_NESTED_OVERFLOW` counter definition. - `ddprof-lib/src/main/cpp/stringDictionary.h` — primary user; see [StringDictionary](StringDictionary.md). - `ddprof-lib/src/main/cpp/callTraceStorage.cpp`, `callTraceHashTable.cpp` — secondary user; see [CallTraceStorage](CallTraceStorage.md). diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 560a216c2e..a8e3264f91 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -14,16 +14,16 @@ slf4j = "2.0.20" # Profiler runtime jmc = "8.3.1" jol = "0.17" -lz4 = "1.12.0" +lz4 = "1.12.1" snappy = "1.1.10.12" -zstd = "1.5.7-23" +zstd = "1.5.7-24" # JFR parsing for test verification (replaces JMC's FlightRecordingLoader, which can hang # forever if one of its internal parallel chunk-parsing workers dies from OOM) jafar = "0.29.0" # Code analysis -asm = "9.11" +asm = "9.11.1" # Datadog tracer (compileOnly for the chaos harness — runtime is provided by dd-java-agent). # Dynamic version: always resolve to the latest released dd-trace-api so the chaos harness diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 77127250e2..8c15ccee00 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-9.8.1-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-9.8.2-bin.zip networkTimeout=30000 retries=5 retryBackOffMs=2000 diff --git a/utils/README.md b/utils/README.md index bb506349b4..e3e0e01e99 100644 --- a/utils/README.md +++ b/utils/README.md @@ -182,6 +182,36 @@ Examples: Single-value commands run one container test configuration immediately. When any dimension expands to multiple cells, the script prints a compact status table first; interactive terminals ask for confirmation, while non-interactive runs require `--run` to execute. Matrix execution prints the status table again after all cells finish and writes summaries to `build/reports/container-matrix/summary.md` and `build/reports/container-matrix/summary.json`. Matrix gtest runs require a short `--gtest-task` name so the task follows each cell's configuration. Cells not run because of `--fail-fast` are reported as cancelled separately from unsupported cells that are skipped. +### `check-signal-reachability.py` + +Static async-signal-safety check. Builds a call graph from the relocations of the debug object files (`objdump -dr`) and reports every function reachable from a signal handler or malloc hook that calls an unsafe symbol (heap allocation, locks, stdio, dynamic-loader calls, dynamic TLS, logging). The entry points, unsafe symbols and known findings are data lists at the top of the script. ELF objects only (Linux x64/aarch64); needs GNU `objdump` and `c++filt`. + +**Usage:** +```bash +./utils/check-signal-reachability.py --obj-dir ddprof-lib/build/obj/main/debug [--allowlist ] +./utils/check-signal-reachability.py --self-test # planted violations in utils/tests/signal-reachability-fixture.cpp +./gradlew :ddprof-lib:checkSignalReachability # opt-in; SKIPPED when the debug objects are not ELF +``` + +Exit codes: `0` no findings outside the allowlist (or self-test passed), `1` new findings (one `: -> (via )` line each), `2` usage/environment error. `--allowlist` replaces the built-in known findings; one entry per line: `[|...] -> # `. + +### Source quality checks + +Standalone, stdlib-only checks over the git-tracked files. Each prints one `[:]: ` line per violation and exits `0` clean, `1` violations, `2` environment error; `--root` selects the checkout (default: the one containing the script). + +| Script | Checks | +|---|---| +| `check-jvm-boundary-includes.py` | Shared code in `ddprof-lib/src/main/cpp` does not `#include "hotspot/"`, `"j9/"` or `"zing/"` headers (quoted or angle-bracket form), except the boundary files and the known exceptions (mirrors `doc/architecture/JvmSupportLayer.md#known-boundary-exceptions`). | +| `check-doc-index.py` | Every `doc/**/*.md` is linked from `doc/README.md` and has a PascalCase file name. | +| `check-sigsetjmp-shape.py` | Every `sigsetjmp()` has a `JmpCtxScope` in effect, `savemask` 1, and a landing pad that starts with `SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP();` then `.restore();` (the `guards.h` pattern). The glibc cancellation buffer in `libraryPatcher_linux.cpp` is the one exception. | +| `check-tracked-artifacts.py` | No tracked path under `docs/sphinx/` or `.sphinx/`, and no `*.scratch*` path. | + +**Usage:** +```bash +./utils/check-doc-index.py # any one check +./gradlew :ddprof-lib:qualityChecks # signal-reachability check (+ --self-test on Linux) plus all four; run by CI (ci.yml source-invariant-checks job) +``` + ### `patch-dd-java-agent.sh` Patches a `dd-java-agent.jar` with a locally-built ddprof library for quick local testing without a full dd-trace-java rebuild. diff --git a/utils/check-doc-index.py b/utils/check-doc-index.py new file mode 100755 index 0000000000..de6b9d9143 --- /dev/null +++ b/utils/check-doc-index.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 +# Copyright 2026, Datadog, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Check that every tracked doc/**/*.md is linked from doc/README.md and named in PascalCase. + +doc/README.md itself is the only exception to both rules. + +Exit codes: 0 clean, 1 violations, 2 environment error. +""" + +import argparse +import os +import posixpath +import re +import subprocess +import sys + +INDEX = "doc/README.md" +_LINK = re.compile(r"\]\((]+>?)\)") +_PASCAL = re.compile(r"^[A-Z][A-Za-z0-9]*\.md$") + + +def main(argv): + ap = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) + ap.add_argument("--root", default=os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + help="repository root (default: the checkout containing this script)") + root = ap.parse_args(argv).root + + try: + docs = subprocess.run(["git", "-C", root, "ls-files", "--", "doc/*.md", "doc/**/*.md"], + capture_output=True, text=True, check=True).stdout.splitlines() + with open(os.path.join(root, INDEX), encoding="utf-8") as f: + index = f.read() + except (OSError, subprocess.CalledProcessError) as e: + print("error: %s" % e, file=sys.stderr) + return 2 + + linked = set() + for target in _LINK.findall(index): + target = target.strip("<>").split("#", 1)[0] + if target and "://" not in target: + linked.add(posixpath.normpath(posixpath.join("doc", target))) + + errors = [] + tracked = set(docs) + for doc in sorted(tracked - {INDEX}): + if doc not in linked: + errors.append("%s: not linked from %s" % (doc, INDEX)) + if not _PASCAL.match(posixpath.basename(doc)): + errors.append("%s: file name is not PascalCase (e.g. BuildSystemGuide.md)" % doc) + # Reverse direction: every relative index link must resolve to a tracked + # file, so renames that leave a dangling link fail here too. + for target in sorted(linked): + if target.startswith("doc/") and target not in tracked: + errors.append("%s: %s links to a file that is not tracked" % (INDEX, target)) + for e in errors: + print(e) + print("doc index: %s" % ("%d problem(s)" % len(errors) if errors else "OK (%d docs)" % (len(docs) - 1))) + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/utils/check-jvm-boundary-includes.py b/utils/check-jvm-boundary-includes.py new file mode 100755 index 0000000000..e60e00f9a3 --- /dev/null +++ b/utils/check-jvm-boundary-includes.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python3 +# Copyright 2026, Datadog, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Check that shared native code reaches JVM implementations only through the boundary. + +Shared files under ddprof-lib/src/main/cpp must not #include "hotspot/...", +"j9/..." or "zing/..." headers (quoted or angle-bracket form). Allowed: the +implementation directories themselves, the boundary files, and the known +exceptions listed below. + +Exit codes: 0 clean, 1 violations or stale exceptions, 2 environment error. +""" + +import argparse +import os +import re +import subprocess +import sys + +SRC = "ddprof-lib/src/main/cpp" +IMPLEMENTATION_DIRS = ("hotspot/", "j9/", "zing/") + +# The abstraction files that dispatch on the detected VM. +BOUNDARY_FILES = { + "jvmSupport.h", "jvmSupport.inline.h", "jvmSupport.cpp", + "jvmThread.h", "jvmThread.cpp", + "vmEntry.h", "vmEntry.cpp", + "javaApi.cpp", +} + +# MUST match doc/architecture/JvmSupportLayer.md#known-boundary-exceptions; +# update both together. New code should not add to this list. +KNOWN_EXCEPTIONS = { + "profiler.cpp": {"hotspot/vmStructs.inline.h", "hotspot/hotspotSupport.h", + "j9/j9Support.h", "j9/j9WallClock.h"}, + "flightRecorder.cpp": {"hotspot/vmStructs.h"}, + "livenessTracker.cpp": {"hotspot/vmStructs.h", "hotspot/vmStructs.inline.h"}, + "sanityCheck.cpp": {"hotspot/vmStructs.h", "hotspot/vmStructs.inline.h"}, + "threadState.inline.h": {"hotspot/vmStructs.h"}, +} + +_INCLUDE = re.compile(r'^\s*#\s*include\s+(?:"((?:hotspot|j9|zing)/[^"]+)"|<((?:hotspot|j9|zing)/[^>]+)>)') + + +def tracked_files(root, path): + try: + out = subprocess.run(["git", "-C", root, "ls-files", "--", path], + capture_output=True, text=True, check=True).stdout + except (OSError, subprocess.CalledProcessError) as e: + print("error: git ls-files failed: %s" % e, file=sys.stderr) + return None + return out.splitlines() + + +def main(argv): + ap = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) + ap.add_argument("--root", default=os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + help="repository root (default: the checkout containing this script)") + root = ap.parse_args(argv).root + + files = tracked_files(root, SRC) + if files is None: + return 2 + errors, seen = [], set() + for path in files: + rel = path[len(SRC) + 1:] + if rel.startswith(IMPLEMENTATION_DIRS) or rel in BOUNDARY_FILES: + continue + with open(os.path.join(root, path), encoding="utf-8", errors="replace") as f: + for n, line in enumerate(f, 1): + m = _INCLUDE.match(line) + if not m: + continue + header = m.group(1) or m.group(2) + if header in KNOWN_EXCEPTIONS.get(rel, ()): + seen.add((rel, header)) + else: + errors.append('%s:%d: shared code includes "%s"; call JVMSupport/JVMThread/VM instead ' + "(see doc/architecture/JvmSupportLayer.md)" % (path, n, header)) + for rel, headers in sorted(KNOWN_EXCEPTIONS.items()): + for header in sorted(headers - {h for r, h in seen if r == rel}): + errors.append('%s/%s: known exception "%s" is no longer included; remove it from ' + "KNOWN_EXCEPTIONS and doc/architecture/JvmSupportLayer.md" % (SRC, rel, header)) + for e in errors: + print(e) + print("jvm boundary includes: %s" % ("%d problem(s)" % len(errors) if errors else "OK")) + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/utils/check-signal-reachability.py b/utils/check-signal-reachability.py new file mode 100755 index 0000000000..855f2c0a6c --- /dev/null +++ b/utils/check-signal-reachability.py @@ -0,0 +1,470 @@ +#!/usr/bin/env python3 +# Copyright 2026, Datadog, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Static async-signal-safety reachability check for the profiler's native code. + +Builds a call graph from the relocations of ELF object files (`objdump -dr`), +walks it from the signal-handler / malloc-hook entry points in ROOTS and reports +every function that calls into an UNSAFE_SINKS symbol (heap allocation, locks, +stdio, dynamic-loader calls, dynamic TLS, logging, ...). + +The graph only contains direct calls, so the objects must be built with +-ffunction-sections (every function gets its own section, so every call carries +a relocation) - the debug configuration (build/obj/main/debug) does that. +Indirect calls (function pointers, virtual dispatch) are not followed. + +Exit codes: + 0 no findings outside the allowlist (or --self-test passed) + 1 one or more new findings, one line each: + : -> (via ) + 2 usage / environment error (missing or non-ELF objects, no GNU objdump, + self-test fixture does not compile, malformed allowlist) +""" + +import argparse +import shlex +import bisect +import collections +import os +import re +import shutil +import subprocess +import sys +import tempfile +from concurrent.futures import ThreadPoolExecutor + +# --------------------------------------------------------------------------- +# Entry points: mangled-symbol regexes (full match) of the functions that run +# in async-signal context or as a malloc interposer. Extend as needed. +# +# The NativeSocketSampler::{send,recv,write,read}_hook interposers are +# deliberately NOT listed: they run on the calling Java thread, not in a +# signal handler, and are allowed to allocate and lock +# (see ddprof-lib/src/main/cpp/nativeSocketSampler.h). +# --------------------------------------------------------------------------- +ROOTS = [ + # Signal handlers + r"_ZN6CTimer13signalHandlerE\w+", + r"_ZN11CTimerJvmti13signalHandlerE\w+", + r"_ZN6ITimer13signalHandlerE\w+", + r"_ZN11ITimerJvmti13signalHandlerE\w+", + r"_ZN10PerfEvents13signalHandlerE\w+", + r"_ZN14WallClockASGCT19sharedSignalHandlerE\w+", + r"_ZN14WallClockASGCT13signalHandlerE\w+", + r"_ZN14WallClockJvmti19sharedSignalHandlerE\w+", + r"_ZN14WallClockJvmti13signalHandlerE\w+", + r"_ZN8Profiler11segvHandlerE\w+", + r"_ZN8Profiler10busHandlerE\w+", + r"_ZL13wakeupHandleri", + # malloc interposers (must not re-enter the allocator) + r"malloc_hook", + r"calloc_hook", + r"realloc_hook", + r"posix_memalign_hook", + r"aligned_alloc_hook", + r"_ZL18nested_malloc_hookm", + r"_ZL25nested_aligned_alloc_hookmm", +] + +# --------------------------------------------------------------------------- +# Unsafe sinks: mangled-symbol regexes (full match) that must not be reached +# from a root. "TLS_ACCESS:" is the synthetic node for a thread_local +# access; the library is -fPIC, so every access is a TLS descriptor call that +# can allocate the DTV slot lazily. +# --------------------------------------------------------------------------- +UNSAFE_SINKS = [ + r"TLS_ACCESS:.*", + r"__tls_get_addr", + # heap + r"malloc", r"calloc", r"realloc", r"free", r"aligned_alloc", r"posix_memalign", r"strdup", + r"__libc_malloc", r"__libc_calloc", r"__libc_free", r"__libc_realloc", + r"_Znwm", r"_Znam", r"_ZnwmRKSt9nothrow_t", r"_ZnamRKSt9nothrow_t", + # over-aligned operator new/delete (C++17 -faligned-new) + r"_ZnwmSt11align_val_t", r"_ZnamSt11align_val_t", + r"_ZnwmSt11align_val_tRKSt9nothrow_t", r"_ZnamSt11align_val_tRKSt9nothrow_t", + r"_ZdlPvSt11align_val_t", r"_ZdaPvSt11align_val_t", + r"_ZdlPvmSt11align_val_t", r"_ZdaPvmSt11align_val_t", + r"_ZdlPv", r"_ZdaPv", r"_ZdlPvm", r"_ZdaPvm", + # locks and one-time initialization + r"pthread_mutex_lock", r"__cxa_guard_acquire", r"__cxa_atexit", + r"_ZN8SpinLock4lockEv", r"_ZN8SpinLock10lockSharedEv", r"_ZN5Mutex4lockEv", + r"_ZN15SharedLockGuardC\dEP8SpinLock", r"_ZN18ExclusiveLockGuardC\dEP8SpinLock", + r"_ZN13MutexLockerC\d\w*", + # stdio + r"vsnprintf", r"snprintf", r"fprintf", r"printf", r"fflush", r"fopen", + # dynamic loader / unwinder + r"dlsym", r"dladdr", r"dlopen", r"dl_iterate_phdr", r"backtrace", + # profiler internals that allocate or lock + r"_ZN3Log\w+", + r"_ZN14ProfiledThread17initCurrentThreadEv", # initCurrentThreadSignalSafe() is fine +] + +# Functions whose callees are not followed. The singleton accessors guard a +# function-local static; the guard is initialized at startup, before any +# handler or hook is installed. +# BARRIERS note: stopping at these singletons assumes their first (lazy) call +# happens before any handler or hook is installed. The walk cannot verify that +# ordering from relocations, so a first-call-in-signal-context would pass +# unseen -- keep the singletons initialized during profiler startup. +BARRIERS = [ + r"_ZN8Counters8instanceEv", + r"_ZN9Libraries8instanceEv", + r"_ZN8Profiler8instanceEv", +] + +# --------------------------------------------------------------------------- +# Known findings: (caller symbol alternatives, unsafe symbol, tracking issue). +# Symbols are demangled names without parameter lists, exactly as printed in +# the report. --allowlist FILE replaces this list; the file holds one entry per +# line in the same shape: +# [|...] -> # [note] +# --------------------------------------------------------------------------- +ALLOWLIST = [] + +SELF_TEST_FIXTURE = os.path.join(os.path.dirname(os.path.abspath(__file__)), + "tests", "signal-reachability-fixture.cpp") + +# --------------------------------------------------------------------------- + +_SYM_LINE = re.compile(r"^([0-9a-f]+) (.{7}) (\S+)\s+[0-9a-f]+ (?:\.hidden |\.protected |\.internal )?(\S+)$") +# Direct calls, plus PC-relative address materialization (clang -O0 on aarch64 +# calls hidden functions via adrp+add+blr; on x86 PC32 covers lea/call alike). +# Taking a function's address is conservatively treated as calling it. +_CALL_RELOC = re.compile(r"^\s+([0-9a-f]+):\s+(R_AARCH64_(?:CALL26|JUMP26|ADR_PREL_PG_HI21|ADR_GOT_PAGE)|R_X86_64_(?:PLT32|PC32|GOTPCREL|GOTPCRELX|REX_GOTPCRELX))\s+(\S+?)([+-]0x[0-9a-f]+)?$") +_TLS_RELOC = re.compile(r"^\s+([0-9a-f]+):\s+(R_AARCH64_TLS\w*|R_X86_64_(?:TLSGD|TLSLD|GOTTPOFF|TPOFF32|GOTPC32_TLSDESC|TLSDESC_CALL))\s+(\S+?)([+-]0x[0-9a-f]+)?$") +_SECTION_HEADER = re.compile(r"^Disassembly of section (\S+):$") +_TLS_SECTION = re.compile(r"^\.(?:tbss|tdata)\.") + + +class EnvError(Exception): + """Usage or environment problem: exit code 2.""" + + +def _full(patterns): + return re.compile("^(?:" + "|".join(patterns) + ")$") + + +ROOT_RE, SINK_RE, BARRIER_RE = _full(ROOTS), _full(UNSAFE_SINKS), _full(BARRIERS) + + +def _symbol(node): + """Node id -> symbol name. Local symbols are qualified as 'sym@object'.""" + return node.rsplit("@", 1)[0] if "@" in node and not node.startswith("TLS_ACCESS:") else node + + +def _run(cmd, **kw): + try: + return subprocess.run(cmd, capture_output=True, text=True, **kw) + except OSError as e: + raise EnvError("cannot run %s: %s" % (cmd[0], e)) + + +def check_tools(): + out = _run(["objdump", "--version"]) + if out.returncode != 0 or "GNU objdump" not in out.stdout: + raise EnvError("GNU objdump is required (found: %s)" % + (out.stdout.splitlines()[0] if out.stdout else "none")) + if not shutil.which("c++filt"): + raise EnvError("c++filt is required") + + +def list_objects(obj_dir): + if not os.path.isdir(obj_dir): + raise EnvError("object directory not found: %s" % obj_dir) + objs = sorted(os.path.join(obj_dir, f) for f in os.listdir(obj_dir) if f.endswith(".o")) + if not objs: + raise EnvError("no .o files in %s" % obj_dir) + for o in objs: + with open(o, "rb") as f: + if f.read(4) != b"\x7fELF": + raise EnvError("%s is not an ELF object; only Linux (ELF) objects are supported" % o) + return objs + + +def _parse_object(obj): + """Returns (defined: {node: obj}, edges: [(caller_node, callee_node)]).""" + name = os.path.basename(obj) + syms = _run(["objdump", "-t", obj]) + dis = _run(["objdump", "-dr", "--no-show-raw-insn", obj]) + if syms.returncode != 0 or dis.returncode != 0: + raise EnvError("objdump failed on %s: %s" % (obj, (syms.stderr or dis.stderr).strip())) + + local = set() + by_section = collections.defaultdict(list) # section -> [(offset, node)] + for line in syms.stdout.splitlines(): + m = _SYM_LINE.match(line) + if not m: + continue + flags, section, sym = m.group(2), m.group(3), m.group(4) + if flags[0] == "l": + local.add(sym) + if flags[6] == "F" and section.startswith(".text"): + node = sym + "@" + name if flags[0] == "l" else sym + by_section[section].append((int(m.group(1), 16), node)) + for funcs in by_section.values(): + funcs.sort() + + def resolve(target, addend, x86): + if target.startswith(".text"): + # Call to a local function through its section symbol. + funcs = by_section.get(target) + if not funcs: + return None + off = addend + (4 if x86 else 0) + best = funcs[0][1] + for start, node in funcs: + if start <= off: + best = node + return best + if target.startswith("."): + return None # data section (.bss, .rodata, ...) + return target + "@" + name if target in local else target + + defined = {n: name for funcs in by_section.values() for _, n in funcs} + edges = [] + funcs, starts = [], [] + for line in dis.stdout.splitlines(): + h = _SECTION_HEADER.match(line) + if h: + funcs = by_section.get(h.group(1), []) + starts = [s for s, _ in funcs] + continue + if not funcs: + continue + m = _CALL_RELOC.match(line) + tls = None if m else _TLS_RELOC.match(line) + if not (m or tls): + continue + g = m or tls + off = int(g.group(1), 16) + # Owning function(s): the last start <= off, plus aliases at the same + # address (e.g. C1/C2 constructor and D1/D2 destructor variants). + i = bisect.bisect_right(starts, off) - 1 + if i < 0: + continue + addend = int(g.group(4), 16) if g.group(4) else 0 + if tls: + callee = "TLS_ACCESS:" + _TLS_SECTION.sub("", g.group(3)) + else: + callee = resolve(g.group(3), addend, g.group(2).startswith("R_X86_64")) + if callee is None: + continue + j = i + while j >= 0 and starts[j] == starts[i]: + edges.append((funcs[j][1], callee)) + j -= 1 + return defined, edges + + +def build_graph(objs): + defined, graph = {}, collections.defaultdict(set) + with ThreadPoolExecutor(max_workers=min(16, (os.cpu_count() or 4))) as pool: + for d, edges in pool.map(_parse_object, objs): + for n, o in d.items(): + defined.setdefault(n, o) + for caller, callee in edges: + graph[caller].add(callee) + return defined, graph + + +def demangle_all(nodes): + nodes = sorted(nodes) + raw = [_symbol(n).replace("TLS_ACCESS:", "") for n in nodes] + out = _run(["c++filt"], input="\n".join(raw) + "\n") + lines = out.stdout.splitlines() + if out.returncode != 0 or len(lines) != len(nodes): + raise EnvError("c++filt failed") + return {n: ("TLS_ACCESS:" if n.startswith("TLS_ACCESS:") else "") + _strip_params(d) + for n, d in zip(nodes, lines)} + + +def _strip_params(name): + """'void ns::f(int) const' -> 'void ns::f'""" + s = name.rstrip() + for q in (" const", " volatile", " &&", " &"): + if s.endswith(q): + s = s[: -len(q)] + if not s.endswith(")"): + return s + depth = 0 + for i in range(len(s) - 1, -1, -1): + if s[i] == ")": + depth += 1 + elif s[i] == "(": + depth -= 1 + if depth == 0: + return s[:i] if i else s + return s + + +_LIBRARY_FRAME = re.compile(r"^(?:.*\s)?(?:std|__gnu_cxx|__cxxabiv1)::") + + +def find_violations(defined, graph): + """BFS from ROOTS; returns [(object, caller, sink, path)] with demangled names.""" + roots = sorted(n for n in set(defined) | set(graph) if ROOT_RE.match(_symbol(n))) + parent = {r: None for r in roots} + queue = collections.deque(roots) + while queue: + u = queue.popleft() + s = _symbol(u) + if SINK_RE.match(s) or BARRIER_RE.match(s): + continue + for v in sorted(graph.get(u, ())): + if v not in parent: + parent[v] = u + queue.append(v) + + names = demangle_all(parent) + found, seen = [], set() + for u in parent: # BFS order: shortest path first + s = _symbol(u) + if SINK_RE.match(s) or BARRIER_RE.match(s): + continue + for v in sorted(graph.get(u, ())): + if not SINK_RE.match(_symbol(v)): + continue + path, x = [], u + while x is not None: + path.append(x) + x = parent[x] + path.reverse() + # Attribute calls made inside the C++ standard library to the + # profiler function that called into it. + caller = u + for p in reversed(path): + if not _LIBRARY_FRAME.match(names[p]): + caller = p + break + key = (names[caller], names[v]) + if key in seen: + continue + seen.add(key) + found.append((defined.get(caller, "?"), names[caller], names[v], + " -> ".join(names[p] for p in path))) + return roots, found + + +_ALLOW_LINE = re.compile(r"^(?P.+?)\s+->\s+(?P.+?)\s+#\s*(?P[A-Z][A-Z0-9]+-\d+)\b.*$") + + +def load_allowlist(path): + entries = [] + with open(path, encoding="utf-8") as f: + for n, line in enumerate(f, 1): + if not line.strip() or line.lstrip().startswith("#"): + continue + m = _ALLOW_LINE.match(line.strip()) + if not m: + raise EnvError("%s:%d: expected '[|...] -> # '" % (path, n)) + entries.append((tuple(c.strip() for c in m.group("callers").split("|")), + m.group("sink").strip(), m.group("key"))) + return entries + + +def check(obj_dir, allowlist): + objs = list_objects(obj_dir) + check_tools() + defined, graph = build_graph(objs) + roots, found = find_violations(defined, graph) + if not roots: + raise EnvError("none of the ROOTS symbols is defined in %s" % obj_dir) + + new, used = [], set() + for obj, caller, sink, path in found: + hit = next((i for i, (callers, s, _) in enumerate(allowlist) if caller in callers and sink == s), None) + if hit is None: + new.append("%s:%s -> %s (via %s)" % (obj, caller, sink, path)) + else: + used.add(hit) + print("allowlisted [%s]: %s:%s -> %s" % (allowlist[hit][2], obj, caller, sink)) + stale = [(callers, sink, key) for i, (callers, sink, key) in enumerate(allowlist) if i not in used] + print("%d objects, %d roots, %d findings (%d allowlisted, %d new)" + % (len(objs), len(roots), len(found), len(found) - len(new), len(new))) + for line in new: + print(line) + return new, stale + + +def self_test(): + """Compile the planted-violation fixture and check its EXPECT* annotations.""" + with open(SELF_TEST_FIXTURE) as f: + src = f.read() + + def expectations(tag): + return set(re.findall(r"^// %s: (.+?) -> (.+?)\s*$" % tag, src, re.M)) + + expected, allowed, forbidden = (expectations(t) for t in ("EXPECT", "EXPECT-ALLOWLISTED", "EXPECT-NOT")) + if not expected or not allowed or not forbidden: + raise EnvError("fixture %s lacks EXPECT / EXPECT-ALLOWLISTED / EXPECT-NOT lines" % SELF_TEST_FIXTURE) + # CXX may be a command with arguments ("ccache g++"); split it the way a shell would. + cxx = shlex.split(os.environ.get("CXX", "c++")) + with tempfile.TemporaryDirectory() as tmp: + obj = os.path.join(tmp, "fixture.o") + # Same codegen-relevant flags as the debug configuration. + cc = _run(cxx + ["-O0", "-g", "-fPIC", "-fvisibility=hidden", "-ffunction-sections", + "-fdata-sections", "-std=c++17", "-c", SELF_TEST_FIXTURE, "-o", obj]) + if cc.returncode != 0: + raise EnvError("fixture compile failed:\n" + cc.stderr) + allowlist = [((c,), s, "SELFTEST-1") for c, s in allowed] + new, _ = check(tmp, allowlist) + _, found = find_violations(*build_graph([obj])) + got = {(c, s) for _, c, s, _ in found} + reported = {(c, s) for c, s in got if (c, s) not in allowed} + errors = (["MISSED %s -> %s" % e for e in sorted(expected - reported)] + + ["NOT FOUND (allowlisted) %s -> %s" % e for e in sorted(allowed - got)] + + ["FALSE POSITIVE %s -> %s" % e for e in sorted(forbidden & got)]) + if len(new) != len(reported): + errors.append("allowlist did not suppress exactly the EXPECT-ALLOWLISTED findings") + for e in errors: + print("self-test: " + e) + print("self-test %s: %d/%d planted violations reported, %d/%d allowlisted suppressed, %d/%d negative controls clean" + % ("FAILED" if errors else "PASSED", len(expected & reported), len(expected), + len(allowed & got), len(allowed), len(forbidden - got), len(forbidden))) + return 1 if errors else 0 + + +def main(argv): + ap = argparse.ArgumentParser(description=__doc__.split("\n\n")[0], + formatter_class=argparse.RawDescriptionHelpFormatter, + epilog=__doc__.split("\n\n", 1)[1]) + ap.add_argument("--obj-dir", help="directory with the ELF .o files to check (e.g. ddprof-lib/build/obj/main/debug)") + ap.add_argument("--allowlist", help="known-findings file replacing the built-in ALLOWLIST") + ap.add_argument("--self-test", action="store_true", help="check the planted-violation fixture instead") + args = ap.parse_args(argv) + try: + if args.self_test: + return self_test() + if not args.obj_dir: + ap.print_usage(sys.stderr) + print("error: --obj-dir is required", file=sys.stderr) + return 2 + allowlist = load_allowlist(args.allowlist) if args.allowlist else ALLOWLIST + new, stale = check(args.obj_dir, allowlist) + if stale: + # An allowlist entry suppresses findings by name; one that no longer + # matches can silently mask a reappeared violation, so it fails the + # run like the sibling checkers do. + for callers, sink, key in stale: + print("error: stale allowlist entry [%s] %s -> %s; remove it" + % (key, "|".join(callers), sink), file=sys.stderr) + return 1 + return 1 if new else 0 + except (EnvError, OSError, ValueError) as e: + print("error: %s" % e, file=sys.stderr) + return 2 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/utils/check-sigsetjmp-shape.py b/utils/check-sigsetjmp-shape.py new file mode 100755 index 0000000000..94e1b1c7ca --- /dev/null +++ b/utils/check-sigsetjmp-shape.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +# Copyright 2026, Datadog, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Check that every crash-protection sigsetjmp() follows the JmpCtxScope pattern (guards.h). + +For each sigsetjmp( / __sigsetjmp( call in tracked files under ddprof-lib/src/main/cpp: + 1. a JmpCtxScope is declared before it, in the same or an enclosing block; + 2. savemask is the literal 1; + 3. the landing pad `if (sigsetjmp(...) != 0) {` starts with + SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); followed by .restore(); + +Exit codes: 0 clean, 1 violations or stale exceptions, 2 environment error. +""" + +import argparse +import os +import re +import subprocess +import sys + +SRC = "ddprof-lib/src/main/cpp" + +# (file, first argument) of calls that are not crash protection. +# libraryPatcher_linux.cpp: glibc pthread cancellation buffer. glibc's unwinder +# siglongjmps here on pthread_exit/cancel (not checkFault() from segvHandler), +# so there is no signal-handler state to unwind and no signal mask to restore. +KNOWN_EXCEPTIONS = { + ("libraryPatcher_linux.cpp", "(struct __jmp_buf_tag*)(void*)cancel_buf.__cancel_jmp_buf"), +} + +_CALL = re.compile(r"\b(?:__)?sigsetjmp\s*\(") +_SCOPE = re.compile(r"\bJmpCtxScope\s+(\w+)\s*[({]") +_PAD = re.compile(r"\s*!=\s*0\s*\)\s*\{\s*SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP\s*\(\s*\)\s*;\s*(\w+)\s*\.\s*restore\s*\(\s*\)\s*;") +_COMMENT_OR_STRING = re.compile(r'//[^\n]*|/\*.*?\*/|"(?:\\.|[^"\\\n])*"|\'(?:\\.|[^\'\\\n])*\'', re.S) + + +def blank(m): + # Keep offsets and line numbers; keep string quotes so code shape is intact. + t = m.group(0) + if t[0] in "\"'": + return t[0] + " " * (len(t) - 2) + t[-1] + return re.sub(r"[^\n]", " ", t) + + +def split_args(code, open_paren): + """Top-level arguments of the call whose '(' is at open_paren; returns (args, end).""" + depth, start, args = 0, open_paren + 1, [] + for i in range(open_paren, len(code)): + c = code[i] + if c in "([{": + depth += 1 + elif c in ")]}": + depth -= 1 + if depth == 0: + args.append(code[start:i].strip()) + return args, i + 1 + elif c == "," and depth == 1: + args.append(code[start:i].strip()) + start = i + 1 + return None, len(code) + + +def scope_in_effect(code, pos): + """Name of the nearest JmpCtxScope declared before pos in the same or an enclosing block.""" + depth = 0 + visible = [] # code ranges at the call's block nesting + end = pos + for i in range(pos - 1, -1, -1): + c = code[i] + if c == "}": + if depth == 0: + visible.append((i + 1, end)) + depth += 1 + elif c == "{": + if depth == 0: + visible.append((i + 1, end)) + end = i + else: + depth -= 1 + if depth == 0: + end = i + 1 + if depth == 0: + visible.append((0, end)) + for a, b in visible: + names = _SCOPE.findall(code[a:b]) + if names: + return names[-1] + return None + + +def check_file(path, text): + code = _COMMENT_OR_STRING.sub(blank, text) + rel = path[len(SRC) + 1:] + errors, exceptions = [], set() + for m in _CALL.finditer(code): + line = code.count("\n", 0, m.start()) + 1 + where = "%s:%d" % (path, line) + args, end = split_args(code, m.end() - 1) + if args is None or len(args) != 2: + errors.append("%s: cannot parse the sigsetjmp() arguments" % where) + continue + first = " ".join(args[0].split()) + key = (rel, first) + if key in KNOWN_EXCEPTIONS: + exceptions.add(key) + continue + scope = scope_in_effect(code, m.start()) + if scope is None: + errors.append("%s: no JmpCtxScope declared before sigsetjmp(%s, ...); see guards.h" % (where, first)) + if args[1] != "1": + errors.append("%s: savemask is '%s', must be 1 (SIGSEGV stays blocked after siglongjmp otherwise)" + % (where, args[1])) + pad = _PAD.match(code, end) + if pad is None: + errors.append("%s: landing pad must be `if (sigsetjmp(...) != 0) {` starting with " + "SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); then .restore();" % where) + elif scope is not None and pad.group(1) != scope: + errors.append("%s: landing pad restores '%s' but the JmpCtxScope in effect is '%s'" + % (where, pad.group(1), scope)) + return errors, exceptions + + +def main(argv): + ap = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) + ap.add_argument("--root", default=os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + help="repository root (default: the checkout containing this script)") + ap.add_argument("files", nargs="*", help="check these files instead of the tracked sources") + args = ap.parse_args(argv) + root = args.root + files = args.files + if not files: + try: + files = subprocess.run(["git", "-C", root, "ls-files", "--", SRC], + capture_output=True, text=True, check=True).stdout.splitlines() + except (OSError, subprocess.CalledProcessError) as e: + print("error: git ls-files failed: %s" % e, file=sys.stderr) + return 2 + + errors, seen, calls = [], set(), 0 + for path in files: + try: + with open(os.path.join(root, path), encoding="utf-8", errors="replace") as f: + text = f.read() + except OSError as e: + print("error: %s" % e, file=sys.stderr) + return 2 + calls += len(_CALL.findall(_COMMENT_OR_STRING.sub(blank, text))) + e, s = check_file(path, text) + errors += e + seen |= s + if not args.files: + for rel, arg in sorted(KNOWN_EXCEPTIONS - seen): + errors.append("%s/%s: known exception __sigsetjmp(%s, ...) no longer found; remove it from " + "KNOWN_EXCEPTIONS" % (SRC, rel, arg)) + for e in errors: + print(e) + print("sigsetjmp shape: %s" % ("%d problem(s)" % len(errors) if errors + else "OK (%d calls, %d known exception(s))" % (calls, len(seen)))) + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/utils/check-tracked-artifacts.py b/utils/check-tracked-artifacts.py new file mode 100755 index 0000000000..41a1488c4d --- /dev/null +++ b/utils/check-tracked-artifacts.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +# Copyright 2026, Datadog, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Check that no session/tool artifacts are tracked by git. + +Rejected paths: anything under docs/sphinx/ or a .sphinx/ directory, and any +path component matching *.scratch* (e.g. .scratch/, notes.scratch.md). + +Exit codes: 0 clean, 1 violations, 2 environment error. +""" + +import argparse +import fnmatch +import os +import subprocess +import sys + + +def reason(path): + parts = path.split("/") + for i, part in enumerate(parts): + if part == "docs" and i + 1 < len(parts) and parts[i + 1] == "sphinx": + return "docs/sphinx/ holds review-session artifacts" + if part == ".sphinx" and i < len(parts) - 1: + return ".sphinx/ holds review-session state" + if fnmatch.fnmatchcase(part, "*.scratch*"): + return "*.scratch* paths are local scratch files" + return None + + +def main(argv): + ap = argparse.ArgumentParser(description=__doc__.split("\n\n")[0]) + ap.add_argument("--root", default=os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + help="repository root (default: the checkout containing this script)") + root = ap.parse_args(argv).root + try: + files = subprocess.run(["git", "-C", root, "ls-files"], + capture_output=True, text=True, check=True).stdout.splitlines() + except (OSError, subprocess.CalledProcessError) as e: + print("error: git ls-files failed: %s" % e, file=sys.stderr) + return 2 + + errors = ["%s: tracked artifact (%s); git rm it" % (f, r) for f in files for r in [reason(f)] if r] + for e in errors: + print(e) + print("tracked artifacts: %s" % ("%d problem(s)" % len(errors) if errors else "OK")) + return 1 if errors else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/utils/tests/signal-reachability-fixture.cpp b/utils/tests/signal-reachability-fixture.cpp new file mode 100644 index 0000000000..f9182c7bec --- /dev/null +++ b/utils/tests/signal-reachability-fixture.cpp @@ -0,0 +1,151 @@ +/* + * Copyright 2026, Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Planted violations for `utils/check-signal-reachability.py --self-test`. +// Compiled on its own into a temporary directory; never linked or run. +// Class and function names mirror the checker's ROOTS / UNSAFE_SINKS. +// +// EXPECT: must be reported as a new finding +// EXPECT-ALLOWLISTED: must be found and suppressed by the self-test allowlist +// EXPECT-NOT: must not be reported +// Entries are " -> " exactly as the checker prints them. + +struct siginfo_t; + +extern "C" { +void *malloc(unsigned long size); +int snprintf(char *buf, unsigned long size, const char *fmt, ...); +} + +class Log { +public: + static void warn(const char *fmt, ...); +}; + +class SpinLock { +public: + void lock(); +}; + +class ProfiledThread { +public: + static ProfiledThread *initCurrentThread(); + static ProfiledThread *initCurrentThreadSignalSafe(); +}; + +// Direct libc call from a signal handler. +// EXPECT: CTimer::signalHandler -> malloc +class CTimer { +public: + static void signalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void CTimer::signalHandler(int, siginfo_t *, void *) { (void)malloc(16); } + +// Transitive call through static helpers: the calls go through section +// symbols (.text._ZL...), not function symbols. +// EXPECT: formatSlowPath -> snprintf +static void formatSlowPath(char *buf) { snprintf(buf, 8, "%d", 1); } +static void formatValue(char *buf) { formatSlowPath(buf); } +class ITimer { +public: + static void signalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void ITimer::signalHandler(int, siginfo_t *, void *) { + char buf[8]; + formatValue(buf); +} + +// Lock taken in a constructor: the call site references the complete-object +// constructor (C1), whose body is emitted as an alias of the base-object +// constructor (C2). +// EXPECT: LockGuard::LockGuard -> SpinLock::lock +class LockGuard { +public: + explicit LockGuard(SpinLock *lock); +}; +LockGuard::LockGuard(SpinLock *lock) { lock->lock(); } +class PerfEvents { +public: + static void signalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void PerfEvents::signalHandler(int, siginfo_t *, void *) { + static SpinLock lock; + LockGuard guard(&lock); +} + +// thread_local access (dynamic TLS) from a signal handler. +// EXPECT: Profiler::segvHandler -> TLS_ACCESS:t_depth +// A singleton accessor is a barrier: its callees are not followed. +// EXPECT-NOT: Profiler::instance -> malloc +thread_local int t_depth; +class Profiler { +public: + static Profiler *instance(); + static bool segvHandler(int signo, siginfo_t *info, void *ucontext); +}; +Profiler *Profiler::instance() { return static_cast(malloc(1)); } +bool Profiler::segvHandler(int, siginfo_t *, void *) { + Profiler::instance(); + return ++t_depth > 1; +} + +// Heap allocation from a malloc interposer. +// EXPECT: malloc_hook -> operator new[] +extern "C" __attribute__((visibility("default"))) void *malloc_hook(unsigned long size) { + return new char[size]; +} + +// Allocation inside a standard-library template is attributed to the +// profiler function calling into it. +// EXPECT: CTimerJvmti::signalHandler -> operator new +namespace std { +template struct fixture_box { + static T *make() { return new T(); } +}; +} // namespace std +class CTimerJvmti { +public: + static void signalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void CTimerJvmti::signalHandler(int, siginfo_t *, void *) { + (void)std::fixture_box::make(); +} + +// Only the non-signal-safe ProfiledThread initializer is a sink. +// EXPECT: WallClockJvmti::sharedSignalHandler -> ProfiledThread::initCurrentThread +// EXPECT-NOT: WallClockASGCT::sharedSignalHandler -> ProfiledThread::initCurrentThreadSignalSafe +class WallClockJvmti { +public: + static void sharedSignalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void WallClockJvmti::sharedSignalHandler(int, siginfo_t *, void *) { + ProfiledThread::initCurrentThread(); +} +class WallClockASGCT { +public: + static void sharedSignalHandler(int signo, siginfo_t *info, void *ucontext); +}; +void WallClockASGCT::sharedSignalHandler(int, siginfo_t *, void *) { + ProfiledThread::initCurrentThreadSignalSafe(); +} + +// Static (local-symbol) root; finding suppressed by the self-test allowlist. +// EXPECT-ALLOWLISTED: wakeupHandler -> Log::warn +__attribute__((used)) static void wakeupHandler(int) { Log::warn("wakeup"); } + +// Not reachable from any root. +// EXPECT-NOT: unrelatedFunction -> malloc +void unrelatedFunction() { (void)malloc(1); } diff --git a/utils/tests/test_quality_check_scripts.sh b/utils/tests/test_quality_check_scripts.sh new file mode 100755 index 0000000000..900f52cd94 --- /dev/null +++ b/utils/tests/test_quality_check_scripts.sh @@ -0,0 +1,368 @@ +#!/usr/bin/env bash + +# Copyright 2026, Datadog, Inc + +# Hermetic tests for the utils/check-*.py quality-check scripts run by +# `./gradlew :ddprof-lib:qualityChecks`. +# Run with: bash utils/tests/test_quality_check_scripts.sh +# +# Linux-only cases (they compile the signal-reachability fixture and need GNU +# objdump) print "SKIP: (not Linux)" elsewhere; a skip counts as a pass. + +set -euo pipefail + +SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +ROOT=$(cd "$SCRIPT_DIR/../.." && pwd) +SIGSETJMP="$ROOT/utils/check-sigsetjmp-shape.py" +BOUNDARY="$ROOT/utils/check-jvm-boundary-includes.py" +REACHABILITY="$ROOT/utils/check-signal-reachability.py" +FIXTURE="$ROOT/utils/tests/signal-reachability-fixture.cpp" +SRC=ddprof-lib/src/main/cpp +TESTS=0 +FAILED=0 + +# Importing the scripts as modules must not leave __pycache__ in the tree. +export PYTHONDONTWRITEBYTECODE=1 + +TEMP_DIR=$(mktemp -d) +trap 'rm -rf "$TEMP_DIR"' EXIT + +pass() { + TESTS=$((TESTS + 1)) +} + +fail() { + TESTS=$((TESTS + 1)) + FAILED=$((FAILED + 1)) + echo "FAIL: $*" >&2 +} + +skip() { + TESTS=$((TESTS + 1)) + echo "SKIP: $*" +} + +on_linux() { + [ "$(uname -s)" = "Linux" ] +} + +# expect [] +expect() { + local name=$1 want_rc=$2 rc=$3 out=$4 want_out=${5:-} + if [ "$rc" -ne "$want_rc" ]; then + fail "$name: exit $rc, expected $want_rc; output:"$'\n'"$out" + elif [ -n "$want_out" ] && [[ "$out" != *"$want_out"* ]]; then + fail "$name: output lacks '$want_out'; output:"$'\n'"$out" + else + pass + fi +} + +# --- T1: check-sigsetjmp-shape.py scope recognition --------------------------- + +SJ_ROOT="$TEMP_DIR/sigsetjmp" +mkdir -p "$SJ_ROOT/$SRC" +PAD_A='if (sigsetjmp(ctx, 1) != 0) { SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); jmp_scope.restore(); return; }' + +# sigsetjmp_case ; prints output, returns the checker's exit code. +sigsetjmp_case() { + local name=$1 body=$2 + printf 'void f(ProfiledThread* pt, bool x) {\n%s\n}\n' "$body" > "$SJ_ROOT/$SRC/$name.cpp" + python3 "$SIGSETJMP" --root "$SJ_ROOT" "$SRC/$name.cpp" 2>&1 +} + +rc=0; out=$(sigsetjmp_case brace_init " JmpCtxScope jmp_scope{pt}; + $PAD_A") || rc=$? +expect "T1a brace-init guard" 0 "$rc" "$out" "OK (1 calls" + +rc=0; out=$(sigsetjmp_case paren_init " JmpCtxScope jmp_scope(pt); + $PAD_A") || rc=$? +expect "T1b paren-init guard" 0 "$rc" "$out" "OK (1 calls" + +rc=0; out=$(sigsetjmp_case block_paren " { JmpCtxScope jmp_scope(pt); } + $PAD_A") || rc=$? +expect "T1c paren-init guard in completed block" 1 "$rc" "$out" "no JmpCtxScope declared" + +rc=0; out=$(sigsetjmp_case block_brace " { JmpCtxScope jmp_scope{pt}; } + $PAD_A") || rc=$? +expect "T1d brace-init guard in completed block" 1 "$rc" "$out" "no JmpCtxScope declared" + +rc=0; out=$(sigsetjmp_case enclosing_brace " JmpCtxScope jmp_scope{pt}; + if (x) { + $PAD_A + }") || rc=$? +expect "T1e brace-init guard in enclosing block" 0 "$rc" "$out" "OK (1 calls" + +rc=0; out=$(sigsetjmp_case pad_mismatch " JmpCtxScope a{pt}; + if (sigsetjmp(ctx, 1) != 0) { SIGNAL_HANDLER_UNWIND_AFTER_LONGJMP(); b.restore(); return; }") || rc=$? +expect "T1f brace-init guard, pad name mismatch" 1 "$rc" "$out" \ + "restores 'b' but the JmpCtxScope in effect is 'a'" + +rc=0; out=$(python3 "$SIGSETJMP" 2>&1) || rc=$? +expect "T1g tracked tree" 0 "$rc" "$out" "OK (7 calls, 1 known exception(s))" + +# --- T2: check-jvm-boundary-includes.py include forms -------------------------- + +# boundary_repo [ ...]; prints the repo dir. +boundary_repo() { + local repo="$TEMP_DIR/boundary/$1" + shift + mkdir -p "$repo/$SRC" + git -C "$repo" init -q + while [ $# -gt 0 ]; do + mkdir -p "$(dirname "$repo/$SRC/$1")" + printf '%s\nvoid f() {}\n' "$2" > "$repo/$SRC/$1" + git -C "$repo" add "$SRC/$1" + shift 2 + done + echo "$repo" +} + +# run_boundary +run_boundary() { + python3 - "$BOUNDARY" "$1" "$2" <<'EOF' 2>&1 +import ast, importlib.util, sys +spec = importlib.util.spec_from_file_location("check_jvm_boundary_includes", sys.argv[1]) +mod = importlib.util.module_from_spec(spec) +spec.loader.exec_module(mod) +mod.KNOWN_EXCEPTIONS = ast.literal_eval(sys.argv[3]) +sys.exit(mod.main(["--root", sys.argv[2]])) +EOF +} + +repo=$(boundary_repo angle shared.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2a angle-bracket include" 1 "$rc" "$out" "shared.cpp:1: shared code includes \"hotspot/vmStructs.h\"" + +repo=$(boundary_repo quoted shared.cpp '#include "hotspot/vmStructs.h"') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2b quoted include" 1 "$rc" "$out" "shared.cpp:1: shared code includes \"hotspot/vmStructs.h\"" + +repo=$(boundary_repo j9zing a.cpp '#include ' b.cpp '# include ') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2c j9 angle include" 1 "$rc" "$out" "a.cpp:1: shared code includes \"j9/j9Support.h\"" +expect "T2c zing angle include" 1 "$rc" "$out" "b.cpp:1: shared code includes \"zing/x.h\"" + +repo=$(boundary_repo mixed1 shared.cpp '#include "hotspot/vmStructs.h>') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2d mixed \"...> delimiters not matched" 0 "$rc" "$out" "jvm boundary includes: OK" + +repo=$(boundary_repo mixed2 shared.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{"shared.cpp": {"hotspot/vmStructs.h"}}') || rc=$? +expect "T2f known exception via angle include" 0 "$rc" "$out" "jvm boundary includes: OK" + +repo=$(boundary_repo stale shared.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{"shared.cpp": {"hotspot/vmStructs.h"}}') || rc=$? +expect "T2g stale known exception" 1 "$rc" "$out" "no longer included" + +repo=$(boundary_repo boundary_file jvmSupport.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2h boundary file exempt" 0 "$rc" "$out" "jvm boundary includes: OK" + +repo=$(boundary_repo impl_dir hotspot/x.cpp '#include ') +rc=0; out=$(run_boundary "$repo" '{}') || rc=$? +expect "T2i implementation dir exempt" 0 "$rc" "$out" "jvm boundary includes: OK" + +rc=0; out=$(python3 "$BOUNDARY" 2>&1) || rc=$? +expect "T2j tracked tree" 0 "$rc" "$out" "jvm boundary includes: OK" + +# --- T3: check-signal-reachability.py --self-test (Linux) ---------------------- + +# self_test_copy []; prints stdout, +# stderr goes to $TEMP_DIR/sr-.err, returns the self-test's exit code. +self_test_copy() { + local dir="$TEMP_DIR/sr-$1" + mkdir -p "$dir/tests" + cp "$REACHABILITY" "$dir/" + cp "$FIXTURE" "$dir/tests/" + if [ -n "${2:-}" ]; then + printf '%s\n' "$2" >> "$dir/tests/signal-reachability-fixture.cpp" + fi + python3 "$dir/check-signal-reachability.py" --self-test 2> "$TEMP_DIR/sr-$1.err" +} + +if on_linux; then + rc=0; out=$(self_test_copy healthy) || rc=$? + expect "T3a healthy self-test" 0 "$rc" "$out" "self-test PASSED" + if [[ "$out" == *MISSED* ]]; then fail "T3a healthy self-test reports MISSED:"$'\n'"$out"; else pass; fi + + rc=0; out=$(self_test_copy missed '// EXPECT: CTimer::signalHandler -> fopen') || rc=$? + expect "T3b missed expectation" 1 "$rc" "$out" "MISSED CTimer::signalHandler -> fopen" + + rc=0; out=$(self_test_copy stale '// EXPECT-ALLOWLISTED: CTimer::signalHandler -> dlopen') || rc=$? + expect "T3c unused allowlist entry" 1 "$rc" "$out" "NOT FOUND (allowlisted) CTimer::signalHandler -> dlopen" + if grep -q 'not enough values to unpack' "$TEMP_DIR/sr-stale.err"; then + fail "T3c unused allowlist entry crashed: $(cat "$TEMP_DIR/sr-stale.err")" + else + pass + fi + + rc=0; out=$(self_test_copy false_positive '// EXPECT-NOT: CTimer::signalHandler -> malloc') || rc=$? + expect "T3d false positive" 1 "$rc" "$out" "FALSE POSITIVE CTimer::signalHandler -> malloc" +else + skip "T3a-T3d fixture self-test (not Linux)" +fi + +# --- T3h: self_test() evaluates the real findings (every host) ---------------- +# +# Stubs the compiler run and the object analysis so self_test()'s own logic is +# exercised without an ELF toolchain: findings must come from +# find_violations(*build_graph([obj])) inside the temp dir, and check()'s +# stale-entry list must never be treated as findings. + +run_self_test_harness() { + python3 - "$REACHABILITY" "$1" <<'EOF' 2>&1 +import contextlib, importlib.util, io, os, re, sys, types + +path, case = sys.argv[1], sys.argv[2] +spec = importlib.util.spec_from_file_location("check_signal_reachability", path) +mod = importlib.util.module_from_spec(spec) +spec.loader.exec_module(mod) + +with open(mod.SELF_TEST_FIXTURE) as f: + src = f.read() + +def pairs(tag): + return set(re.findall(r"^// %s: (.+?) -> (.+?)\s*$" % tag, src, re.M)) + +E, A, N = pairs("EXPECT"), pairs("EXPECT-ALLOWLISTED"), pairs("EXPECT-NOT") +FP = ("Profiler::instance", "malloc") +MISS = ("CTimer::signalHandler", "malloc") +if not (E and A and N) or (E & A) or (E & N) or (A & N) or FP not in N or MISS not in E: + print("FAIL: %s: fixture precondition broken (E=%r A=%r N=%r)" % (case, E, A, N)) + sys.exit(1) + +cases = { + # name findings stale new findings + "healthy": (E | A, [], E), + "stale": (E | A, [(("NoSuch::caller",), "malloc", "SELFTEST-1")], E), + "allowlisted_absent": (E, [], E), + "false_positive": (E | A | {FP}, [], E | {FP}), + "missed": ((E - {MISS}) | A, [], E - {MISS}), +} +F, stale, E_new = cases[case] + +DEFINED, GRAPH = object(), object() +state = {"obj": None, "fv_calls": 0, "problems": []} + +def fake_run(cmd, **kw): + state["obj"] = cmd[cmd.index("-o") + 1] + return types.SimpleNamespace(returncode=0, stdout="", stderr="") + +def fake_check(obj_dir, allowlist): + return ["x:%s -> %s (via y)" % p for p in sorted(E_new)], stale + +def fake_build_graph(objs): + if objs != [state["obj"]]: + state["problems"].append("build_graph(%r), expected [%r]" % (objs, state["obj"])) + if not os.path.isdir(os.path.dirname(objs[0])): + state["problems"].append("build_graph called after the temp dir was removed") + return DEFINED, GRAPH + +def fake_find_violations(defined, graph): + if defined is not DEFINED or graph is not GRAPH: + state["problems"].append("find_violations not called with build_graph's result") + state["fv_calls"] += 1 + return {"root"}, [("fixture.o", c, s, "path") for c, s in sorted(F)] + +mod._run, mod.check = fake_run, fake_check +mod.build_graph, mod.find_violations = fake_build_graph, fake_find_violations + +buf = io.StringIO() +try: + with contextlib.redirect_stdout(buf): + rc = mod.self_test() +except Exception as e: + print("FAIL: %s: self_test raised %s: %s" % (case, type(e).__name__, e)) + sys.exit(1) +if state["fv_calls"] != 1: + state["problems"].append("find_violations called %d times, expected 1" % state["fv_calls"]) +print(buf.getvalue(), end="") +for p in state["problems"]: + print("FAIL: %s: %s" % (case, p)) +print("rc=%d" % rc) +sys.exit(1 if state["problems"] else 0) +EOF +} + +# self_test_harness_case [] +self_test_harness_case() { + local name=$1 want=$2 want_out=${3:-} + local rc=0 out + out=$(run_self_test_harness "$name") || rc=$? + if [ "$rc" -ne 0 ]; then + fail "T3h-$name: harness failed:"$'\n'"$out" + elif [[ "$out" != *"rc=$want"* ]]; then + fail "T3h-$name: self_test() did not return $want:"$'\n'"$out" + elif [ -n "$want_out" ] && [[ "$out" != *"$want_out"* ]]; then + fail "T3h-$name: output lacks '$want_out':"$'\n'"$out" + else + pass + fi +} + +self_test_harness_case healthy 0 "self-test PASSED" +self_test_harness_case stale 0 "self-test PASSED" +self_test_harness_case allowlisted_absent 1 "NOT FOUND (allowlisted) wakeupHandler -> Log::warn" +self_test_harness_case false_positive 1 "FALSE POSITIVE Profiler::instance -> malloc" +self_test_harness_case missed 1 "MISSED CTimer::signalHandler -> malloc" + +# --- T4: tree-level script health and doc/build wiring ------------------------ + +rc=0; out=$(python3 "$ROOT/utils/check-doc-index.py" 2>&1) || rc=$? +expect "T4 check-doc-index.py" 0 "$rc" "$out" + +rc=0; out=$(python3 "$ROOT/utils/check-tracked-artifacts.py" 2>&1) || rc=$? +expect "T4 check-tracked-artifacts.py" 0 "$rc" "$out" + +# Built without the literal so this file never matches itself. +pat=s_nested_overflow; pat="${pat}_warned" +if out=$(git -C "$ROOT" grep -n "$pat" -- doc ddprof-lib AGENTS.md README.md utils/README.md); then + fail "T4 removed overflow-warning latch still documented:"$'\n'"$out" +else + pass +fi + +if grep -q 'opt-in; checkSignalReachability plus all four' "$ROOT/utils/README.md"; then + fail "T4 utils/README.md still calls qualityChecks opt-in" +else + pass +fi + +if grep -n 'Opt-in' "$ROOT/ddprof-lib/build.gradle.kts"; then + fail "T4 ddprof-lib/build.gradle.kts still says Opt-in" +else + pass +fi + +GRADLE="$ROOT/ddprof-lib/build.gradle.kts" +n=$(grep -c 'hasElfObjects(debugObjDir)' "$GRADLE" || true) +if [ "$n" -ne 2 ]; then fail "T4 hasElfObjects(debugObjDir) used $n times, expected 2"; else pass; fi +n=$(grep -c '0x7f' "$GRADLE" || true) +fun_line=$(grep -n '^fun hasElfObjects' "$GRADLE" | head -1 | cut -d: -f1 || true) +magic_line=$(grep -n '0x7f' "$GRADLE" | head -1 | cut -d: -f1 || true) +if [ "$n" -ne 1 ] || [ -z "$fun_line" ] || [ -z "$magic_line" ] || + [ "$magic_line" -le "$fun_line" ] || [ $((magic_line - fun_line)) -gt 6 ]; then + fail "T4 ELF magic must appear once, inside fun hasElfObjects (count=$n, fun=$fun_line, magic=$magic_line)" +else + pass +fi + +n=$(grep -cxF './gradlew :ddprof-lib:checkSignalReachability # opt-in; SKIPPED when the debug objects are not ELF' \ + "$ROOT/utils/README.md" || true) +if [ "$n" -ne 1 ]; then fail "T4 utils/README.md standalone checkSignalReachability line changed"; else pass; fi + +n=$(grep -c 'REFCOUNT_NESTED_OVERFLOW' "$ROOT/doc/architecture/RefCountGuard.md" || true) +if [ "$n" -lt 2 ]; then fail "T4 RefCountGuard.md mentions REFCOUNT_NESTED_OVERFLOW $n times, expected >= 2"; else pass; fi + +# --- Summary ----------------------------------------------------------------- + +if [ "$FAILED" -gt 0 ]; then + echo "FAIL: $FAILED/$TESTS test groups failed" >&2 + exit 1 +fi +echo "PASS: $TESTS test groups"