Store all information needed to symbolize a frame in one frame instead of using previousFrame hack

Author: gnurizen

host/host.go

@@ -44,6 +44,8 @@ type Frame struct {
 	Lineno        libpf.AddressOrLineno
 	Type          libpf.FrameType
 	ReturnAddress bool
+	LJCalleePC    uint32
+	LJCallerPC    uint32
 }
 
 type Trace struct {

interpreter/luajit/luajit.go

@@ -79,10 +79,7 @@ type luajitInstance struct {
 	traceHashes map[libpf.Address]uint64
 	cycle       int
 
-	// In order to symbolize frame n we need to know the caller or frame n+1 so we don't
-	// symbolize frame n until symbolize is called fo frame n+1.
-	previousFrame *host.Frame
-	g2Traces      uint16
+	g2Traces uint16
 }
 
 var (
@@ -349,12 +346,7 @@ func (l *luajitInstance) getGCproto(pt libpf.Address) (*proto, error) {
 
 // symbolizeFrame symbolizes the previous (up the stack)
 func (l *luajitInstance) symbolizeFrame(symbolReporter reporter.SymbolReporter,
-	funcName string, trace *libpf.Trace) error {
-	if l.previousFrame == nil || l.previousFrame.File == 0 {
-		return errors.New("previous frame not set")
-	}
-	ptAddr := libpf.Address(l.previousFrame.File)
-	pc := uint32(l.previousFrame.Lineno)
+	funcName string, trace *libpf.Trace, ptAddr libpf.Address, pc uint32) error {
 	var line uint32
 	var fileName string
 	if ptAddr != C.LUAJIT_FFI_FUNC {
@@ -388,29 +380,25 @@ func (l *luajitInstance) symbolizeFrame(symbolReporter reporter.SymbolReporter,
 	return nil
 }
 
-// Symbolize is a little weird in lua since we need the caller frame to get the name of the
-// function being called. So we stash the info for the current frame and each symbolize call
-// actually symolizes the previous frame.  The unwinder emits a special frame with file 0 to
-// indicate the end of the lua stack.
 func (l *luajitInstance) Symbolize(symbolReporter reporter.SymbolReporter, frame *host.Frame,
 	trace *libpf.Trace) error {
 	if !frame.Type.IsInterpType(libpf.LuaJIT) {
 		return interpreter.ErrMismatchInterpreterType
 	}
 
-	// We need the calling frame to find the name of the current frame so save it and do it on next
-	// frame. This assumes that the harness calls Symbolize from top to bottom of the stack in
-	// order!!  The final luajit frame will have a 0 frame.File.
-	if l.previousFrame == nil {
-		l.previousFrame = frame
-		logf("lj: new LuaJIT frame")
+	if frame.File == 0 && frame.Lineno != 0 {
+		// The BPF program will stash pointer to "G" when it sees a JIT frame w/o trace information
+		// which may fail to unwind, we use it to see if the traces for this VM have changed. When
+		// we reach a steady state where there's no new JIT activity this will always be 0.
+		g := libpf.Address(frame.Lineno)
+		if g != 0 {
+			l.mu.Lock()
+			defer l.mu.Unlock()
+			l.vms[g] = struct{}{}
+		}
 		return nil
 	}
 
-	// The function being invoked at this frame (caller) is the name for the previous
-	// frame (callee). For the last frame frame.File will be 0 and pt will be nil and funcName
-	// will be "main".  Lua is a real deal dynamic functional language, functions don't have
-	// inherent names they are just values.
 	var funcName string
 	if frame.File == C.LUAJIT_FFI_FUNC {
 		switch frame.Lineno & 7 {
@@ -431,32 +419,15 @@ func (l *luajitInstance) Symbolize(symbolReporter reporter.SymbolReporter, frame
 			funcName = "ff-pcall-hook"
 		}
 	} else {
-		ptaddr := libpf.Address(frame.File)
-		pc := uint32(frame.Lineno)
-		pt, err := l.getGCproto(ptaddr)
+		callerPT := libpf.Address(frame.Lineno)
+		pt, err := l.getGCproto(callerPT)
 		if err != nil {
 			return err
 		}
-		funcName = pt.getFunctionName(pc)
+		funcName = pt.getFunctionName(frame.LJCallerPC)
 	}
 
-	err := l.symbolizeFrame(symbolReporter, funcName, trace)
-
-	if frame.File == 0 {
-		logf("lj: end LuaJIT frame")
-		l.previousFrame = nil
-		// The BPF program will stash pointer to "G" when it sees a JIT frame w/o trace information
-		// which may fail to unwind, we use it to see if the traces for this VM have changed. When
-		// we reach a steady state where there's no new JIT activity this will always be 0.
-		g := libpf.Address(frame.Lineno)
-		if g != 0 {
-			l.mu.Lock()
-			defer l.mu.Unlock()
-			l.vms[g] = struct{}{}
-		}
-	} else {
-		l.previousFrame = frame
-	}
+	err := l.symbolizeFrame(symbolReporter, funcName, trace, libpf.Address(frame.File), frame.LJCalleePC)
 
 	return err
 }

interpreter/luajit/luajit_test.go

@@ -84,15 +84,14 @@ func TestIntegration(t *testing.T) {
 				"1.21.4.3-buster",
 				"1.25.3.2-bullseye",
 				"jammy",
+				"alpine",
 			} {
 				t.Run(tag, func(t *testing.T) {
 					image := openrestyBase + ":" + tag
 					ctx, cancel := context.WithCancel(context.Background())
 					t.Cleanup(cancel)
 
 					defer cancel()
-					r := &mockReporter{symbols: make(symbolMap)}
-					traceCh, trc := startTracer(ctx, t, r)
 
 					cont := startContainer(ctx, t, image)
 
@@ -106,13 +105,22 @@ func TestIntegration(t *testing.T) {
 					defer waitGroup.Wait()
 					makeRequests(ctx, t, &waitGroup, tc.resource, h, port)
 
+					r := &mockReporter{symbols: make(symbolMap)}
+					traceCh, trc := startTracer(ctx, t, r)
+
 					passes, fails, traces := 0, 0, 0
 				done:
 					for {
 						select {
 						case <-ctx.Done():
 							break done
 						case trace := <-traceCh:
+							st, err := cont.State(ctx)
+							require.NoError(t, err)
+							// See if PID is openresty
+							if int(trace.PID) != st.Pid {
+								continue
+							}
 							traces++
 							if validateTrace(t, trc, trace, tc.structure, r) {
 								passes++
@@ -151,18 +159,6 @@ func validateTrace(t *testing.T, trc *tracer.Tracer, trace *host.Trace,
 		return false
 	}
 
-	var zeroFileIdx, lastFrameIdx int
-	for i, t := range trace.Frames {
-		if t.Type == libpf.LuaJITFrame {
-			if t.File == 0 {
-				zeroFileIdx = i
-			}
-			lastFrameIdx = i
-		}
-	}
-
-	require.Equal(t, zeroFileIdx, lastFrameIdx)
-
 	// Finally convert it to flex all the proto parsing/remote code
 	ct := trc.TraceProcessor().ConvertTrace(trace)
 	require.NotNil(t, ct)

interpreter/types.go

@@ -140,9 +140,7 @@ type Instance interface {
 
 	// Symbolize requests symbolization of the given frame, and dispatches this symbolization
 	// to the collection agent. The frame's contents (frame type, file ID and line number)
-	// are appended to newTrace. This must be called on each from in order from
-	// top to bottom of stack to support functional languages which infer callee
-	// name from caller frame.
+	// are appended to newTrace.
 	Symbolize(symbolReporter reporter.SymbolReporter, frame *host.Frame,
 		trace *libpf.Trace) error
 

support/ebpf/errors.h

@@ -173,7 +173,10 @@ typedef enum ErrorCode {
   ERR_LUAJIT_FRAME_READ = 7002,
 
   // LuaJIT: context pointer validity check failed
-  ERR_LUAJIT_L_MISMATCH = 7003
+  ERR_LUAJIT_L_MISMATCH = 7003,
+
+  // LuaJIT: PC exceeds 24 bits
+  ERR_LUAJIT_INVALID_PC = 7004
 } ErrorCode;
 
 #endif // OPTI_ERRORS_H