refactor(vtable_target): defer receiver resolution to dump time via SafeAccess

[jbachorik]

What does this PR do?:

Redesigns vtable_target receiver-class resolution. Instead of eagerly pre-registering every loaded class into _class_map at profiler start and on ClassPrepare, the signal handler now captures the receiver's raw VMSymbol* in a new BCI_VTABLE_RECEIVER frame, and the Symbol is read at dump time via SafeAccess — crash-safe against concurrent class unloading.

Changes:

• New BCI BCI_VTABLE_RECEIVER = -21 (vmEntry.h): method_id is a VMSymbol*, not a jmethodID. Doc comment cross-referenced from the signal-handler write site (hotspotSupport.cpp) and the dump-time read site (flightRecorder.cpp).
• New SafeAccess::safeCopy primitive (safeAccess.{h,cpp}): bulk byte copy through the safefetch trampoline, with an alignment-aware strategy that keeps every 4-byte load inside a single page so over-reads past src + len never spuriously fault. Replaces a memcpy that would have crashed if the Symbol's page was unmapped between the readability check and the copy.
• Lookup::resolveVTableReceiver + resolveVTableReceiverCached (flightRecorder.{h,cpp}): SafeAccess-protected length + body read, printable-byte filter, synthetic-accessor/LambdaForm normalisation. Per-dump VMSymbol* → class_id cache so distinct Symbol addresses for the same class collapse to one MethodInfo.
• New MethodMap::makeVTableReceiverKey(u32) (flightRecorder.h): adds a fourth key namespace (11 high-bit pattern, VTABLE_RECEIVER_MARK) so BCI_VTABLE_RECEIVER frames key by resolved class_id instead of by VMSymbol* address.
• Deletions: Profiler::preregisterLoadedClasses (~100 lines + 3 call sites + decl), the ClassPrepare JVMTI hook block for vtable_target in vmEntry.cpp, classMapTrySharedGuard(), the shared-lock try-dance in walkVM. Conditional end-of-dump _class_map.clear() is restored to unconditional (was if (!_features.vtable_target || ...) for the previous design's persistence requirement).
• Counter VTABLE_RECEIVER_RESOLVE_FAILED records resolution failures (Symbol page unmapped, length out of range, printable filter rejects).
• Test renamed: VtableTargetPreregistrationTest → VtableReceiverFrameTest; assertion message updated to describe the SafeAccess path.

Motivation:

The prior preregistration design forced unbounded growth of _class_map — every loaded class ever seen by GetLoadedClasses or ClassPrepare was inserted permanently, with no removal path (clearing the map would require a JVMTI-safepoint re-enumeration on every dump). Long-running JVMs with synthetic-class generators (LambdaForm, reflection accessors, Groovy/Kotlin invokedynamic, CGLIB) accumulated millions of entries indefinitely.

The new design bounds _class_map to "classes actually sampled this chunk" (cleared unconditionally at end-of-dump), removes the eager JVMTI enumeration and the ClassPrepare insert path, eliminates the shared/exclusive lock dance in the signal handler, and is robust to concurrent class unloading via SafeAccess-based dump-time reads.

Additional Notes:

• VMSymbol lifetime / class unload between sample and dump. If the receiver class unloads after sample but before dump, three outcomes: (1) page unmapped → safeFetch32 returns sentinel → frame becomes <unresolved_vtable_receiver>, VTABLE_RECEIVER_RESOLVE_FAILED ticks. (2) memory reused for unrelated data → length / printable filter rejects → same as (1). (3) memory reused for another Symbol whose bytes pass the printable filter → wrong class name attached to the trace (silent miscorrelation). Cases (1)+(2) dominate; (3) is rare per-Symbol but non-zero in aggregate on class-unload-heavy workloads. Tracked as follow-up in PROF-14780 — class-unload breakpoint hook + Symbol-bytes shadow cache.
• CallTrace dedup across Symbol address differences. CallTraceHashTable::calcHash mixes the raw bytes of every frame (including method_id). Two samples of the same logical class whose VMSymbol* address differs (class unload + reload within a chunk) produce distinct trace ids. Accepted: normalising at sample time would require an in-signal-handler Symbol read, which the redesign explicitly avoids. The dump-time MethodMap key is class_id-based, so the synthetic <vtable_receiver> MethodInfo does collapse across distinct Symbol addresses; only CallTrace dedup is affected.
• SafeAccess::safeCopy correctness. Pages on supported platforms (x86_64 4 KiB; aarch64 Linux 4 KiB; aarch64 macOS 16 KiB) divide evenly by 4. A 4-byte-aligned 4-byte load therefore never crosses a page boundary, so over-reads beyond src + len are guaranteed safe as long as the start address itself is in a mapped page. For misaligned src, the front fixup fetches at the previous aligned address (same 4-byte word, same page) and discards leading k ∈ {1,2,3} bytes. Buffer is 4096 bytes (stack-allocated); names longer than that are recorded as resolve failures via the sentinel.
• Empty _class_map is fine across chunks. JFR class IDs are per-chunk; clearing at end-of-dump is the correct steady state.
• Adversary review surfaced and addressed: dead BCI_ALLOC branch in resolveMethod deleted (it was the previous receiver path; no other producer remains), normalisation logic consolidated into resolveVTableReceiver (no more duplicated LambdaForm$MH table). Help text in arguments.cpp updated to describe the new design.

How to test the change?:

• Existing C++ gtest: SafeFetchTest — 19/19 passes (9 pre-existing + 7 new safeCopy_* cases covering happy path, zero-length, unmapped source, PROT_NONE source, tail near unmapped boundary now succeeds (regression for the previous design), requested-range-crosses-unmapped still legitimately fails, unaligned source for all k ∈ {1,2,3}, real data equal to a single sentinel).
• Existing Java test: VtableReceiverFrameTest (renamed) compiles cleanly.
• Release build: :ddprof-lib:assembleRelease succeeds on darwin-aarch64.
• CI matrix (delegated to project CI): full gtest + Java test runs across Linux x86_64 / aarch64, musl, OpenJDK 8 / 11 / 17 / 21 / 25 — see CI.

For Datadog employees:

• If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.

• This PR doesn't touch any of that.

• JIRA: PROF-14618

• Follow-up: PROF-14780 — class-unload breakpoint hook for mid-chunk unload preservation

[Copilot]

Pull request overview

This PR restores vtable_target frame capture for CPU-only recordings by ensuring _class_map is populated from JVM-thread contexts (rather than relying on allocation/liveness paths that don’t run in CPU-only mode), keeping the signal-handler lookup path read-only and signal-safe.

Changes:

• Bulk pre-registers loaded classes into _class_map during Profiler::start() and Profiler::dump() under the existing exclusive _class_map_lock.
• Moves VM::ClassPrepare out-of-line and extends it to insert newly prepared classes into _class_map when vtable_target is enabled.
• Adds a C++ unit test covering the “bulk insert then bounded_lookup(…, 0) hits” contract and stages a (currently disabled) Java integration test.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
ddprof-test/src/test/java/com/datadoghq/profiler/cpu/VtableTargetCpuTest.java	Adds a staged (disabled) integration test outlining expected vtable-target behavior in CPU samples.
ddprof-lib/src/test/cpp/dictionary_concurrent_ut.cpp	Adds a unit test validating bulk insertion visibility to bounded_lookup(..., 0) and sentinel behavior on miss.
ddprof-lib/src/main/cpp/vmEntry.h	Moves VM::ClassPrepare definition out of the header into vmEntry.cpp.
ddprof-lib/src/main/cpp/vmEntry.cpp	Implements VM::ClassPrepare to insert normalized class signatures into the class map when vtable_target is enabled.
ddprof-lib/src/main/cpp/profiler.h	Adds Profiler::preregisterLoadedClasses(jvmtiEnv*) declaration and documents intended locking/threading constraints.
ddprof-lib/src/main/cpp/profiler.cpp	Calls preregisterLoadedClasses() during start/dump class-map resets and implements the bulk JVMTI enumeration/insertion logic.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dd-octo-sts]

