Skip to content

Commit

Permalink
Fix panicparse for Go 1.23 (#92)
Browse files Browse the repository at this point in the history 
* fix example tests; apparently anonymous function names changed

* fix parsing go1.23 stack traces

Go 1.21 changed how the runtime prints deep stacks:
golang/go@9eba17f

fixes #90

* GitHub Actions: replace broken 1.17 workflow with 1.23 for now

(With 1.17.13, the tests would not even compile.)
  • Loading branch information
@stapelberg
stapelberg authored Feb 11, 2025
1 parent 7d1abed commit 9264a04
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 93 deletions.
22 changes: 6 additions & 16 deletions .github/workflows/test.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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: |
Expand Down Expand Up @@ -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 ./...


Expand All @@ -265,7 +255,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:
Expand Down
2 changes: 1 addition & 1 deletion internal/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
19 changes: 15 additions & 4 deletions stack/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -356,7 +355,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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
32 changes: 17 additions & 15 deletions stack/context_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1394,7 +1394,7 @@ func TestScanSnapshotSyntheticTwoSnapshots(t *testing.T) {
93,
),
newCallLocal(
"main.glob..func9",
"main.init.func9",
Args{},
pathJoin(ppDir, "main.go"),
315,
Expand Down Expand Up @@ -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),
},
},
Expand Down Expand Up @@ -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),
},
},
Expand Down Expand Up @@ -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),
},
},
Expand Down Expand Up @@ -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),
},
},
Expand Down Expand Up @@ -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") {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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"
Expand All @@ -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)
Expand All @@ -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
Expand Down
112 changes: 57 additions & 55 deletions stack/example_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,9 @@ func Example_httpHandlerMiddleware() {
// Output:
// recovered: "It happens"
// Parsed stack:
// stack_test example_test.go:243 wrapPanic.func1.1(<args>)
// stack_test example_test.go:239 recoverPanic(<args>)
// stack_test example_test.go:233 panickingHandler(<args>)
// stack_test example_test.go:293 wrapPanic.func1(<args>)
// stack_test example_test.go:295 Example_httpHandlerMiddleware.wrapPanic.func2(<args>)
}

// panickingHandler is an http.HandlerFunc that panics.
Expand All @@ -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>"
//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>"
//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)
})
}
7 changes: 5 additions & 2 deletions stack/source_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Loading

0 comments on commit 9264a04

Please sign in to comment.