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

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

@@ -163,6 +163,13 @@ Error Arguments::parse(const char *args) {
       if (_cpu < 0) {
         msg = "cpu must be >= 0";
       }
+      // vtable_target: resolve vtable/itable stub receiver classes in CPU traces.
+      // Signal handler stores the raw receiver VMSymbol* in a BCI_VTABLE_RECEIVER
+      // frame (no lock, no map lookup, no allocation). Resolution happens at dump
+      // time via SafeAccess-protected reads in Lookup::resolveVTableReceiver,
+      // which is crash-safe against concurrent class unloading. _class_map only
+      // grows with classes actually sampled during the chunk.
+      _features.vtable_target = 1;
 
       CASE("wall")
       if (value == NULL) {

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

@@ -67,6 +67,7 @@
   X(REMOTE_SYMBOLICATION_FRAMES, "remote_symbolication_frames")                \
   X(REMOTE_SYMBOLICATION_LIBS_WITH_BUILD_ID, "remote_symbolication_libs_with_build_id") \
   X(REMOTE_SYMBOLICATION_BUILD_ID_CACHE_HITS, "remote_symbolication_build_id_cache_hits") \
+  X(VTABLE_RECEIVER_RESOLVE_FAILED, "vtable_receiver_resolve_failed")           \
   X(THREAD_ENTRY_MARK_DETECTIONS, "thread_entry_mark_detections")              \
   X(WALKVM_THREAD_INACCESSIBLE, "walkvm_thread_inaccessible")                  \
   X(WALKVM_ANCHOR_NULL, "walkvm_anchor_null")                                  \

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

@@ -359,18 +359,128 @@ void Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method,
   jni->PopLocalFrame(NULL);
 }
 
+bool Lookup::resolveVTableReceiver(VMSymbol *sym, char *buf, size_t bufsize,
+                                    u32 *out_class_id) {
+  if (sym == nullptr || !SafeAccess::isReadable(sym)) {
+    return false;
+  }
+  // Read the 4-byte word containing the u2 length field. In all HotSpot
+  // versions we support the length is at offset 0 of Symbol; we still go
+  // through VMStructs in case that ever changes. The low 16 bits hold the
+  // length on little-endian targets (all supported platforms).
+  int32_t *len_word_addr =
+      (int32_t *)((char *)sym + VMSymbol::lengthOffset());
+  int32_t w1 = SafeAccess::safeFetch32(len_word_addr, -1);
+  int32_t w2 = SafeAccess::safeFetch32(len_word_addr, 0);
+  if (w1 == -1 && w2 == 0) {
+    return false;
+  }
+  unsigned len = (unsigned)(w1 & 0xFFFF);
+  // Bounds: a usable internal class name needs at least 1 byte (single-char
+  // descriptors like "B"/"C" for primitives never appear as vtable receivers
+  // because primitives can't be receivers of virtual or interface dispatch).
+  // Upper bound is the caller-provided buffer; class names above this length
+  // are dropped — operators see VTABLE_RECEIVER_RESOLVE_FAILED rise.
+  if (len == 0 || len > bufsize) {
+    return false;
+  }
+  const void *body = (const char *)sym + VMSymbol::bodyOffset();
+  if (!SafeAccess::safeCopy(buf, body, len)) {
+    return false;
+  }
+  // Reject anything that doesn't look like a JVM internal class name.
+  // Valid bytes for slash-separated internal names: '/', '$', '[', ';', '_',
+  // alnum. Rejecting reduces — but does not eliminate — the case where the
+  // Symbol slot was reused for unrelated data that happens to be printable.
+  for (unsigned i = 0; i < len; i++) {
+    unsigned char c = (unsigned char)buf[i];
+    if (c < 0x20 || c >= 0x7F) {
+      return false;
+    }
+  }
+  u32 class_id = _classes->lookup(buf, len);
+  // Apply synthetic-accessor/LambdaForm normalisation so that the many
+  // distinct names HotSpot generates for these families (..Accessor1234,
+  // LambdaForm$MH/0x...) collapse to one bucket each in the JFR class pool.
+  // Folding the normalisation inside resolveVTableReceiver keeps the call
+  // site in resolveMethod minimal and ensures the cache stores normalised
+  // class ids (so MethodMap deduplication works for these families too).
+  if (has_prefix_n(buf, len,
+                   "jdk/internal/reflect/GeneratedConstructorAccessor")) {
+    class_id =
+        _classes->lookup("jdk/internal/reflect/GeneratedConstructorAccessor");
+  } else if (has_prefix_n(buf, len, "sun/reflect/GeneratedConstructorAccessor")) {
+    class_id = _classes->lookup("sun/reflect/GeneratedConstructorAccessor");
+  } else if (has_prefix_n(buf, len,
+                          "jdk/internal/reflect/GeneratedMethodAccessor")) {
+    class_id = _classes->lookup("jdk/internal/reflect/GeneratedMethodAccessor");
+  } else if (has_prefix_n(buf, len, "sun/reflect/GeneratedMethodAccessor")) {
+    class_id = _classes->lookup("sun/reflect/GeneratedMethodAccessor");
+  } else if (has_prefix_n(buf, len, "java/lang/invoke/LambdaForm$")) {
+    size_t prefix_len = strlen("java/lang/invoke/LambdaForm$");
+    const char *suffix = buf + prefix_len;
+    size_t suffix_len = len - prefix_len;
+    if (suffix_len >= 2 && suffix[0] == 'M' && suffix[1] == 'H') {
+      class_id = _classes->lookup("java/lang/invoke/LambdaForm$MH");
+    } else if (suffix_len >= 3 && suffix[0] == 'B' && suffix[1] == 'M' &&
+               suffix[2] == 'H') {
+      class_id = _classes->lookup("java/lang/invoke/LambdaForm$BMH");
+    } else if (suffix_len >= 3 && suffix[0] == 'D' && suffix[1] == 'M' &&
+               suffix[2] == 'H') {
+      class_id = _classes->lookup("java/lang/invoke/LambdaForm$DMH");
+    }
+  }
+  *out_class_id = class_id;
+  return true;
+}
+
+u32 Lookup::resolveVTableReceiverCached(void *sym) {
+  auto cached = _vtable_receiver_cache.find(sym);
+  if (cached != _vtable_receiver_cache.end()) {
+    return cached->second;
+  }
+  // Stack buffer sized to fit virtually every real class name. HotSpot
+  // Symbol length is u2 (max 65535); names beyond 4096 bytes are rare
+  // (deeply nested LambdaForm signatures, large CGLIB proxies) and are
+  // recorded as resolve failures via the sentinel below.
+  char buf[4096];
+  u32 class_id = 0;
+  if (!resolveVTableReceiver((VMSymbol *)sym, buf, sizeof(buf), &class_id)) {
+    Counters::increment(VTABLE_RECEIVER_RESOLVE_FAILED);
+    // Explicit sentinel so JFR renders an obvious "we couldn't read it"
+    // marker instead of an empty class name (which is indistinguishable
+    // from a parser/encoder error downstream).
+    class_id = _classes->lookup("<unresolved_vtable_receiver>");
+  }
+  _vtable_receiver_cache[sym] = class_id;
+  return class_id;
+}
+
 MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
   static const char* UNKNOWN = "unknown";
   unsigned long key;
   jint bci = frame.bci;
 
   jmethodID method = frame.method_id;
+
+  // BCI_VTABLE_RECEIVER: method holds a VMSymbol* (see vmEntry.h). Resolve
+  // to a class_id via the per-dump cache once, then key MethodMap by the
+  // resolved class_id so two distinct Symbol addresses for the same class
+  // name (class unload + reload within a chunk) collapse to one MethodInfo
+  // row.
+  u32 vtable_class_id = 0;
+  if (bci == BCI_VTABLE_RECEIVER) {
+    vtable_class_id = resolveVTableReceiverCached((void *)method);
+  }
+
   if (method == nullptr) {
     key = MethodMap::makeKey(UNKNOWN);
   } else if (bci == BCI_ERROR || bci == BCI_NATIVE_FRAME) {
     key = MethodMap::makeKey(frame.native_function_name);
   } else if (bci == BCI_NATIVE_FRAME_REMOTE) {
     key = MethodMap::makeKey(frame.packed_remote_frame);
+  } else if (bci == BCI_VTABLE_RECEIVER) {
+    key = MethodMap::makeVTableReceiverKey(vtable_class_id);
   } else {
     FrameTypeId frame_type = FrameType::decode(bci);
     assert(frame_type == FRAME_INTERPRETED || frame_type == FRAME_JIT_COMPILED ||
@@ -427,6 +537,18 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
         TEST_LOG("WARNING: Library lookup failed for index %u", lib_index);
         fillNativeMethodInfo(mi, "unknown_library", nullptr);
       }
+    } else if (bci == BCI_VTABLE_RECEIVER) {
+      // Synthetic vtable-receiver frame: method_id holds a VMSymbol*
+      // captured in walkVM. The Symbol -> class_id resolution (with
+      // synthetic-accessor/LambdaForm normalisation) was already done
+      // above via resolveVTableReceiverCached, which also handles
+      // resolution failures by mapping them to "<unresolved_vtable_receiver>"
+      // and incrementing VTABLE_RECEIVER_RESOLVE_FAILED.
+      mi->_class = vtable_class_id;
+      mi->_name = _symbols.lookup("<vtable_receiver>");
+      mi->_sig = _symbols.lookup("()V");
+      mi->_type = FRAME_NATIVE;
+      mi->_is_entry = false;
     } else {
       fillJavaMethodInfo(mi, method, first_time);
     }
@@ -435,6 +557,18 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
   return mi;
 }
 
