Skip to content

Commit 9264a04

Browse files
stapelbergstapelberg
authored
Fix panicparse for Go 1.23 (#92)
* 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.)
1 parent 7d1abed commit 9264a04

File tree

7 files changed

+109
-93
lines changed

7 files changed

+109
-93
lines changed

‎.github/workflows/test.yml‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ jobs:
2020
matrix:
2121
os: [ubuntu-latest, macos-latest, windows-latest]
2222
# Do not forget to bump every 6 months!
23-
gover: ["1.20"]
23+
gover: ["1.23"]
2424
env:
2525
PYTHONDONTWRITEBYTECODE: x
2626
steps:
@@ -92,7 +92,7 @@ jobs:
9292
# Windows.
9393
os: [ubuntu-latest, macos-latest, windows-latest]
9494
# Do not forget to bump every 6 months!
95-
gover: ["1.20"]
95+
gover: ["1.23"]
9696
env:
9797
PYTHONDONTWRITEBYTECODE: x
9898
steps:
@@ -109,7 +109,7 @@ jobs:
109109
go install github.com/gordonklaus/ineffassign@latest
110110
go install github.com/securego/gosec/v2/cmd/gosec@v2.18.2
111111
go install golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow@v0.24.0
112-
go install honnef.co/go/tools/cmd/staticcheck@v0.4.7
112+
go install honnef.co/go/tools/cmd/staticcheck@v0.5.1
113113
- name: 'go install necessary tools (ubuntu)'
114114
if: always() && matrix.os == 'ubuntu-latest'
115115
run: |
@@ -230,29 +230,19 @@ jobs:
230230
fail-fast: false
231231
matrix:
232232
os: [ubuntu-latest]
233-
# https://github.com/golang/go/issues/55078
234-
# golang.org/x/sys/unix broke on Go versions before 1.17. Not worth
235-
# fixing.
236-
gover: ['1.17.13']
233+
# TODO: switch to an older Go version once tests are made compatible
234+
gover: ['1.23']
237235
env:
238236
PYTHONDONTWRITEBYTECODE: x
239-
GOPATH: ${{github.workspace}}
240-
GO111MODULE: off
241237
steps:
242238
- name: Turn off git core.autocrlf
243239
if: matrix.os == 'windows-latest'
244240
run: git config --global core.autocrlf false
245241
- uses: actions/checkout@v4
246-
with:
247-
path: src/github.com/maruel/panicparse
248242
- uses: actions/setup-go@v5
249243
with:
250244
go-version: "=${{matrix.gover}}"
251-
- name: 'Check: go get -d -t'
252-
working-directory: src/github.com/maruel/panicparse
253-
run: go get -d -t ./...
254245
- name: 'Check: go test'
255-
working-directory: src/github.com/maruel/panicparse
256246
run: go test -timeout=120s -bench=. -benchtime=1x ./...
257247

258248

@@ -265,7 +255,7 @@ jobs:
265255
matrix:
266256
os: [ubuntu-latest]
267257
# Do not forget to bump every 6 months!
268-
gover: ["1.20"]
258+
gover: ["1.23"]
269259
permissions:
270260
security-events: write
271261
steps:

‎internal/main_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func TestProcessTwoSnapshots(t *testing.T) {
113113
"panic: 42\n\n" +
114114
"1: running\n" +
115115
" main main.go:93 panicint(0x2a)\n" +
116-
" main main.go:315 glob..func9()\n" +
116+
" main main.go:315 init.func9()\n" +
117117
" main main.go:76 main()\n" +
118118
"Yo\n")
119119
compareString(t, want, out.String())

‎stack/context.go‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,6 @@ const pathSeparator = string(filepath.Separator)
252252

