From f0fe5f931e9731fe6901db43275b63e489ff8a86 Mon Sep 17 00:00:00 2001 From: Tommy Reilly Date: Wed, 30 Apr 2025 09:49:24 -0400 Subject: [PATCH 1/3] Add some more logging to and make processVMs more resilient Don't stop completely if processVMs sees an error, instead log it and try additional traces and other vm instances. --- interpreter/luajit/luajit.go | 22 +++++++++++++++------- interpreter/luajit/trace.go | 1 + support/types_def.go | 4 ++-- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/interpreter/luajit/luajit.go b/interpreter/luajit/luajit.go index cb8fd60f7..2734a3afe 100644 --- a/interpreter/luajit/luajit.go +++ b/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 diff --git a/interpreter/luajit/trace.go b/interpreter/luajit/trace.go index a0b91dfbb..84f0fc075 100644 --- a/interpreter/luajit/trace.go +++ b/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 diff --git a/support/types_def.go b/support/types_def.go index cc40065f6..9127a0322 100644 --- a/support/types_def.go +++ b/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 ) From 8afb398c725b01a5055f6283c9bc5334302e71e1 Mon Sep 17 00:00:00 2001 From: Tommy Reilly Date: Wed, 30 Apr 2025 14:48:25 -0400 Subject: [PATCH 2/3] Allow esi/rsi to be used interchangably --- interpreter/luajit/extractor_x86.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interpreter/luajit/extractor_x86.go b/interpreter/luajit/extractor_x86.go index 355c0afc1..206be827d 100644 --- a/interpreter/luajit/extractor_x86.go +++ b/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 } From f037bd698371a639e7386e235c4e5470ddf7b0cf Mon Sep 17 00:00:00 2001 From: Tommy Reilly Date: Wed, 30 Apr 2025 14:48:54 -0400 Subject: [PATCH 3/3] Make TestFiles more useful --- interpreter/luajit/offsets_test.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/interpreter/luajit/offsets_test.go b/interpreter/luajit/offsets_test.go index d7aee6603..412b0eeae 100644 --- a/interpreter/luajit/offsets_test.go +++ b/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) } }