From 24a5cf9775ccec4c603e79974624cae0a3011e33 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Sun, 9 Feb 2025 12:02:33 +0100 Subject: [PATCH 1/3] fix example tests; apparently anonymous function names changed --- internal/main_test.go | 2 +- stack/context.go | 2 +- stack/context_test.go | 10 ++-- stack/example_test.go | 112 +++++++++++++++++++++--------------------- 4 files changed, 64 insertions(+), 62 deletions(-) diff --git a/internal/main_test.go b/internal/main_test.go index cd1fa77..1c38185 100644 --- a/internal/main_test.go +++ b/internal/main_test.go @@ -113,7 +113,7 @@ func TestProcessTwoSnapshots(t *testing.T) { "panic: 42\n\n" + "1: running\n" + " main main.go:93 panicint(0x2a)\n" + - " main main.go:315 glob..func9()\n" + + " main main.go:315 init.func9()\n" + " main main.go:76 main()\n" + "Yo\n") compareString(t, want, out.String()) diff --git a/stack/context.go b/stack/context.go index b67ef47..787f688 100644 --- a/stack/context.go +++ b/stack/context.go @@ -356,7 +356,7 @@ const ( // to: gotFileFunc gotFunc // Regexp: reCreated - // Signature: "created by main.glob..func4" + // Signature: "created by main.init.func4" // Goroutine creation line was found. // from: gotFileFunc // to: gotFileCreated diff --git a/stack/context_test.go b/stack/context_test.go index 704af9f..93006d6 100644 --- a/stack/context_test.go +++ b/stack/context_test.go @@ -1394,7 +1394,7 @@ func TestScanSnapshotSyntheticTwoSnapshots(t *testing.T) { 93, ), newCallLocal( - "main.glob..func9", + "main.init.func9", Args{}, pathJoin(ppDir, "main.go"), 315, @@ -1813,7 +1813,7 @@ func testPanicArgsElided(t *testing.T, s *Snapshot, b *bytes.Buffer, ppDir strin }, pathJoin(ppDir, "main.go"), 58), - newCallLocal("main.glob..func1", Args{}, pathJoin(ppDir, "main.go"), 134), + newCallLocal("main.init.func1", Args{}, pathJoin(ppDir, "main.go"), 134), newCallLocal("main.main", Args{}, pathJoin(ppDir, "main.go"), 340), }, }, @@ -1852,7 +1852,7 @@ func testPanicMismatched(t *testing.T, s *Snapshot, b *bytes.Buffer, ppDir strin Args{}, pathJoin(ppDir, "internal", "incorrect", "correct.go"), 7), - newCallLocal("main.glob..func20", Args{}, pathJoin(ppDir, "main.go"), 314), + newCallLocal("main.init.func20", Args{}, pathJoin(ppDir, "main.go"), 314), newCallLocal("main.main", Args{}, pathJoin(ppDir, "main.go"), 340), }, }, @@ -1986,7 +1986,7 @@ func testPanicStr(t *testing.T, s *Snapshot, b *bytes.Buffer, ppDir string) { }}}}, pathJoin(ppDir, "main.go"), 50), - newCallLocal("main.glob..func19", Args{}, pathJoin(ppDir, "main.go"), 307), + newCallLocal("main.init.func19", Args{}, pathJoin(ppDir, "main.go"), 307), newCallLocal("main.main", Args{}, pathJoin(ppDir, "main.go"), 340), }, }, @@ -2025,7 +2025,7 @@ func testPanicUTF8(t *testing.T, s *Snapshot, b *bytes.Buffer, ppDir string) { // Call in this situation. pathJoin(ppDir, "internal", "utf8", "utf8.go"), 10), - newCallLocal("main.glob..func21", Args{}, pathJoin(ppDir, "main.go"), 322), + newCallLocal("main.init.func21", Args{}, pathJoin(ppDir, "main.go"), 322), newCallLocal("main.main", Args{}, pathJoin(ppDir, "main.go"), 340), }, }, diff --git a/stack/example_test.go b/stack/example_test.go index 8c699dd..bf40ec9 100644 --- a/stack/example_test.go +++ b/stack/example_test.go @@ -222,9 +222,9 @@ func Example_httpHandlerMiddleware() { // Output: // recovered: "It happens" // Parsed stack: - // stack_test example_test.go:243 wrapPanic.func1.1() + // stack_test example_test.go:239 recoverPanic() // stack_test example_test.go:233 panickingHandler() - // stack_test example_test.go:293 wrapPanic.func1() + // stack_test example_test.go:295 Example_httpHandlerMiddleware.wrapPanic.func2() } // panickingHandler is an http.HandlerFunc that panics. @@ -233,63 +233,65 @@ func panickingHandler(w http.ResponseWriter, r *http.Request) { panic("It happens") } +func recoverPanic() { + if v := recover(); v != nil { + // Collect the stack and process it. + rawStack := append(debug.Stack(), '\n', '\n') + st, _, err := stack.ScanSnapshot(bytes.NewReader(rawStack), io.Discard, stack.DefaultOpts()) + + if err != nil || len(st.Goroutines) != 1 { + // Processing failed. Print out the raw stack. + fmt.Fprintf(os.Stdout, "recovered: %q\nStack processing failed: %v\nRaw stack:\n%s", v, err, rawStack) + return + } + + // Calculate alignment. + srcLen := 0 + pkgLen := 0 + for _, line := range st.Goroutines[0].Stack.Calls { + if l := len(fmt.Sprintf("%s:%d", line.SrcName, line.Line)); l > srcLen { + srcLen = l + } + if l := len(filepath.Base(line.Func.ImportPath)); l > pkgLen { + pkgLen = l + } + } + buf := bytes.Buffer{} + // Reduce memory allocation. + buf.Grow(len(st.Goroutines[0].Stack.Calls) * (40 + srcLen + pkgLen)) + for _, line := range st.Goroutines[0].Stack.Calls { + + // REMOVE: Skip the standard library in this test since it would + // make it Go version dependent. + if line.Location == stack.Stdlib { + continue + } + + // REMOVE: Not printing args here to make the test deterministic. + args := "" + //args := line.Args.String() + + fmt.Fprintf( + &buf, + " %-*s %-*s %s(%s)\n", + pkgLen, line.Func.DirName, srcLen, + fmt.Sprintf("%s:%d", line.SrcName, line.Line), + line.Func.Name, + args) + } + if st.Goroutines[0].Stack.Elided { + io.WriteString(&buf, " (...)\n") + } + // Print out the formatted stack. + fmt.Fprintf(os.Stdout, "recovered: %q\nParsed stack:\n%s", v, buf.String()) + } +} + // wrapPanic is a http.Handler middleware that traps panics and print it out to // os.Stdout. func wrapPanic(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - defer func() { - if v := recover(); v != nil { - // Collect the stack and process it. - rawStack := append(debug.Stack(), '\n', '\n') - st, _, err := stack.ScanSnapshot(bytes.NewReader(rawStack), io.Discard, stack.DefaultOpts()) - - if err != nil || len(st.Goroutines) != 1 { - // Processing failed. Print out the raw stack. - fmt.Fprintf(os.Stdout, "recovered: %q\nStack processing failed: %v\nRaw stack:\n%s", v, err, rawStack) - return - } - - // Calculate alignment. - srcLen := 0 - pkgLen := 0 - for _, line := range st.Goroutines[0].Stack.Calls { - if l := len(fmt.Sprintf("%s:%d", line.SrcName, line.Line)); l > srcLen { - srcLen = l - } - if l := len(filepath.Base(line.Func.ImportPath)); l > pkgLen { - pkgLen = l - } - } - buf := bytes.Buffer{} - // Reduce memory allocation. - buf.Grow(len(st.Goroutines[0].Stack.Calls) * (40 + srcLen + pkgLen)) - for _, line := range st.Goroutines[0].Stack.Calls { - - // REMOVE: Skip the standard library in this test since it would - // make it Go version dependent. - if line.Location == stack.Stdlib { - continue - } - - // REMOVE: Not printing args here to make the test deterministic. - args := "" - //args := line.Args.String() - - fmt.Fprintf( - &buf, - " %-*s %-*s %s(%s)\n", - pkgLen, line.Func.DirName, srcLen, - fmt.Sprintf("%s:%d", line.SrcName, line.Line), - line.Func.Name, - args) - } - if st.Goroutines[0].Stack.Elided { - io.WriteString(&buf, " (...)\n") - } - // Print out the formatted stack. - fmt.Fprintf(os.Stdout, "recovered: %q\nParsed stack:\n%s", v, buf.String()) - } - }() + defer recoverPanic() h.ServeHTTP(w, r) }) } From 94fe49e2897733f3acd7b05e5d4a4cd99304b4cf Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Sun, 9 Feb 2025 12:13:27 +0100 Subject: [PATCH 2/3] fix parsing go1.23 stack traces Go 1.21 changed how the runtime prints deep stacks: https://github.com/golang/go/commit/9eba17ff90963cdbbe47af887fb3152c0c4d1ebb fixes https://github.com/maruel/panicparse/issues/90 --- .github/workflows/test.yml | 8 ++++---- stack/context.go | 17 ++++++++++++++--- stack/context_test.go | 22 ++++++++++++---------- stack/source_test.go | 7 +++++-- stack/stack.go | 8 ++++++++ 5 files changed, 43 insertions(+), 19 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 131731a..2c7a86e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,7 +20,7 @@ jobs: matrix: os: [ubuntu-latest, macos-latest, windows-latest] # Do not forget to bump every 6 months! - gover: ["1.20"] + gover: ["1.23"] env: PYTHONDONTWRITEBYTECODE: x steps: @@ -92,7 +92,7 @@ jobs: # Windows. os: [ubuntu-latest, macos-latest, windows-latest] # Do not forget to bump every 6 months! - gover: ["1.20"] + gover: ["1.23"] env: PYTHONDONTWRITEBYTECODE: x steps: @@ -109,7 +109,7 @@ jobs: go install github.com/gordonklaus/ineffassign@latest go install github.com/securego/gosec/v2/cmd/gosec@v2.18.2 go install golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow@v0.24.0 - go install honnef.co/go/tools/cmd/staticcheck@v0.4.7 + go install honnef.co/go/tools/cmd/staticcheck@v0.5.1 - name: 'go install necessary tools (ubuntu)' if: always() && matrix.os == 'ubuntu-latest' run: | @@ -265,7 +265,7 @@ jobs: matrix: os: [ubuntu-latest] # Do not forget to bump every 6 months! - gover: ["1.20"] + gover: ["1.23"] permissions: security-events: write steps: diff --git a/stack/context.go b/stack/context.go index 787f688..2edc1e4 100644 --- a/stack/context.go +++ b/stack/context.go @@ -252,7 +252,6 @@ const pathSeparator = string(filepath.Separator) var ( lockedToThread = []byte("locked to thread") - framesElided = []byte("...additional frames elided...") // gotRaceHeader1, done raceHeaderFooter = []byte("==================") // gotRaceHeader2 @@ -270,7 +269,7 @@ var ( // These are effectively constants. var ( // gotRoutineHeader - reRoutineHeader = regexp.MustCompile("^([ \t]*)goroutine (\\d+) \\[([^\\]]+)\\]\\:$") + reRoutineHeader = regexp.MustCompile("^([ \t]*)goroutine (\\d+)(?: gp=[^ ]+ m=[^ ]+(?: mp=[^ ]+)?)? \\[([^\\]]+)\\]\\:$") reMinutes = regexp.MustCompile(`^(\d+) minutes$`) // gotUnavail @@ -450,6 +449,18 @@ type scanningState struct { goroutineIndex int } +func isFramesElidedLine(line []byte) bool { + // before go1.21: + // ...additional frames elided... + // + // go1.21 and newer: + // print("...", elide, " frames elided...\n") + framesElided := []byte("...additional frames elided...") + return bytes.Equal(line, framesElided) || + bytes.HasPrefix(line, []byte("...")) && + bytes.HasSuffix(line, []byte(" frames elided...")) +} + // scan scans one line, updates goroutines and move to the next state. // // Returns true if the line was processed and thus should not be printed out. @@ -605,7 +616,7 @@ func (s *scanningState) scan(line []byte) (bool, error) { s.state = gotCreated return true, nil } - if bytes.Equal(trimmed, framesElided) { + if isFramesElidedLine(trimmed) { cur.Stack.Elided = true // TODO(maruel): New state. return true, nil diff --git a/stack/context_test.go b/stack/context_test.go index 93006d6..1da5d03 100644 --- a/stack/context_test.go +++ b/stack/context_test.go @@ -2350,7 +2350,7 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb t.Fatal("expected Locked") } // This is a change detector on internal/main.go. - want := Stack{Calls: []Call{newCallLocal("main.main", Args{}, pathJoin(pwebDir, "main.go"), 145)}} + want := Stack{Calls: []Call{newCallLocal("main.main in goroutine 1", Args{}, pathJoin(pwebDir, "main.go"), 145)}} compareStacks(t, &b.Signature.CreatedBy, &want) for i := range b.Signature.Stack.Calls { if strings.HasPrefix(b.Signature.Stack.Calls[i].ImportPath, "github.com/mattn/go-colorable") { @@ -2364,7 +2364,7 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb { want := Stack{ Calls: []Call{ - newCallLocal("main.main", Args{}, pathJoin(pwebDir, "main.go"), 63), + newCallLocal("main.main in goroutine 1", Args{}, pathJoin(pwebDir, "main.go"), 63), }, } zapStacks(t, &want, &b.CreatedBy) @@ -2374,8 +2374,8 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb t.Fatalf("expected 1 goroutine for the signature, got %d", l) } if runtime.GOOS == "windows" { - if l := len(b.Stack.Calls); l != 5 { - t.Fatalf("expected %d calls, got %d", 5, l) + if got, want := len(b.Stack.Calls), 4; got != want { + t.Fatalf("expected %d calls, got %d", want, got) } if s := b.Stack.Calls[0].RelSrcPath; s != "runtime/syscall_windows.go" { t.Fatalf("expected %q file, got %q", "runtime/syscall_windows.go", s) @@ -2396,6 +2396,7 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb } } // Process the golang.org/x/sys call specifically. + callIdx := 1 path := "golang.org/x/sys/unix" fn := "Nanosleep" mainOS := "main_unix.go" @@ -2409,21 +2410,22 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb } if runtime.GOOS == "windows" { // This changes across Go version, this check is super fragile. + callIdx = 0 path = "syscall" fn = "Syscall" mainOS = "main_windows.go" prefix = "runtime/syscall_windows.go" expectVersion = false } - if b.Stack.Calls[1].Func.ImportPath != path || b.Stack.Calls[1].Func.Name != fn { - t.Fatalf("expected %q & %q, got %#v", path, fn, b.Stack.Calls[1].Func) + if b.Stack.Calls[callIdx].Func.ImportPath != path || b.Stack.Calls[callIdx].Func.Name != fn { + t.Fatalf("expected %q & %q, got %#v", path, fn, b.Stack.Calls[callIdx].Func) } - if !strings.HasPrefix(b.Stack.Calls[1].RelSrcPath, prefix) { - t.Fatalf("expected %q, got %q", prefix, b.Stack.Calls[1].RelSrcPath) + if !strings.HasPrefix(b.Stack.Calls[callIdx].RelSrcPath, prefix) { + t.Fatalf("expected %q, got %q", prefix, b.Stack.Calls[callIdx].RelSrcPath) } if expectVersion { // Assert that it's using v0.1.0 format. - ver := strings.SplitN(b.Stack.Calls[1].RelSrcPath[len(prefix):], "/", 2)[0] + ver := strings.SplitN(b.Stack.Calls[callIdx].RelSrcPath[len(prefix):], "/", 2)[0] re := regexp.MustCompile(`^v\d+\.\d+\.\d+$`) if !re.MatchString(ver) { t.Fatalf("unexpected version string %q", ver) @@ -2438,7 +2440,7 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb pathJoin(pwebDir, "main.go"), 65), } - got := b.Stack.Calls[2:] + got := b.Stack.Calls[callIdx+1:] if ver := internaltest.GetGoMinorVersion(); (ver > 0 && ver < 18 && !is64Bit) || runtime.GOOS == "windows" { // TODO(maruel): Fix check on Windows. // On go1.17 on 32 bits this is failing but not on go1.18, so only diff --git a/stack/source_test.go b/stack/source_test.go index 35e73df..df1461b 100644 --- a/stack/source_test.go +++ b/stack/source_test.go @@ -494,9 +494,12 @@ func TestAugment(t *testing.T) { "main.f", Args{ Values: []Arg{{IsAggregate: true, Fields: Args{ - Values: []Arg{{Value: pointer, IsPtr: true}, {Value: 3}}, + Values: []Arg{ + {Value: pointer, IsPtr: true}, + {Value: pointer, IsPtr: true}, + }, }}}, - Processed: []string{"error{0x2fffffff, 0x3}"}, + Processed: []string{"error{0x2fffffff, 0x2fffffff}"}, }, "/root/main.go", 7), diff --git a/stack/stack.go b/stack/stack.go index b98af8d..4d936bd 100644 --- a/stack/stack.go +++ b/stack/stack.go @@ -79,6 +79,14 @@ func (f *Func) Init(raw string) error { f.ImportPath = f.Complete[:endPkg] } f.Name = f.Complete[endPkg+1:] + if idx := strings.LastIndexByte(f.Name, ' '); idx > -1 { + cut := f.Name[:idx] + // TODO(go1.20): switch to strings.CutSuffix + const inGoroutineSuffix = " in goroutine" + if strings.HasSuffix(cut, inGoroutineSuffix) { + f.Name = strings.TrimSuffix(cut, inGoroutineSuffix) + } + } f.DirName = f.ImportPath if i := strings.LastIndexByte(f.DirName, '/'); i != -1 { f.DirName = f.DirName[i+1:] From aa2551a8878eac2e781fd97cb2a19c2bb8b16d4a Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 10 Feb 2025 22:27:47 +0100 Subject: [PATCH 3/3] GitHub Actions: replace broken 1.17 workflow with 1.23 for now (With 1.17.13, the tests would not even compile.) --- .github/workflows/test.yml | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2c7a86e..e6a8812 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -230,29 +230,19 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest] - # https://github.com/golang/go/issues/55078 - # golang.org/x/sys/unix broke on Go versions before 1.17. Not worth - # fixing. - gover: ['1.17.13'] + # TODO: switch to an older Go version once tests are made compatible + gover: ['1.23'] env: PYTHONDONTWRITEBYTECODE: x - GOPATH: ${{github.workspace}} - GO111MODULE: off steps: - name: Turn off git core.autocrlf if: matrix.os == 'windows-latest' run: git config --global core.autocrlf false - uses: actions/checkout@v4 - with: - path: src/github.com/maruel/panicparse - uses: actions/setup-go@v5 with: go-version: "=${{matrix.gover}}" - - name: 'Check: go get -d -t' - working-directory: src/github.com/maruel/panicparse - run: go get -d -t ./... - name: 'Check: go test' - working-directory: src/github.com/maruel/panicparse run: go test -timeout=120s -bench=. -benchtime=1x ./...