Fix leaf lineno

Author: dalehamel

host/host.go

@@ -38,6 +38,7 @@ type Frame struct {
 	File          FileID
 	Lineno        libpf.AddressOrLineno
 	Type          libpf.FrameType
+	Extra         libpf.AddressOrLineno
 	ReturnAddress bool
 }
 

interpreter/ruby/ruby.go

@@ -318,9 +318,6 @@ type rubyIseq struct {
 	// methodName is the optional method name for this iseq
 	// only present on CME-based iseq
 	methodName libpf.String
-
-	// line of code in source file for this instruction sequence
-	line libpf.SourceLineno
 }
 
 type rubyInstance struct {
@@ -862,11 +859,6 @@ func unpackEnvFlags(packed uint16) uint32 {
 
 func (r *rubyInstance) readIseqBody(iseqBody, pc libpf.Address, frameAddrType uint8, frameFlags uint32) (*rubyIseq, error) {
 	vms := &r.r.vmStructs
-	lineNo, err := r.getRubyLineNo(iseqBody, uint64(pc))
-	if err != nil {
-		lineNo = 0
-		log.Warnf("RubySymbolizer: Failed to get line number (%d) %v", frameAddrType, err)
-	}
 
 	// Read contiguous pointer values into a buffer to be more efficient
 	dataBytes := make([]byte, 3*vms.size_of_value)
@@ -919,7 +911,6 @@ func (r *rubyInstance) readIseqBody(iseqBody, pc libpf.Address, frameAddrType ui
 		baseLabel:      iseqBaseLabel,
 		methodName:     methodName,
 		sourceFileName: sourceFileName,
-		line:           libpf.SourceLineno(lineNo),
 	}, nil
 }
 
@@ -1009,14 +1000,22 @@ func (r *rubyInstance) Symbolize(frame *host.Frame, frames *libpf.Frames) error
 		fullLabel = qualifiedMethodName(classPath, methodName, singleton)
 		sourceFile = cfuncDummyFile
 	} else {
+
 		// The Ruby VM program counter that was extracted from the current call frame is embedded in
-		// the Linenos field.
+		// the Linenos field, and we use the iseq address passed in frame.Extra to decode it
+
+		lineNo, err := r.getRubyLineNo(libpf.Address(frame.Extra), uint64(pc))
+		if err != nil {
+			lineNo = 0
+			log.Warnf("RubySymbolizer: Failed to get line number (%d) %v", frameAddrType, err)
+		}
+
 		iseq, err := r.readIseqBody(iseqBody, pc, frameAddrType, frameFlags)
 		if err != nil {
 			return err
 		}
 		sourceFile = iseq.sourceFileName
-		sourceLine = iseq.line
+		sourceLine = libpf.SourceLineno(lineNo)
 
 		fullLabel = profileFrameFullLabel(classPath, iseq.label, iseq.baseLabel, iseq.methodName, singleton, cframe)
 

support/ebpf/ruby_tracer.ebpf.c

@@ -66,7 +66,8 @@ typedef struct vm_env_struct {
 // frame_type is encoded into the "file" attribute of frame in the spare bits
 // frame flags are encoded in the upper bits of "line" for debugging purposes, but this
 // is may change in the future.
-static EBPF_INLINE ErrorCode push_ruby(Trace *trace, u16 flags, u8 frame_type, u64 file, u64 line)
+static EBPF_INLINE ErrorCode
+push_ruby(Trace *trace, u16 flags, u8 frame_type, u64 file, u64 line, u64 iseq)
 {
   if (frame_type != FRAME_TYPE_NONE) {
     // Ensure address is actually no more than 48-bits
@@ -90,7 +91,7 @@ static EBPF_INLINE ErrorCode push_ruby(Trace *trace, u16 flags, u8 frame_type, u
       line       = packed;
     }
   }
-  return _push(trace, file, line, FRAME_MARKER_RUBY);
+  return _push_with_extra(trace, file, line, iseq, FRAME_MARKER_RUBY);
 }
 
 // Read a single Ruby frame
@@ -107,6 +108,8 @@ static EBPF_INLINE ErrorCode read_ruby_frame(
   u8 frame_type;
   // Actual frame address of the given type
   u64 frame_addr;
+  // Address of the cfp->iseq, used to get the line number using the pc
+  u64 iseq_addr = 0;
   u64 pc;
 
   Trace *trace     = &record->trace;
@@ -268,27 +271,30 @@ static EBPF_INLINE ErrorCode read_ruby_frame(
     }
   }
 
-  // Fallback to just reading the iseq if we couldn't detect a supported CME type
-  if (frame_type == FRAME_TYPE_NONE) {
+  if (!cfunc) {
+    // Read the control frame iseq so we can get the line number
     if (control_frame.iseq == NULL) {
       increment_metric(metricID_UnwindRubyErrInvalidIseq);
       return ERR_RUBY_INVALID_ISEQ;
-    }
-
-    if (control_frame.iseq != NULL) {
+    } else {
       if (bpf_probe_read_user(
-            &frame_addr, sizeof(frame_addr), (void *)(control_frame.iseq + rubyinfo->body))) {
+            &iseq_addr, sizeof(iseq_addr), (void *)(control_frame.iseq + rubyinfo->body))) {
         increment_metric(metricID_UnwindRubyErrReadIseqBody);
         return ERR_RUBY_READ_ISEQ_BODY;
       }
-      frame_type = FRAME_TYPE_ISEQ;
     }
   }
 
+  // Fallback to just reading the iseq if we couldn't detect a supported CME type
+  if (frame_type == FRAME_TYPE_NONE) {
+    frame_addr = iseq_addr;
+    frame_type = FRAME_TYPE_ISEQ;
+  }
+
   // For symbolization of the frame we forward the information about the CME,
   // or plain iseq to userspace, along with the pc so we can get line information.
   // From this we can then extract information like file or function name and line number.
-  ErrorCode error = push_ruby(trace, packed_flags, frame_type, frame_addr, pc);
+  ErrorCode error = push_ruby(trace, packed_flags, frame_type, frame_addr, pc, iseq_addr);
   if (error) {
     DEBUG_PRINT("ruby: failed to push frame");
     return error;
@@ -398,7 +404,8 @@ static EBPF_INLINE ErrorCode walk_ruby_stack(
   // If we entered native unwinding because we saw a cfunc frame, lets push that
   // frame now so it can take "ownership" of the native code that was unwound
   if (record->rubyUnwindState.cfunc_saved_frame != 0) {
-    error = push_ruby(trace, 0, FRAME_TYPE_CME_CFUNC, record->rubyUnwindState.cfunc_saved_frame, 0);
+    error =
+      push_ruby(trace, 0, FRAME_TYPE_CME_CFUNC, record->rubyUnwindState.cfunc_saved_frame, 0, 0);
     if (error) {
       DEBUG_PRINT("ruby: failed to push cframe");
       return error;

support/ebpf/tracemgmt.h

@@ -329,27 +329,32 @@ static inline EBPF_INLINE bool unwinder_unwind_frame_pointer(UnwindState *state)
 //       calc_line). This should probably be renamed to something like "frame type
 //       specific data".
 static inline EBPF_INLINE ErrorCode _push_with_max_frames(
-  Trace *trace, u64 file, u64 line, u8 frame_type, u8 return_address, u32 max_frames)
+  Trace *trace, u64 file, u64 line, u64 extra, u8 frame_type, u8 return_address, u32 max_frames)
 {
   if (trace->stack_len >= max_frames) {
     DEBUG_PRINT("unable to push frame: stack is full");
     increment_metric(metricID_UnwindErrStackLengthExceeded);
     return ERR_STACK_LENGTH_EXCEEDED;
   }
 
+  u64 extra_addr = (u64)extra & 0x0000FFFFFFFFFFFFULL;
 #ifdef TESTING_COREDUMP
   // tools/coredump uses CGO to build the eBPF code. This dispatches
   // the frame information directly to helper implemented in ebpfhelpers.go.
-  int __push_frame(u64, u64, u64, u8, u8);
+  int __push_frame(u64, u64, u64, u64, u8, u8);
   trace->stack_len++;
-  return __push_frame(__cgo_ctx->id, file, line, frame_type, return_address);
+  return __push_frame(__cgo_ctx->id, file, line, extra_addr, frame_type, return_address);
 #else
-  trace->frames[trace->stack_len++] = (Frame){
+  Frame frame = {
     .file_id        = file,
     .addr_or_line   = line,
     .kind           = frame_type,
     .return_address = return_address,
   };
+  if (extra_addr != 0) {
+    __builtin_memcpy(frame.pad, &extra_addr, 6);
+  }
+  trace->frames[trace->stack_len++] = frame;
 
   return ERR_OK;
 #endif
@@ -360,19 +365,27 @@ static inline EBPF_INLINE ErrorCode
 _push_with_return_address(Trace *trace, u64 file, u64 line, u8 frame_type, bool return_address)
 {
   return _push_with_max_frames(
-    trace, file, line, frame_type, return_address, MAX_NON_ERROR_FRAME_UNWINDS);
+    trace, file, line, 0, frame_type, return_address, MAX_NON_ERROR_FRAME_UNWINDS);
 }
 
 // Push the file ID, line number and frame type into FrameList
 static inline EBPF_INLINE ErrorCode _push(Trace *trace, u64 file, u64 line, u8 frame_type)
 {
-  return _push_with_max_frames(trace, file, line, frame_type, 0, MAX_NON_ERROR_FRAME_UNWINDS);
+  return _push_with_max_frames(trace, file, line, 0, frame_type, 0, MAX_NON_ERROR_FRAME_UNWINDS);
+}
+
+// Push the file ID, line number and frame type with an extra address into FrameList
+static inline EBPF_INLINE ErrorCode
+_push_with_extra(Trace *trace, u64 file, u64 line, u64 extra, u8 frame_type)
+{
+  return _push_with_max_frames(
+    trace, file, line, extra, frame_type, 0, MAX_NON_ERROR_FRAME_UNWINDS);
 }
 
 // Push a critical error frame.
 static inline EBPF_INLINE ErrorCode push_error(Trace *trace, ErrorCode error)
 {
-  return _push_with_max_frames(trace, 0, error, FRAME_MARKER_ABORT, 0, MAX_FRAME_UNWINDS);
+  return _push_with_max_frames(trace, 0, error, 0, FRAME_MARKER_ABORT, 0, MAX_FRAME_UNWINDS);
 }
 
 // Send a trace to user-land via the `trace_events` perf event buffer.

support/ebpf/tracer.ebpf.amd64

@@ -36,13 +36,14 @@ func __bpf_log(buf unsafe.Pointer, sz C.int) {
 }
 
 //export __push_frame
-func __push_frame(id, file, line C.u64, frameType, returnAddress C.uchar) C.int {
+func __push_frame(id, file, line, extra C.u64, frameType, returnAddress C.uchar) C.int {
 	ctx := ebpfContextMap[id]
 
 	ctx.trace.Frames = append(ctx.trace.Frames, host.Frame{
 		File:          host.FileID(file),
 		Lineno:        libpf.AddressOrLineno(line),
 		Type:          libpf.FrameType(frameType),
+		Extra:         libpf.AddressOrLineno(extra),
 		ReturnAddress: returnAddress != 0,
 	})
 

support/ebpf/tracer.ebpf.arm64

@@ -4,7 +4,7 @@
     {
       "lwp": 1849675,
       "frames": [
-        "Object#is_prime+0 in /app/loop.rb:45481984",
+        "Object#is_prime+0 in /app/loop.rb:10",
         "libruby.so.3.3.9+0x31ba80",
         "libruby.so.3.3.9+0x321007",
         "libruby.so.3.3.9+0x22912d",

tools/coredump/ebpfhelpers.go

@@ -4,7 +4,7 @@
     {
       "lwp": 1177439,
       "frames": [
-        "Object#is_prime+0 in /app/loop.rb:0",
+        "Object#is_prime+0 in /app/loop.rb:10",
         "libruby.so.3.4.5+0x346a40",
         "libruby.so.3.4.5+0x34bb17",
         "libruby.so.3.4.5+0x24a095",

tools/coredump/testdata/amd64/ruby-3.3.9-loop.json

@@ -4,7 +4,7 @@
     {
       "lwp": 1844728,
       "frames": [
-        "Object#is_prime+0 in /app/loop.rb:0",
+        "Object#is_prime+0 in /app/loop.rb:10",
         "libruby.so.3.5.0+0x3399b0",
         "libruby.so.3.5.0+0x33e9d7",
         "libruby.so.3.5.0+0x23e175",