Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

testing: fuzz funcs that panic print redundant stack frames #48885

Open
mvdan opened this issue Oct 9, 2021 · 1 comment
Open

testing: fuzz funcs that panic print redundant stack frames #48885

mvdan opened this issue Oct 9, 2021 · 1 comment
Labels
fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@mvdan
Copy link
Member

mvdan commented Oct 9, 2021 

$ go version
go version devel go1.18-7fcf9a1e58 Fri Oct 8 19:41:16 2021 +0000 linux/amd64
$ cat fuzz_test.go 
package main

import (
	"testing"
)

func FuzzFoo(f *testing.F) {
	f.Fuzz(func(t *testing.T, n uint8) {
		if n%3 == 0 {
			panic("foo")
		}
	})
}
$ rm -rf testdata/fuzz && rm -rf $(go env GOCACHE)/fuzz
$ go test -fuzz=.
warning: starting with empty corpus
fuzz: elapsed: 0s, execs: 0 (0/sec), new interesting: 0 (total: 0)
fuzz: elapsed: 0s, execs: 3 (39/sec), new interesting: 0 (total: 0)
--- FAIL: FuzzFoo (0.08s)
        --- FAIL: FuzzFoo (0.00s)
            testing.go:1299: panic: foo
                goroutine 31 [running]:
                runtime/debug.Stack()
                	/home/mvdan/tip/src/runtime/debug/stack.go:24 +0x90
                testing.tRunner.func1()
                	/home/mvdan/tip/src/testing/testing.go:1299 +0x5a5
                panic({0x58dc40, 0x5f47f0})
                	/home/mvdan/tip/src/runtime/panic.go:814 +0x207
                test.FuzzFoo.func1(0x0, 0xd9)
                	/home/mvdan/src/test/fuzz_test.go:10 +0x89
                reflect.Value.call({0x58f7c0, 0x5c9a50, 0x13}, {0x5bb82c, 0x4}, {0xc0000aac60, 0x2, 0x2})
                	/home/mvdan/tip/src/reflect/value.go:542 +0x814
                reflect.Value.Call({0x58f7c0, 0x5c9a50, 0x4f7ae0}, {0xc0000aac60, 0x2, 0x2})
                	/home/mvdan/tip/src/reflect/value.go:338 +0xc5
                testing.(*F).Fuzz.func1.1(0x0)
                	/home/mvdan/tip/src/testing/fuzz.go:419 +0x20b
                testing.tRunner(0xc0001089c0, 0xc0000fa6c0)
                	/home/mvdan/tip/src/testing/testing.go:1389 +0x102
                created by testing.(*F).Fuzz.func1
                	/home/mvdan/tip/src/testing/fuzz.go:408 +0x4df
                
    
    Crash written to testdata/fuzz/FuzzFoo/1ae33179c96f37941724957ef1e01afb5882690ced17341968011c75d694a8ba
    To re-run:
    go test test -run=FuzzFoo/1ae33179c96f37941724957ef1e01afb5882690ced17341968011c75d694a8ba
FAIL

Note that I got a fairly large stack frame, but I don't actually find most of those entries useful. Compare that with a func TestFoo(t *testing.T) { panic("foo") }, which prints roughly the same as a func FuzzFoo(f *testing.F) { panic("foo") }:

--- FAIL: TestFoo (0.00s)
panic: foo [recovered]
	panic: foo

goroutine 6 [running]:
testing.tRunner.func1.2({0x4f9b80, 0x543840})
	/home/mvdan/tip/src/testing/testing.go:1339 +0x24e
testing.tRunner.func1()
	/home/mvdan/tip/src/testing/testing.go:1342 +0x357
panic({0x4f9b80, 0x543840})
	/home/mvdan/tip/src/runtime/panic.go:814 +0x207
test.TestFoo(0x0)
	/home/mvdan/src/test/fuzz_test.go:8 +0x27
testing.tRunner(0xc000134340, 0x5244d0)
	/home/mvdan/tip/src/testing/testing.go:1389 +0x102
created by testing.(*T).Run
	/home/mvdan/tip/src/testing/testing.go:1436 +0x35f
exit status 2
FAIL	test	0.004s

The lowest hanging fruit is the runtime/debug.Stack stack frame from the original output. https://pkg.go.dev/runtime#Callers certainly supports skipping a number of frames, for example. I'm not sure why calling debug.Stack includes its own frame, but I've run out of time to figure out if that is by design.

And, more in general and not specifically to fuzzing, I would hope that other frames like testing.tRunner.func1 or testing.tRunner would not be included. Ideally, the frame would go from TestFoo to panic, and nothing else - because as a user, that's the code I wrote. I get that including the full stack trace from func main is correct, but in practice that's usually not what the user cares about.

cc @katiehockman @jayconrod
@seankhliao seankhliao added fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 9, 2021
@katiehockman
Copy link
Contributor

Similar to what I said in #48884, I think this will be fixed by #48709, which is the next bug I plan to fix.

Let's re-visit this one once that's fixed to see if the issue is still reproducible.

@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
@julieqiu julieqiu modified the milestones: Unplanned, Backlog Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
fuzz Issues related to native fuzzing support NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Go Security
Status: No status
Milestone
Backlog
Development

No branches or pull requests
4 participants
@mvdan @julieqiu @seankhliao @katiehockman