Render unsymbolized native frames as [lib+0xoffset] instead of unknown

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: jbachorik

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

@@ -375,11 +375,19 @@ MethodInfo *Lookup::resolveMethod(ASGCT_CallFrame &frame) {
       // Note: This is called during JFR serialization with lockAll() held (see Profiler::dump),
       // so the library array is stable - no concurrent dlopen_hook calls can modify it.
       CodeCache* lib = Libraries::instance()->getLibraryByIndex(lib_index);
-      if (lib != nullptr) {
+      if (lib != nullptr && lib->hasBuildId() && Profiler::instance()->isRemoteSymbolication()) {
         TEST_LOG("Found library: %s, build_id=%s", lib->name(), lib->buildId());
-        // Create temporary RemoteFrameInfo for fillRemoteFrameInfo
+        // Remote symbolication: defer to backend
         RemoteFrameInfo rfi(lib->buildId(), pc_offset, lib_index);
         fillRemoteFrameInfo(mi, &rfi);
+      } else if (lib != nullptr) {
+        // Locally unsymbolized: render as [libname+0xoffset]
+        char name_buf[256];
+        const char* s = lib->name();
+        const char* basename = strrchr(s, '/');
+        if (basename) basename++; else basename = s;
+        snprintf(name_buf, sizeof(name_buf), "[%s+0x%" PRIxPTR "]", basename, pc_offset);
+        fillNativeMethodInfo(mi, name_buf, nullptr);
       } else {
         TEST_LOG("WARNING: Library lookup failed for index %u", lib_index);
         fillNativeMethodInfo(mi, "unknown_library", nullptr);

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

@@ -365,40 +365,54 @@ void Profiler::populateRemoteFrame(ASGCT_CallFrame* frame, uintptr_t pc, CodeCac
  * - Returns is_marked=true if mark indicates termination (MARK_INTERPRETER, etc.)
  *
  * For traditional symbolication:
- * - Does full symbol resolution via findNativeMethod()
+ * - Does symbol resolution via binarySearch() on the found library
  * - Checks marks after symbol resolution (same O(log n) + O(1) cost)
+ * - If no symbol found but PC is in a known library, packs as
+ *   BCI_NATIVE_FRAME_REMOTE for library-relative rendering ([lib+0xoffset])
  */
 Profiler::NativeFrameResolution Profiler::resolveNativeFrameForWalkVM(uintptr_t pc, int lock_index) {
-  if (_remote_symbolication) {
-    CodeCache* lib = _libs->findLibraryByAddress((void*)pc);
-    if (lib != nullptr && lib->hasBuildId()) {
-      // Get symbol name and check mark
-      const char *method_name = nullptr;
-      lib->binarySearch((void*)pc, &method_name);
-      char mark = (method_name != nullptr) ? NativeFunc::read_mark(method_name) : 0;
-
-      if (mark != 0) {
-        return {nullptr, BCI_NATIVE_FRAME, true};  // Marked - stop processing
-      }
-
-      // Pack remote symbolication data using utility struct
-      uintptr_t pc_offset = pc - (uintptr_t)lib->imageBase();
-      uint32_t lib_index = (uint32_t)lib->libIndex();
-      unsigned long packed = RemoteFramePacker::pack(pc_offset, mark, lib_index);
+  CodeCache* lib = _libs->findLibraryByAddress((void*)pc);
 
-      TEST_LOG("resolveNativeFrameForWalkVM: lib=%s, build_id=%s, pc=0x%lx, pc_offset=0x%lx, mark=%d, lib_index=%u, packed=0x%zx",
-               lib->name(), lib->buildId(), pc, pc_offset, (int)mark, lib_index, packed);
+  if (_remote_symbolication && lib != nullptr && lib->hasBuildId()) {
+    // Get symbol name and check mark
+    const char *method_name = nullptr;
+    lib->binarySearch((void*)pc, &method_name);
+    char mark = (method_name != nullptr) ? NativeFunc::read_mark(method_name) : 0;
 
-      return NativeFrameResolution(packed, BCI_NATIVE_FRAME_REMOTE, false);
+    if (mark != 0) {
+      return {nullptr, BCI_NATIVE_FRAME, true};  // Marked - stop processing
     }
+
+    // Pack remote symbolication data using utility struct
+    uintptr_t pc_offset = pc - (uintptr_t)lib->imageBase();
+    uint32_t lib_index = (uint32_t)lib->libIndex();
+    unsigned long packed = RemoteFramePacker::pack(pc_offset, mark, lib_index);
+
+    TEST_LOG("resolveNativeFrameForWalkVM: lib=%s, build_id=%s, pc=0x%lx, pc_offset=0x%lx, mark=%d, lib_index=%u, packed=0x%zx",
+             lib->name(), lib->buildId(), pc, pc_offset, (int)mark, lib_index, packed);
+
+    return NativeFrameResolution(packed, BCI_NATIVE_FRAME_REMOTE, false);
   }
 
-  // Fallback: Traditional symbol resolution
-  const char *method_name = findNativeMethod((void*)pc);
+  // Traditional symbol resolution
+  const char *method_name = nullptr;
+  if (lib != nullptr) {
+    lib->binarySearch((void*)pc, &method_name);
+  }
   if (method_name != nullptr && NativeFunc::is_marked(method_name)) {
     return NativeFrameResolution(nullptr, BCI_NATIVE_FRAME, true);
   }
 
+  // No symbol but known library: pack for library-relative identification.
+  // Reuses BCI_NATIVE_FRAME_REMOTE encoding; resolveMethod() in flightRecorder.cpp
+  // distinguishes remote vs local rendering via hasBuildId() && isRemoteSymbolication().
+  if (method_name == nullptr && lib != nullptr) {
+    uintptr_t pc_offset = pc - (uintptr_t)lib->imageBase();
+    uint32_t lib_index = (uint32_t)lib->libIndex();
+    unsigned long packed = RemoteFramePacker::pack(pc_offset, 0, lib_index);
+    return NativeFrameResolution(packed, BCI_NATIVE_FRAME_REMOTE, false);
+  }
+
   return NativeFrameResolution(method_name, BCI_NATIVE_FRAME, false);
 }
 

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

@@ -315,7 +315,7 @@ class alignas(alignof(SpinLock)) Profiler {
 
     /**
      * Pack remote symbolication data into a 64-bit jmethodID.
-     * Layout: pc_offset (44 bits) | mark (3 bits) | lib_index (17 bits)
+     * Layout: pc_offset (44 bits) | mark (3 bits) | lib_index (15 bits)
      */
     static inline unsigned long pack(uintptr_t pc_offset, char mark, uint32_t lib_index) {
       return (unsigned long)(
@@ -381,6 +381,7 @@ class alignas(alignof(SpinLock)) Profiler {
                                    const char *value, const char *unit);
   void writeHeapUsage(long value, bool live);
   int eventMask() const { return _event_mask; }
+  bool isRemoteSymbolication() const { return _remote_symbolication; }
 
   const void *resolveSymbol(const char *name);
   const char *getLibraryName(const char *native_symbol);

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

@@ -657,14 +657,18 @@ __attribute__((no_sanitize("address"))) int StackWalker::walkVM(void* ucontext,
                     Counters::increment(WALKVM_ANCHOR_INLINE_NO_SP);
                 }
                 // Check previous frame for thread entry points (Rust, libc/pthread)
+                // Only check marks for traditionally-resolved frames; packed remote
+                // frames store an integer in the method_name union, not a valid pointer.
                 if (prev_native_pc != NULL) {
                     Profiler::NativeFrameResolution prev_resolution = profiler->resolveNativeFrameForWalkVM((uintptr_t)prev_native_pc, lock_index);
-                    const char* prev_method_name = (const char*)prev_resolution.method_name;
-                    if (prev_method_name != NULL) {
-                        char prev_mark = NativeFunc::read_mark(prev_method_name);
-                        if (prev_mark == MARK_THREAD_ENTRY) {
-                            Counters::increment(THREAD_ENTRY_MARK_DETECTIONS);
-                            break;
+                    if (prev_resolution.bci != BCI_NATIVE_FRAME_REMOTE) {
+                        const char* prev_method_name = prev_resolution.method_name;
+                        if (prev_method_name != NULL) {
+                            char prev_mark = NativeFunc::read_mark(prev_method_name);
+                            if (prev_mark == MARK_THREAD_ENTRY) {
+                                Counters::increment(THREAD_ENTRY_MARK_DETECTIONS);
+                                break;
+                            }
                         }
                     }
                 }

ddprof-lib/src/test/cpp/remotesymbolication_ut.cpp

@@ -8,9 +8,81 @@
 #include <unistd.h>
 #include <sys/stat.h>
 #include "symbols_linux.h"
+#include "profiler.h"
 #include "vmEntry.h"
 #include "../../main/cpp/gtest_crash_handler.h"
 
+// RemoteFramePacker tests are platform-independent (pure bit manipulation)
+using RFP = Profiler::RemoteFramePacker;
+
+TEST(RemoteFramePackerTest, RoundTrip) {
+    uintptr_t pc_offset = 0x123456789AB;
+    char mark = 5;
+    uint32_t lib_index = 1000;
+
+    unsigned long packed = RFP::pack(pc_offset, mark, lib_index);
+    EXPECT_EQ(RFP::unpackPcOffset(packed), pc_offset);
+    EXPECT_EQ(RFP::unpackMark(packed), mark);
+    EXPECT_EQ(RFP::unpackLibIndex(packed), lib_index);
+}
+
+TEST(RemoteFramePackerTest, ZeroValues) {
+    unsigned long packed = RFP::pack(0, 0, 0);
+    EXPECT_EQ(packed, 0UL);
+    EXPECT_EQ(RFP::unpackPcOffset(packed), 0UL);
+    EXPECT_EQ(RFP::unpackMark(packed), 0);
+    EXPECT_EQ(RFP::unpackLibIndex(packed), 0U);
+}
+
+TEST(RemoteFramePackerTest, MaxValues) {
+    uintptr_t max_pc = RFP::PC_OFFSET_MASK;     // 44 bits all set
+    char max_mark = (char)RFP::MARK_MASK;        // 3 bits all set = 7
+    uint32_t max_lib = (uint32_t)RFP::LIB_INDEX_MASK; // 15 bits all set = 32767
+
+    unsigned long packed = RFP::pack(max_pc, max_mark, max_lib);
+    EXPECT_EQ(RFP::unpackPcOffset(packed), max_pc);
+    EXPECT_EQ(RFP::unpackMark(packed), max_mark);
+    EXPECT_EQ(RFP::unpackLibIndex(packed), max_lib);
+}
+
+TEST(RemoteFramePackerTest, FieldIsolation) {
+    // Setting only pc_offset should not affect mark or lib_index
+    unsigned long packed_pc = RFP::pack(0xABCDE, 0, 0);
+    EXPECT_EQ(RFP::unpackMark(packed_pc), 0);
+    EXPECT_EQ(RFP::unpackLibIndex(packed_pc), 0U);
+
+    // Setting only mark should not affect pc_offset or lib_index
+    unsigned long packed_mark = RFP::pack(0, 3, 0);
+    EXPECT_EQ(RFP::unpackPcOffset(packed_mark), 0UL);
+    EXPECT_EQ(RFP::unpackLibIndex(packed_mark), 0U);
+
+    // Setting only lib_index should not affect pc_offset or mark
+    unsigned long packed_lib = RFP::pack(0, 0, 500);
+    EXPECT_EQ(RFP::unpackPcOffset(packed_lib), 0UL);
+    EXPECT_EQ(RFP::unpackMark(packed_lib), 0);
+}
+
+TEST(RemoteFramePackerTest, Overflow_PcOffsetTruncated) {
+    // pc_offset larger than 44 bits should be silently truncated
+    uintptr_t oversize_pc = (1ULL << 44) | 0x42;
+    unsigned long packed = RFP::pack(oversize_pc, 0, 0);
+    EXPECT_EQ(RFP::unpackPcOffset(packed), (uintptr_t)0x42)
+        << "Bits above 44 should be masked off";
+}
+
+TEST(RemoteFramePackerTest, UnsymbolizedFramePacksWithZeroMark) {
+    // The new code path packs unsymbolized frames with mark=0
+    uintptr_t pc_offset = 0x1A2B3C;
+    uint32_t lib_index = 7;
+    unsigned long packed = RFP::pack(pc_offset, 0, lib_index);
+
+    EXPECT_EQ(RFP::unpackPcOffset(packed), pc_offset);
+    EXPECT_EQ(RFP::unpackMark(packed), 0);
+    EXPECT_EQ(RFP::unpackLibIndex(packed), lib_index);
+    // Packed value is non-zero even with mark=0 (prevents false NULL check)
+    EXPECT_NE(packed, 0UL);
+}
+
 #ifdef __linux__
 
 static constexpr char REMOTE_TEST_NAME[] = "RemoteSymbolicationTest";