253253
var (
254254
lockedToThread = []byte("locked to thread")
255-
framesElided = []byte("...additional frames elided...")
256255
// gotRaceHeader1, done
257256
raceHeaderFooter = []byte("==================")
258257
// gotRaceHeader2
@@ -270,7 +269,7 @@ var (
270269
// These are effectively constants.
271270
var (
272271
// gotRoutineHeader
273-
reRoutineHeader = regexp.MustCompile("^([ \t]*)goroutine (\\d+) \\[([^\\]]+)\\]\\:$")
272+
reRoutineHeader = regexp.MustCompile("^([ \t]*)goroutine (\\d+)(?: gp=[^ ]+ m=[^ ]+(?: mp=[^ ]+)?)? \\[([^\\]]+)\\]\\:$")
274273
reMinutes = regexp.MustCompile(`^(\d+) minutes$`)
275274

276275
// gotUnavail
@@ -356,7 +355,7 @@ const (
356355
// to: gotFileFunc
357356
gotFunc
358357
// Regexp: reCreated
359-
// Signature: "created by main.glob..func4"
358+
// Signature: "created by main.init.func4"
360359
// Goroutine creation line was found.
361360
// from: gotFileFunc
362361
// to: gotFileCreated
@@ -450,6 +449,18 @@ type scanningState struct {
450449
goroutineIndex int
451450
}
452451

452+
func isFramesElidedLine(line []byte) bool {
453+
// before go1.21:
454+
// ...additional frames elided...
455+
//
456+
// go1.21 and newer:
457+
// print("...", elide, " frames elided...\n")
458+
framesElided := []byte("...additional frames elided...")
459+
return bytes.Equal(line, framesElided) ||
460+
bytes.HasPrefix(line, []byte("...")) &&
461+
bytes.HasSuffix(line, []byte(" frames elided..."))
462+
}
463+
453464
// scan scans one line, updates goroutines and move to the next state.
454465
//
455466
// 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) {
605616
s.state = gotCreated
606617
return true, nil
607618
}
608-
if bytes.Equal(trimmed, framesElided) {
619+
if isFramesElidedLine(trimmed) {
609620
cur.Stack.Elided = true
610621
// TODO(maruel): New state.
611622
return true, nil

‎stack/context_test.go‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1394,7 +1394,7 @@ func TestScanSnapshotSyntheticTwoSnapshots(t *testing.T) {
13941394
93,
13951395
),
13961396
newCallLocal(
1397-
"main.glob..func9",
1397+
"main.init.func9",
13981398
Args{},
13991399
pathJoin(ppDir, "main.go"),
14001400
315,
@@ -1813,7 +1813,7 @@ func testPanicArgsElided(t *testing.T, s *Snapshot, b *bytes.Buffer, ppDir strin
18131813
},
18141814
pathJoin(ppDir, "main.go"),
18151815
58),
1816-
newCallLocal("main.glob..func1", Args{}, pathJoin(ppDir, "main.go"), 134),
1816+
newCallLocal("main.init.func1", Args{}, pathJoin(ppDir, "main.go"), 134),
18171817
newCallLocal("main.main", Args{}, pathJoin(ppDir, "main.go"), 340),
18181818
},
18191819
},
@@ -1852,7 +1852,7 @@ func testPanicMismatched(t *testing.T, s *Snapshot, b *bytes.Buffer, ppDir strin
18521852
Args{},
18531853
pathJoin(ppDir, "internal", "incorrect", "correct.go"),
18541854
7),
1855-
newCallLocal("main.glob..func20", Args{}, pathJoin(ppDir, "main.go"), 314),
1855+
newCallLocal("main.init.func20", Args{}, pathJoin(ppDir, "main.go"), 314),
18561856
newCallLocal("main.main", Args{}, pathJoin(ppDir, "main.go"), 340),
18571857
},
18581858
},
@@ -1986,7 +1986,7 @@ func testPanicStr(t *testing.T, s *Snapshot, b *bytes.Buffer, ppDir string) {
19861986
}}}},
19871987
pathJoin(ppDir, "main.go"),
19881988
50),
1989-
newCallLocal("main.glob..func19", Args{}, pathJoin(ppDir, "main.go"), 307),
1989+
newCallLocal("main.init.func19", Args{}, pathJoin(ppDir, "main.go"), 307),
19901990
newCallLocal("main.main", Args{}, pathJoin(ppDir, "main.go"), 340),
19911991
},
19921992
},
@@ -2025,7 +2025,7 @@ func testPanicUTF8(t *testing.T, s *Snapshot, b *bytes.Buffer, ppDir string) {
20252025
// Call in this situation.
20262026
pathJoin(ppDir, "internal", "utf8", "utf8.go"),
20272027
10),
2028-
newCallLocal("main.glob..func21", Args{}, pathJoin(ppDir, "main.go"), 322),
2028+
newCallLocal("main.init.func21", Args{}, pathJoin(ppDir, "main.go"), 322),
20292029
newCallLocal("main.main", Args{}, pathJoin(ppDir, "main.go"), 340),
20302030
},
20312031
},
@@ -2350,7 +2350,7 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb
23502350
t.Fatal("expected Locked")
23512351
}
23522352
// This is a change detector on internal/main.go.
2353-
want := Stack{Calls: []Call{newCallLocal("main.main", Args{}, pathJoin(pwebDir, "main.go"), 145)}}
2353+
want := Stack{Calls: []Call{newCallLocal("main.main in goroutine 1", Args{}, pathJoin(pwebDir, "main.go"), 145)}}
23542354
compareStacks(t, &b.Signature.CreatedBy, &want)
23552355
for i := range b.Signature.Stack.Calls {
23562356
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
23642364
{
23652365
want := Stack{
23662366
Calls: []Call{
2367-
newCallLocal("main.main", Args{}, pathJoin(pwebDir, "main.go"), 63),
2367+
newCallLocal("main.main in goroutine 1", Args{}, pathJoin(pwebDir, "main.go"), 63),
23682368
},
23692369
}
23702370
zapStacks(t, &want, &b.CreatedBy)
@@ -2374,8 +2374,8 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb
23742374
t.Fatalf("expected 1 goroutine for the signature, got %d", l)
23752375
}
23762376
if runtime.GOOS == "windows" {
2377-
if l := len(b.Stack.Calls); l != 5 {
2378-
t.Fatalf("expected %d calls, got %d", 5, l)
2377+
if got, want := len(b.Stack.Calls), 4; got != want {
2378+
t.Fatalf("expected %d calls, got %d", want, got)
23792379
}
23802380
if s := b.Stack.Calls[0].RelSrcPath; s != "runtime/syscall_windows.go" {
23812381
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
23962396
}
23972397
}
23982398
// Process the golang.org/x/sys call specifically.
2399+
callIdx := 1
23992400
path := "golang.org/x/sys/unix"
24002401
fn := "Nanosleep"
24012402
mainOS := "main_unix.go"
@@ -2409,21 +2410,22 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb
24092410
}
24102411
if runtime.GOOS == "windows" {
24112412
// This changes across Go version, this check is super fragile.
2413+
callIdx = 0
24122414
path = "syscall"
24132415
fn = "Syscall"
24142416
mainOS = "main_windows.go"
24152417
prefix = "runtime/syscall_windows.go"
24162418
expectVersion = false
24172419
}
2418-
if b.Stack.Calls[1].Func.ImportPath != path || b.Stack.Calls[1].Func.Name != fn {
2419-
t.Fatalf("expected %q & %q, got %#v", path, fn, b.Stack.Calls[1].Func)
2420+
if b.Stack.Calls[callIdx].Func.ImportPath != path || b.Stack.Calls[callIdx].Func.Name != fn {
2421+
t.Fatalf("expected %q & %q, got %#v", path, fn, b.Stack.Calls[callIdx].Func)
24202422
}
2421-
if !strings.HasPrefix(b.Stack.Calls[1].RelSrcPath, prefix) {
2422-
t.Fatalf("expected %q, got %q", prefix, b.Stack.Calls[1].RelSrcPath)
2423+
if !strings.HasPrefix(b.Stack.Calls[callIdx].RelSrcPath, prefix) {
2424+
t.Fatalf("expected %q, got %q", prefix, b.Stack.Calls[callIdx].RelSrcPath)
24232425
}
24242426
if expectVersion {
24252427
// Assert that it's using v0.1.0 format.
2426-
ver := strings.SplitN(b.Stack.Calls[1].RelSrcPath[len(prefix):], "/", 2)[0]
2428+
ver := strings.SplitN(b.Stack.Calls[callIdx].RelSrcPath[len(prefix):], "/", 2)[0]
24272429
re := regexp.MustCompile(`^v\d+\.\d+\.\d+$`)
24282430
if !re.MatchString(ver) {
24292431
t.Fatalf("unexpected version string %q", ver)
@@ -2438,7 +2440,7 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb
24382440
pathJoin(pwebDir, "main.go"),
24392441
65),
24402442
}
2441-
got := b.Stack.Calls[2:]
2443+
got := b.Stack.Calls[callIdx+1:]
24422444
if ver := internaltest.GetGoMinorVersion(); (ver > 0 && ver < 18 && !is64Bit) || runtime.GOOS == "windows" {
24432445
// TODO(maruel): Fix check on Windows.
24442446
// On go1.17 on 32 bits this is failing but not on go1.18, so only

‎stack/example_test.go‎

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,9 @@ func Example_httpHandlerMiddleware() {
222222
// Output:
223223
// recovered: "It happens"
224224
// Parsed stack:
225-
// stack_test example_test.go:243 wrapPanic.func1.1(<args>)
225+
// stack_test example_test.go:239 recoverPanic(<args>)
226226
// stack_test example_test.go:233 panickingHandler(<args>)
227-
// stack_test example_test.go:293 wrapPanic.func1(<args>)
227+
// stack_test example_test.go:295 Example_httpHandlerMiddleware.wrapPanic.func2(<args>)
228228
}
229229

230230
// panickingHandler is an http.HandlerFunc that panics.
@@ -233,63 +233,65 @@ func panickingHandler(w http.ResponseWriter, r *http.Request) {
233233
panic("It happens")
234234
}
235235

236+
func recoverPanic() {
237+
if v := recover(); v != nil {
238+
// Collect the stack and process it.
239+
rawStack := append(debug.Stack(), '\n', '\n')
240+
st, _, err := stack.ScanSnapshot(bytes.NewReader(rawStack), io.Discard, stack.DefaultOpts())
241+
242+
if err != nil || len(st.Goroutines) != 1 {
243+
// Processing failed. Print out the raw stack.
244+
fmt.Fprintf(os.Stdout, "recovered: %q\nStack processing failed: %v\nRaw stack:\n%s", v, err, rawStack)
245+
return
246+
}
247+
248+
// Calculate alignment.
249+
srcLen := 0
250+
pkgLen := 0
251+
for _, line := range st.Goroutines[0].Stack.Calls {
252+
if l := len(fmt.Sprintf("%s:%d", line.SrcName, line.Line)); l > srcLen {
253+
srcLen = l
254+
}
255+
if l := len(filepath.Base(line.Func.ImportPath)); l > pkgLen {
256+
pkgLen = l
257+
}
258+
}
259+
buf := bytes.Buffer{}
260+
// Reduce memory allocation.
261+
buf.Grow(len(st.Goroutines[0].Stack.Calls) * (40 + srcLen + pkgLen))
262+
for _, line := range st.Goroutines[0].Stack.Calls {
263+
264+
// REMOVE: Skip the standard library in this test since it would
265+
// make it Go version dependent.
266+
if line.Location == stack.Stdlib {
267+
continue
268+
}
269+
270+
// REMOVE: Not printing args here to make the test deterministic.
271+
args := "<args>"
272+
//args := line.Args.String()
273+
274+
fmt.Fprintf(
275+
&buf,
276+
" %-*s %-*s %s(%s)\n",
277+
pkgLen, line.Func.DirName, srcLen,
278+
fmt.Sprintf("%s:%d", line.SrcName, line.Line),
279+
line.Func.Name,
280+
args)
281+
}
282+
if st.Goroutines[0].Stack.Elided {
283+
io.WriteString(&buf, " (...)\n")
284+
}
285+
// Print out the formatted stack.
286+
fmt.Fprintf(os.Stdout, "recovered: %q\nParsed stack:\n%s", v, buf.String())
287+
}
288+
}
289+
236290
// wrapPanic is a http.Handler middleware that traps panics and print it out to
237291
// os.Stdout.
238292
func wrapPanic(h http.Handler) http.Handler {
239293
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
240-
defer func() {
241-
if v := recover(); v != nil {
242-
// Collect the stack and process it.
243-
rawStack := append(debug.Stack(), '\n', '\n')
244-
st, _, err := stack.ScanSnapshot(bytes.NewReader(rawStack), io.Discard, stack.DefaultOpts())
245-
246-
if err != nil || len(st.Goroutines) != 1 {
247-
// Processing failed. Print out the raw stack.
248-
fmt.Fprintf(os.Stdout, "recovered: %q\nStack processing failed: %v\nRaw stack:\n%s", v, err, rawStack)
249-
return
250-
}
251-
252-
// Calculate alignment.
253-
srcLen := 0
254-
pkgLen := 0
255-
for _, line := range st.Goroutines[0].Stack.Calls {
256-
if l := len(fmt.Sprintf("%s:%d", line.SrcName, line.Line)); l > srcLen {
257-
srcLen = l
258-
}
259-
if l := len(filepath.Base(line.Func.ImportPath)); l > pkgLen {
260-
pkgLen = l
261-
}
262-
}
263-
buf := bytes.Buffer{}
264-
// Reduce memory allocation.
265-
buf.Grow(len(st.Goroutines[0].Stack.Calls) * (40 + srcLen + pkgLen))
266-
for _, line := range st.Goroutines[0].Stack.Calls {
267-
268-
// REMOVE: Skip the standard library in this test since it would
269-
// make it Go version dependent.
270-
if line.Location == stack.Stdlib {
271-
continue
272-
}
273-
274-
// REMOVE: Not printing args here to make the test deterministic.
275-
args := "<args>"
276-
//args := line.Args.String()
277-
278-
fmt.Fprintf(
279-
&buf,
280-
" %-*s %-*s %s(%s)\n",
281-
pkgLen, line.Func.DirName, srcLen,
282-
fmt.Sprintf("%s:%d", line.SrcName, line.Line),
283-
line.Func.Name,
284-
args)
285-
}
286-
if st.Goroutines[0].Stack.Elided {
287-
io.WriteString(&buf, " (...)\n")
288-
}
289-
// Print out the formatted stack.
290-
fmt.Fprintf(os.Stdout, "recovered: %q\nParsed stack:\n%s", v, buf.String())
291-
}
292-
}()
294+
defer recoverPanic()
293295
h.ServeHTTP(w, r)
294296
})
295297
}

‎stack/source_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,9 +494,12 @@ func TestAugment(t *testing.T) {
494494
"main.f",
495495
Args{
496496
Values: []Arg{{IsAggregate: true, Fields: Args{
497-
Values: []Arg{{Value: pointer, IsPtr: true}, {Value: 3}},
497+
Values: []Arg{
498+
{Value: pointer, IsPtr: true},
499+
{Value: pointer, IsPtr: true},
500+
},
498501
}}},
499-
Processed: []string{"error{0x2fffffff, 0x3}"},
502+
Processed: []string{"error{0x2fffffff, 0x2fffffff}"},
500503
},
501504
"/root/main.go",
502505
7),

0 commit comments

Comments
 (0)