-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/go2go: fuzzing triggers various crashes in type checker #39634
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
Comments
These are all non-urgent, but I agree the tools shouldn't crash. Will get to these eventually. If you find more similar crashes, please add to this issue, rather than creating many more. Thanks for reporting! |
Hi Robert, that makes sense, and will do. Thanks for your work here! |
I've got some more examples of some crashes that I found manually messing around with things, and this issue seemed like an appropriate place to put them. Crash 7: panic: assertion failed
https://go2goplay.golang.org/p/1VztHp6nDlp Crash 8: fatal error: stack overflow
https://go2goplay.golang.org/p/fX5PYaFHB1Z Crash 9: internal compiler error: typecheck DCLFIELD
https://go2goplay.golang.org/p/8-gvXzWvuky This last one isn't necessarily a crash, but I don't know how to categorize it because it seems like some sort of unknown error class. Crash 10: looping while expanding type
|
EDIT: it looks like this panic occurs when any function doesn't have a body, so for example: https://go2goplay.golang.org/p/Q4yf9FPaydk package main
func main() The usage of Here's another one discovered by @zeebo while we were discussing gaining access to the runtime's map hasher functions: Works fine in normal playground: https://play.golang.org/p/WWVqIxtH7b1
|
Here is another batch of crashes from fuzzing. (For numbering, I am treating the five from @zeebo and @mdlayher above as part of the numbering sequence, and hence am starting with 12 here). This is still with:
Crash 12: panic: unreachablego/types.(*Checker).assignment
https://go2goplay.golang.org/p/zZxlD1B7QpN Crash 13: panic: internal error: final action list grewgo/types.(*Checker).processFinals
https://go2goplay.golang.org/p/fUILIW9WD5E Crash 14: fatal error: stack overflowgo/types.(*unifier).nify
https://go2goplay.golang.org/p/BaXFnTQjzE4 Crash 15: panic: assertion failedgo/types.makeSubstMap
https://go2goplay.golang.org/p/R9gtjp611eP Crash 16: panic: assertion failedgo/types.(*subster).typ
https://go2goplay.golang.org/p/pXrh7-AoAZi Crash 17: panic: duplicate method cgo/types.(*Interface).Complete
https://go2goplay.golang.org/p/Q9iDpXeF3u- Crash 18: panic: nil typgo/types.(*subster).typ
https://go2goplay.golang.org/p/cTESh52u3h6 Crash 19: panic: assertion failedgo/types.(*Checker).rawExpr
|
Sorry, Crash 15 is probably a duplicate of @zeebo's first crash in #39634 (comment), but I will leave it here regardless (including to avoid renumbering 😅) |
Of the first batch, crashes 3, 5, and 6 are real. The others (1, 2, 4) don't crash anymore on dev.go2go. 3, 5, and 6 are unrelated to generics and may also crash in the regular type checker (try go vet on them). If so, please file individual bugs against the regular go/types package. |
@zeebo Crashes 7, 8, 9, and 10 don't reproduce in dev.go2go with the type checker. I haven't tried the translator. |
Hi @griesemer thanks for taking a look at these. In case it is more convenient for you or otherwise helps, I put the fuzz-induced crashes from this issue into a set of test cases that exercise go/types, and which you can see here in the standard playground: https://play.golang.org/p/rtBwSht8Urf 4 of the 14 crash in go1.14.4, while the other 10 pass in go1.14.4. Here are the current results from the standard playground: go1.14.4
I haven't yet run them on the latest dev.go2go. Also, running those in the standard Go Playground executes the tests. However, go2goplay.golang.org seems to have a bug or limitation such that it does not recognize and run those as tests (perhaps related to whatever the root cause might be for #39675). |
Hi @griesemer here is the state of crash/pass/skip in the latest commit on dev.go2go, using the fuzzing-originated unit tests I had placed in https://play.golang.org/p/rtBwSht8Urf (same playground link as immediately prior comment).
dev.go2go (6cf535a Fri Jun 19 04:37:09 2020)
Progress! Summary
edit: sorry, this comment was initially missing crash-14, which still fails with a stack overflow. |
Crash 14 is caused by Line 95 in 0a03088
y is *types.Basic which have types.Invalid kind, so expand just returns y
and Lines 150 to 157 in 0a03088
recursive call. Change Lines 94 to 95 in 0a03088
to x = expand(x)
if basicX, ok := x.(*Basic); ok && basicX.Kind() == Invalid {
return false
}
y = expand(y)
if basicY, ok := y.(*Basic); ok && basicY.Kind() == Invalid {
return false
} prevents endless recursion |
That's great @tdakkota! Thanks for looking at that! |
@tdakkota Thanks for analyzing the problem. The correct fix (forthcoming) is a bit more subtle, though. Checking for the invalid type solves this specific case, but is insufficient in general. For instance, this test case will still recurse infinitely: func F(type T)(func(T) T) { F(func(T) int {}) } |
Hi @griesemer, here is a third batch of fuzz-induced crashes (Crash 20 through Crash 27 for this issue). For now, I am just posting the unit test form in this link to the standard playground: https://play.golang.org/p/qXP94Ai_vQq If you are looking at stack overflows, this new batch includes two new variations of stack overflows. This new batch of Crashes 20-27 does not crash in Go 1.14.4. Here are the results from running these via 'go test' in the latest dev.go2go: dev.go2go (0a03088 Sat Jun 20 04:46:40 2020)
edit: updated playground link with slightly cleaner version of unit tests. (Original link here). |
Hi @griesemer, here is the third batch of fuzz-induced crashes again, but now expanded out in markdown. Crash 20 and Crash 21 might be duplicates at this point. (Previously, Crash 21 used to crash in Finally, I am including go2go playground links here, but the reported behavior in this comment is for 0a03088 (and the go2go playground is older than that).
Crash 20: invalid memory address or nil pointer dereferencego/types.(*Checker).completeInterface
https://go2goplay.golang.org/p/wAHpBNS7ZPn Crash 21: invalid memory address or nil pointer dereferencego/types.(*Checker).completeInterface
https://go2goplay.golang.org/p/H64m4NteTx5 Crash 22: fatal error: stack overflowgo/types.(*instance).expand
https://go2goplay.golang.org/p/CoxvTTa5xP1 Crash 23: fatal error: stack overflowgo/types.Comparable
https://go2goplay.golang.org/p/75sK6SAI2c6 Crash 24: panic: assertion failedgo/types.(*unifier).nify
https://go2goplay.golang.org/p/kWYVyRvSGDd Crash 25: panic: assertion failedgo/types.(*unifier).nify
https://go2goplay.golang.org/p/m5k0NKSMK26 Crash 26: panic: assertion failedgo/types.isParameterized
https://go2goplay.golang.org/p/BWt_Xe6iwsE Crash 27: panic: interface is incompletego/types.(*Interface).assertCompleteness
|
@thepudds Thanks. I will look at these eventually, they are useful to test the corners of the implementation (keep in mind, this is still a prototype, and some pieces are pretty rough and known to be not correct for all cases). |
Change https://golang.org/cl/240901 mentions this issue: |
…idity Addresses crash #20 of #39634. Added test, also for some of the other cases that don't crash anymore. Updates #39634. Change-Id: I999e376985a443ac435f64a7c249f891e7b7a6d7 Reviewed-on: https://go-review.googlesource.com/c/go/+/240901 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/240902 mentions this issue: |
…erized predicate Also, take type lists into account when computing predicate. Addresses crash #27 of #39634. Updates #39634. Change-Id: Id6e1d0d86ac1cb9d79828a3b5fdfbeba0e6aace0 Reviewed-on: https://go-review.googlesource.com/c/go/+/240902 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/240903 mentions this issue: |
… type Addresses crash #19 of #39634. Updates #39634. Change-Id: I42208511a3fc27432891243dcf4799e80432c2c1 Reviewed-on: https://go-review.googlesource.com/c/go/+/240903 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/240904 mentions this issue: |
…pe declarations. Updates golang#39634. Fixes golang#40037.
Change https://golang.org/cl/241037 mentions this issue: |
…e not fully set up Addresses crash #18 of #39634. Updates #39634. Change-Id: I4b1a3d81ce9dc2b59ae9af3146d26f79d8c05973 Reviewed-on: https://go-review.googlesource.com/c/go/+/240904 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/241122 mentions this issue: |
…e not fully set up Addresses crash #16 of #39634. Updates #39634. Change-Id: I33063d6e2176586f097d2d1391f490f1775a2202 Reviewed-on: https://go-review.googlesource.com/c/go/+/241122 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/241123 mentions this issue: |
This test case doesn't crash anymore. Updates #39634. Change-Id: I29b13762265ef1c16c0298d141fdb6cf2c01f4ee Reviewed-on: https://go-review.googlesource.com/c/go/+/241123 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/245737 mentions this issue: |
Also, update some comments. Updates #39634. Change-Id: I85c63e4b941e2bc04e8b5a59497fc47e5d578b8b Reviewed-on: https://go-review.googlesource.com/c/go/+/245737 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/245738 mentions this issue: |
Addresses crash #12 of 39634. Updates #39634. Change-Id: Ie67545d21b6a1e80fcdec1b2eb101b69c25e2952 Reviewed-on: https://go-review.googlesource.com/c/go/+/245738 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/245739 mentions this issue: |
…nstantiated Addresses crash 24 of #39634. Updates #39634. Change-Id: If44e834834a07719e7f727d12cc24ac4af0997e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/245739 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/245740 mentions this issue: |
…atory comment Addresses crash 26 of #39634. Also, added test case for crash 25 which was fixed with https://golang.org/cl/245739. Updates #39634. Change-Id: I19793eb79fce63052ba055cde0b1dafdd9476895 Reviewed-on: https://go-review.googlesource.com/c/go/+/245740 Reviewed-by: Robert Griesemer <[email protected]>
Change https://golang.org/cl/245742 mentions this issue: |
This test passes type-checking without problems but the generated code causes a cmd/compile error (filed #40486). Updates #39634. Change-Id: If3dd28605f6d8792c6bd5bb032624e9a6140b2c0 Reviewed-on: https://go-review.googlesource.com/c/go/+/245742 Reviewed-by: Robert Griesemer <[email protected]>
What version of Go are you using (
go version
)?My local go version used for the fuzzing:
Does this issue reproduce with the latest release?
Yes with a recent
git checkout dev.go2go
What operating system and processor architecture are you using (
go env
)?linux/amd64
What did you do?
I tried fuzzing the type checker from the
dev.go2go
branch for a short period of time (roughly around 45 minutes) and was able to generate a variety of crashes. Here are six samples.I realize this is still early days, and I suspect it is much more useful to have crashes or panics from code that actual humans wrote, but posting these examples of crashes in case there is interest in hardending against more corner cases, or perhaps using some as test cases in the future.
I was running fzgo, which is a prototype of making fuzzing a first class citizen in the go command (#19109).
The engine underneath fzgo is @dvyukov's go-fuzz. I can post the fuzz function later if interested, but it was a cut down and slightly tweaked fuzz function adapted from https://github.com/dvyukov/go-fuzz-corpus/tree/master/gotypes.
These all crash on the current go2goplay.golang.org:
Crash 1: invalid memory address or nil pointer dereference
go/types.(*Checker).instantiate
http://go2goplay.golang.org/p/7Jk4PT9GX3k
Crash 2: invalid memory address or nil pointer dereference
go/types.optype
https://go2goplay.golang.org/p/46EZOUKBLLu
Crash 3: invalid memory address or nil pointer dereference
go/types.IsInterface
https://go2goplay.golang.org/p/HChlkK2A_Di
Crash 4: invalid memory address or nil pointer dereference
go/types.(*Interface).Complete
https://go2goplay.golang.org/p/TwoY4k9kR1w
Crash 5: panic: multiplication of zero with infinity
math/big.(*Float).Mul
https://go2goplay.golang.org/p/avwOXp4HJrC
Crash 6: panic: assertion failed
go/types.(*Checker).shift
https://go2goplay.golang.org/p/pv_BlSJ9v5W
(Side note: it would be nice to be able to issue commands like
go test -fuzz=. ./...
(#19109) on the stdlib and elsewhere ;-)The text was updated successfully, but these errors were encountered: