Merge pull request #68 from parca-dev/lj-fixes

LuaJIT improved diagnostics and rsi/esi fix

Author: gnurizen

interpreter/luajit/extractor_x86.go

@@ -510,6 +510,8 @@ func sameReg(r1, r2 x86asm.Reg) bool {
 			return r2 == x86asm.RDX
 		case x86asm.EBX:
 			return r2 == x86asm.RBX
+		case x86asm.ESI:
+			return r2 == x86asm.RSI
 		default:
 			return false
 		}

interpreter/luajit/luajit.go

@@ -13,12 +13,12 @@ package luajit // import "go.opentelemetry.io/ebpf-profiler/interpreter/luajit"
 
 import (
 	"errors"
-	"fmt"
 	"path"
 	"strings"
 	"sync"
 	"unsafe"
 
+	log "github.com/sirupsen/logrus"
 	"go.opentelemetry.io/ebpf-profiler/host"
 	"go.opentelemetry.io/ebpf-profiler/interpreter"
 	"go.opentelemetry.io/ebpf-profiler/libpf"
@@ -285,6 +285,7 @@ func (l *luajitInstance) processVMs(ebpf interpreter.EbpfHandler, pid libpf.PID)
 		hash, traces, err := loadTraces(g+libpf.Address(l.g2Traces), l.rm)
 		if err != nil {
 			// if g is bad remove it
+			log.Warnf("LuaJIT instance (%v) deleted: %v", g, err)
 			badVMs = append(badVMs, g)
 			continue
 		}
@@ -301,6 +302,7 @@ func (l *luajitInstance) processVMs(ebpf interpreter.EbpfHandler, pid libpf.PID)
 		}
 
 		newPrefixes := []lpm.Prefix{}
+	traceLoop:
 		for i := range traces {
 			t := traces[i]
 			// Validate the trace
@@ -310,14 +312,16 @@ func (l *luajitInstance) processVMs(ebpf interpreter.EbpfHandler, pid libpf.PID)
 					foundRegion = true
 					end := t.mcode + uint64(t.szmcode)
 					if end > reg.Vaddr+reg.Length {
-						return fmt.Errorf("trace %v end goes beyond JIT region", t)
+						log.Errorf("trace %v end goes beyond JIT region, bad szmcode", t)
+						continue traceLoop
 					}
 					break
 				}
 			}
 
 			if !foundRegion {
-				return fmt.Errorf("trace %v not in a JIT region", t)
+				log.Errorf("trace %v not in a JIT region", t)
+				continue
 			}
 
 			stackDelta := uint64(t.spadjust) + uint64(cframeSizeJIT)
@@ -329,12 +333,13 @@ func (l *luajitInstance) processVMs(ebpf interpreter.EbpfHandler, pid libpf.PID)
 			}
 			p, err := l.addTrace(ebpf, pid, t, uint64(g), stackDelta)
 			if err != nil {
-				return err
+				log.Errorf("Error adding trace(%d): %v", t.traceno, err)
+				continue
 			}
 			newPrefixes = append(newPrefixes, p...)
 		}
 
-		logf("lj: new traces detected pid(%v) added: %d with %d prefixes and removed %d prefixes",
+		log.Infof("LuaJIT traces for pid(%v) added: %d with %d prefixes and removed %d prefixes",
 			pid, len(traces), len(newPrefixes), len(prefixes))
 
 		l.prefixesByG[g] = newPrefixes
@@ -417,8 +422,11 @@ func (l *luajitInstance) Symbolize(symbolReporter reporter.SymbolReporter, frame
 		g := libpf.Address(frame.Lineno)
 		if g != 0 {
 			unseen := l.addVM(g)
-			if unseen && l.ebpf.CoredumpTest() {
-				return interpreter.ErrLJRestart
+			if unseen {
+				log.Infof("New LuaJIT instance detected: %v", g)
+				if l.ebpf.CoredumpTest() {
+					return interpreter.ErrLJRestart
+				}
 			}
 		}
 		return nil

interpreter/luajit/offsets_test.go

@@ -165,15 +165,20 @@ func getLibFromImage(t *testing.T, name, platform, fullPath, target string) {
 }
 
 // spot testing
-func TestFile(t *testing.T) {
-	for _, target := range []string{
-		"./testdata/libluajit-5.1-jammy.so",
-		"./testdata/luajit-nixos"} {
-		if _, err := os.Stat(target); os.IsNotExist(err) {
+func TestFiles(t *testing.T) {
+	files, err := os.ReadDir("./testdata")
+	require.NoError(t, err)
+	for _, de := range files {
+		target := "./testdata/" + de.Name()
+		fi, err := os.Stat(target)
+		if err != nil || fi.IsDir() {
 			continue
 		}
 		ef, err := pfelf.Open(target)
-		require.NoError(t, err)
+		// Skip non-elf files
+		if err != nil {
+			continue
+		}
 		ljd := luajitData{}
 
 		// create stacktrace deltas to make sure we can find interp bounds
@@ -208,7 +213,7 @@ func TestFile(t *testing.T) {
 			require.Equal(t, uint64(du.Address), du2)
 		}
 
-		t.Logf("%+v, interp: %+v", ljd, interp)
+		t.Logf("%s: %+v, interp: %+v", target, ljd, interp)
 	}
 }
 

interpreter/luajit/trace.go

@@ -93,6 +93,7 @@ func loadTraces(tracesAddr libpf.Address, rm remotememory.RemoteMemory) (uint64,
 		if t.traceno > uint16(sztrace) {
 			return 0, nil, errors.New("invalid traceno")
 		}
+		logf("lj: added trace(%d) from %x", t.traceno, tracesAddr)
 		traces[t.traceno] = t
 	}
 	return h, traces, nil

support/types_def.go

@@ -110,6 +110,6 @@ const (
 )
 
 const (
-	CustomLabelMaxKeyLen	= C.CUSTOM_LABEL_MAX_KEY_LEN
-	CustomLabelMaxValLen	= C.CUSTOM_LABEL_MAX_VAL_LEN
+	CustomLabelMaxKeyLen = C.CUSTOM_LABEL_MAX_KEY_LEN
+	CustomLabelMaxValLen = C.CUSTOM_LABEL_MAX_VAL_LEN
 )