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.

Sign up for GitHub

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

x/tools/go/analysis/passes/printf: segfault coming from printf #55350

Closed
shoenig opened this issue Sep 22, 2022 · 6 comments
Closed

x/tools/go/analysis/passes/printf: segfault coming from printf #55350

shoenig opened this issue Sep 22, 2022 · 6 comments
Assignees
@zpavlinovic
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@shoenig
Copy link
Contributor

shoenig commented Sep 22, 2022

What version of Go are you using (go version)?

$ go version
go version go1.19.1 linux/amd64

Started adding a test case and found I can segfault the compiler via fmt.Println

func TestSet_GoString(t *testing.T) {
	t.Run("example", func(t *testing.T) {
		s := From[string]([]string{"a", "b", "c"})
		fmt.Println(s) // segfault
	})
}

➜ go test
# github.com/hashicorp/go-set
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x6700e4]

goroutine 65 [running]:
go/types.(*Scope).Contains(...)
	/usr/local/go/src/go/types/scope.go:179
cmd/vendor/golang.org/x/tools/go/analysis/passes/printf.recursiveStringer(0xc000190340, {0x759dc0?, 0xc00025f020?})
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go:955 +0x1a4
cmd/vendor/golang.org/x/tools/go/analysis/passes/printf.checkPrint(0xc000190340, 0xc000260780, 0xc000294360)
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go:1100 +0x7c9
cmd/vendor/golang.org/x/tools/go/analysis/passes/printf.checkCall.func1({0x758bd8?, 0xc000260780})
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go:483 +0x7b
cmd/vendor/golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0xc0008a8a98, {0xc000622c80?, 0x1?, 0xc00009e630?}, 0xc000a93c90)
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/ast/inspector/inspector.go:77 +0x9a
cmd/vendor/golang.org/x/tools/go/analysis/passes/printf.checkCall(0xc000190340)
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go:476 +0x98
cmd/vendor/golang.org/x/tools/go/analysis/passes/printf.run(0x6aab00?)
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/printf/printf.go:141 +0x72
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func5.1()
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:354 +0x864
sync.(*Once).doSlow(0x6a8ac0?, 0xc001080270?)
	/usr/local/go/src/sync/once.go:74 +0xc2
sync.(*Once).Do(...)
	/usr/local/go/src/sync/once.go:65
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func5(0x8d2300?)
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:306 +0x1a5
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func6.1(0x0?)
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:366 +0x29
created by cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func6
	/usr/local/go/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:365 +0x47
FAIL	github.com/hashicorp/go-set [build failed]

Branch where I ran into this (project is small, easy to repro just by running go test):
https://github.com/hashicorp/go-set/tree/panic-on-go-string

I suspect it has something to do with our non normal String(func()) method, but I dunno
@shoenig shoenig changed the title affected/package: go/types segfault coming from printf go/types: segfault coming from printf Sep 22, 2022
@mvdan
Copy link
Member

mvdan commented Sep 22, 2022

What did you run? Presumably you ran something like go vet or gopls to run the static analysis over the code, which crashed. As far as I can tell, this may be a bug in x/tools/go/analysis/passes/printf, not go/types.

@shoenig
Copy link
Contributor Author

shoenig commented Sep 22, 2022

I ran go test.

I just marked go/types because that's the first line in the output; I'm not familiar with Go source.

goroutine 65 [running]:
go/types.(*Scope).Contains(...)

@mvdan
Copy link
Member

mvdan commented Sep 22, 2022

Forgive me; both of those are clearly visible in your original report. I clearly need to rest my eyes :)

@cherrymui cherrymui changed the title go/types: segfault coming from printf x/tools/go/analysis/passes/printf: segfault coming from printf Sep 22, 2022
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 22, 2022
@gopherbot gopherbot added this to the Unreleased milestone Sep 22, 2022
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 22, 2022
@cherrymui
Copy link
Member

Tentatively marking it as an issue of the analysis instead of go/types, as the go/types function is just a small helper function. Maybe it calls Contains with a nil *Scope?

cc @timothy-king @zpavlinovic

@zpavlinovic
Copy link
Contributor

zpavlinovic commented Sep 23, 2022 •
edited
Loading

I believe this is related to generics.

If I replace the call to From[string](...) with FromString(...) where

func FromString(items []string) *SetString {
        return nil
}               
                
type SetString struct {
        items map[string]nothing
} 

func (s *SetString) String(f func(element string) string) string { 
        return ""
}

I don't get this error. I am not sure yet if the *Scope is expected to be nil there.

(Note that the simplified body of FromString does not play a role here. One can do the same with From[T].)

@zpavlinovic zpavlinovic self-assigned this Sep 23, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/433755 mentions this issue: go/analysis/passes/printf: check for nil scope of instance methods

@golang golang locked and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@zpavlinovic zpavlinovic

Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@shoenig @zpavlinovic @mvdan @gopherbot @cherrymui