CI Test Results

Run: #26456720745 | Commit: 0f710b7 | Duration: 12m 57s (longest job)

✅ All 32 test jobs passed

Status Overview

JDK	glibc-aarch64/debug	glibc-amd64/debug	musl-aarch64/debug	musl-amd64/debug
8	-	✅	-	-
8-ibm	-	✅	-	-
8-j9	✅	✅	-	-
8-librca	-	-	✅	✅
8-orcl	-	✅	-	-
11	-	✅	-	-
11-j9	✅	✅	-	-
11-librca	-	-	✅	✅
17	✅	✅	-	-
17-graal	✅	✅	-	-
17-j9	✅	✅	-	-
17-librca	-	-	✅	✅
21	✅	✅	-	-
21-graal	✅	✅	-	-
21-librca	-	-	✅	✅
25	✅	✅	-	-
25-graal	✅	✅	-	-
25-librca	-	-	✅	✅

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0

---

Updated: 2026-05-26 15:23:33 UTC

[jbachorik]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2dbee3f5a1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

ddprof-lib/src/test/cpp/dictionary_ut.cpp:292

• The test "WritersDuringRotateProduceNoCorruption" doesn’t actually run writers during rotate: all writer threads are joined before the rotate/clearStandby loop starts. This means the test can’t catch races between concurrent lookup() calls and rotate()/clearStandby(); either rename the test to match its behavior or keep writers running while rotating to exercise the intended concurrency contract.

    std::vector<std::thread> writers;
    for (int i = 0; i < 4; i++) writers.emplace_back(writer, i);
    for (auto& t : writers) t.join();

    for (int cycle = 0; cycle < 20; cycle++) {
        dict.rotate();
        dict.clearStandby();
    }

[jbachorik]

@codex review

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5a06eb7e54

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jbachorik]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[jbachorik]

@github-copilot please review

I have addressed all your concerns. I am going to dismiss the review not to block - I really need this integrated ASAP. Please, feel free to look at the PR retroactively and if there are any concerns left, I will address them then.

[kaahos]

I have found some potential bugs. Particularly a counter-ordering bug due to preregisterLoadedClasses being called before Counters::reset(), which seems to have already been noticed in commit be0e50a6 but the test was apparently dropped.