+void Lookup::initClassCache() {
+  // Snapshot _classes into _class_cache for use by resolveMethod(BCI_ALLOC).
+  // Must be called before writeStackTraces() so the snapshot covers all
+  // vtable-receiver classes (pre-registered before profiling starts).
+  // This snapshot is intentionally NOT used by writeClasses(): regular Java
+  // classes are inserted into _classes by fillJavaMethodInfo() during
+  // writeStackTraces/writeMethods, so writeClasses() must re-collect after
+  // those passes to obtain the complete class pool.
+  auto guard = Profiler::instance()->classMapSharedGuard();
+  _classes->collect(_class_cache);
+}
+
 u32 Lookup::getPackage(const char *class_name) {
   const char *package = strrchr(class_name, '/');
   if (package == NULL) {
@@ -1196,11 +1330,16 @@ void Recording::writeCpool(Buffer *buf) {
   // constant pool count - bump each time a new pool is added
   buf->put8(12);
 
-  // classMap() is shared across the dump (this thread) and the JVMTI shared-lock
-  // writers (Profiler::lookupClass and friends). writeClasses() holds
-  // classMapSharedGuard() for its full duration; the exclusive classMap()->clear()
-  // in Profiler::dump runs only after this method returns.
+  // Two-phase classMap locking: initClassCache() takes the shared lock early to
+  // snapshot vtable-receiver class names for resolveMethod(BCI_ALLOC).  The snapshot
+  // is valid for the whole writeCpool() call because classMap()->clear() (exclusive
+  // lock) only runs in Profiler::dump after writeCpool() returns.
+  // writeClasses() takes the shared lock a second time to collect the COMPLETE class
+  // set: fillJavaMethodInfo() inserts every Java class into _classes during
+  // writeStackTraces/writeMethods, so the early snapshot would miss them all and
+  // produce a class pool with null class names in every stack frame.
   Lookup lookup(this, &_method_map, Profiler::instance()->classMap());
+  lookup.initClassCache();
   writeFrameTypes(buf);
   writeThreadStates(buf);
   writeExecutionModes(buf);
@@ -1413,11 +1552,11 @@ void Recording::writeMethods(Buffer *buf, Lookup *lookup) {
 
 void Recording::writeClasses(Buffer *buf, Lookup *lookup) {
   DEBUG_ASSERT_NOT_IN_SIGNAL();
-  std::map<u32, const char *> classes;
   // Hold classMapSharedGuard() for the full function. The const char* pointers
   // stored in classes point into dictionary row storage; clear() frees that
   // storage under the exclusive lock, so we must not release the shared lock
   // until we have finished iterating.
+  std::map<u32, const char *> classes;
   auto guard = Profiler::instance()->classMapSharedGuard();
   lookup->_classes->collect(classes);
 

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

@@ -8,6 +8,7 @@
 #define _FLIGHTRECORDER_H
 
 #include <map>
+#include <unordered_map>
 #include <unordered_set>
 
 #include <limits.h>
@@ -28,6 +29,8 @@
 #include "threadIdTable.h"
 #include "vmEntry.h"
 
+class VMSymbol;  // hotspot/vmStructs.h
+
 const u64 MAX_JLONG = 0x7fffffffffffffffULL;
 const u64 MIN_JLONG = 0x8000000000000000ULL;
 const int MAX_JFR_EVENT_SIZE = 256;
@@ -115,13 +118,16 @@ class MethodInfo {
 // 3) Encoded RemoteFrameInfo
 // The values of the keys are potentially overlapping, so we use 
 // the highest 2 bits to distinguish them.
-// 00 - jmethodID
-// 10 - void* address
-// 01 - RemoteFrameInfo
+// Key encoding (top two bits):
+//   00 - jmethodID
+//   10 - void* address (native frame names)
+//   01 - RemoteFrameInfo (packed remote symbolication)
+//   11 - vtable_receiver class_id (BCI_VTABLE_RECEIVER frames)
 class MethodMap : public std::map<unsigned long, MethodInfo> {
 public:
   static constexpr unsigned long ADDRESS_MARK = 0x8000000000000000ULL;
   static constexpr unsigned long REMOTE_FRAME_MARK = 0x4000000000000000ULL;
+  static constexpr unsigned long VTABLE_RECEIVER_MARK = ADDRESS_MARK | REMOTE_FRAME_MARK;
   static constexpr unsigned long KEY_TYPE_MASK = ADDRESS_MARK | REMOTE_FRAME_MARK;
 
   MethodMap() {}
@@ -142,6 +148,15 @@ class MethodMap : public std::map<unsigned long, MethodInfo> {
     unsigned long key = packed_remote_frame;
     assert((key & KEY_TYPE_MASK) == 0);
     return (key | REMOTE_FRAME_MARK);}
+
+  // BCI_VTABLE_RECEIVER frames key by the resolved class_id (not by the
+  // VMSymbol* captured at sample time), so two distinct Symbol addresses
+  // for the same class name collapse to a single MethodInfo row.
+  static unsigned long makeVTableReceiverKey(u32 class_id) {
+    unsigned long key = (unsigned long)class_id;
+    assert((key & KEY_TYPE_MASK) == 0);
+    return (key | VTABLE_RECEIVER_MARK);
+  }
 };
 
 class Recording {
@@ -306,6 +321,14 @@ class Lookup {
   Recording *_rec;
   MethodMap *_method_map;
   Dictionary *_classes;
+  std::map<u32, const char*> _class_cache;  // snapshot of _classes, populated once at dump time
+  // Per-dump VMSymbol* -> resolved class_id cache for BCI_VTABLE_RECEIVER
+  // frames. Two purposes: (1) amortise the SafeAccess work to once per
+  // distinct Symbol pointer per dump; (2) the resolved class_id is used
+  // as the MethodMap key, so distinct Symbol* addresses for the same
+  // class name (class unload/reload mid-chunk) collapse to a single
+  // MethodInfo row.
+  std::unordered_map<void*, u32> _vtable_receiver_cache;
   Dictionary _packages;
   Dictionary _symbols;
 
@@ -318,12 +341,40 @@ class Lookup {
   bool has_prefix(const char *str, const char *prefix) const {
     return strncmp(str, prefix, strlen(prefix)) == 0;
   }
+  // Length-bounded variant for buffers that may not be NUL-terminated.
+  bool has_prefix_n(const char *buf, size_t buf_len, const char *prefix) const {
+    size_t plen = strlen(prefix);
+    return buf_len >= plen && strncmp(buf, prefix, plen) == 0;
+  }
+
+  // Resolves a VMSymbol* captured at sample time (BCI_VTABLE_RECEIVER) into a
+  // class id in _classes, applying the synthetic-accessor/LambdaForm
+  // normalisation inline. Crash-safe under concurrent class unloading: all
+  // reads of the Symbol go through SafeAccess (safefetch + bounded copy), so
+  // a Symbol freed and its page unmapped between sample and dump cannot
+  // SIGSEGV the dump thread. On success returns true and fills *out_class_id
+  // with the normalised class id. `buf` is a working area used internally;
+  // its contents on return are unspecified.
+  bool resolveVTableReceiver(VMSymbol *sym, char *buf, size_t bufsize,
+                             u32 *out_class_id);
+
+  // Cache wrapper: look up Symbol* in _vtable_receiver_cache; on miss,
+  // resolve via resolveVTableReceiver and cache the result. On any
+  // resolution failure (SafeAccess fault, length out of range, non-printable
+  // bytes) returns the sentinel "<unresolved_vtable_receiver>" class_id and
+  // increments VTABLE_RECEIVER_RESOLVE_FAILED.
+  u32 resolveVTableReceiverCached(void *sym);
 
 public:
   Lookup(Recording *rec, MethodMap *method_map, Dictionary *classes)
       : _rec(rec), _method_map(method_map), _classes(classes), _packages(),
         _symbols() {}
 
+  // Call once before writeStackTraces. Collects the class-map snapshot under
+  // the shared lock so that resolveMethod (BCI_ALLOC) and writeClasses can
+  // both use _class_cache without a second collect.
+  void initClassCache();
+
   MethodInfo *resolveMethod(ASGCT_CallFrame &frame);
   u32 getPackage(const char *class_name);
   u32 getSymbol(const char *name);

ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp

@@ -535,22 +535,16 @@ __attribute__((no_sanitize("address"))) int HotspotSupport::walkVM(void* ucontex
                     uintptr_t receiver = frame.jarg0();
                     if (receiver != 0) {
                         VMSymbol* symbol = VMKlass::fromOop(receiver)->name();
-                        // walkVM runs in a signal handler. _class_map is mutated
-                        // under _class_map_lock (shared by Profiler::lookupClass
-                        // inserters, exclusive by _class_map.clear() in the dump
-                        // path between unlockAll() and lock()). bounded_lookup
-                        // with size_limit=0 never inserts (no malloc), but it
-                        // still traverses row->next and reads row->keys, which
-                        // clear() concurrently frees. Take the lock shared via
-                        // try-lock; if an exclusive clear() is in progress, drop
-                        // the synthetic frame rather than read freed memory.
-                        auto guard = profiler->classMapTrySharedGuard();
-                        if (guard.ownsLock()) {
-                            u32 class_id = profiler->classMap()->bounded_lookup(
-                                symbol->body(), symbol->length(), 0);
-                            if (class_id != INT_MAX) {
-                                fillFrame(frames[depth++], BCI_ALLOC, class_id);
-                            }
+                        // Store the raw VMSymbol* in the frame's method_id
+                        // slot. BCI_VTABLE_RECEIVER (vmEntry.h) repurposes
+                        // method_id for this pointer — same precedent as
+                        // BCI_NATIVE_FRAME storing const char* and
+                        // BCI_NATIVE_FRAME_REMOTE storing a packed blob.
+                        // Resolution happens at dump time via SafeAccess so
+                        // a concurrent class-unload + Symbol free cannot
+                        // crash the dump thread (see Lookup::resolveVTableReceiver).
+                        if (symbol != nullptr) {
+                            fillFrame(frames[depth++], BCI_VTABLE_RECEIVER, (void*)symbol);
                         }
                     }
                 }

ddprof-lib/src/main/cpp/hotspot/vmStructs.h

@@ -606,6 +606,12 @@ DECLARE(VMSymbol)
         assert(_symbol_body_offset >= 0);
         return at(_symbol_body_offset);
     }
+
+    // Public accessors for safefetch-based dump-time resolution (no `this`
+    // deref): used to compute the address of the length/body fields without
+    // touching the Symbol's memory, so callers can probe with SafeAccess.
+    static int lengthOffset() { return _symbol_length_offset; }
+    static int bodyOffset()   { return _symbol_body_offset; }
 DECLARE_END
 
 DECLARE(VMClassLoaderData)