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

x/tools/go/ssa: panic: variadic parameter must be of unnamed slice type #54946

Closed
visualfc opened this issue Sep 8, 2022 · 11 comments
Closed

x/tools/go/ssa: panic: variadic parameter must be of unnamed slice type #54946

visualfc opened this issue Sep 8, 2022 · 11 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@visualfc
Copy link

visualfc commented Sep 8, 2022 •
edited

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

$ go version
go version go1.18.4 darwin/amd64

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

Input program: $GOROOT/test/typeparam/issue376214.go

package main

func add[S ~string | ~[]byte](buf *[]byte, s S) {
	*buf = append(*buf, s...)
}

func main() {
	var buf []byte
	add(&buf, "foo")
	add(&buf, []byte("bar"))
	if string(buf) != "foobar" {
		panic("got " + string(buf))
	}
}

I run it with ssadump -build=G issue376214.go

What did you expect to see?

I expect it pass.

What did you see instead?

panic: variadic parameter must be of unnamed slice type

goroutine 1 [running]:
go/types.NewSignatureType(0x0, {0x0, 0x0, 0x0}, {0x0, 0x0, 0x0}, 0xc00000d908, 0xc00000d758, 0x1)
	$home/myproj/go/src/go/types/signature.go:53 +0x29e
golang.org/x/tools/internal/typeparams.NewSignatureType(...)
	$home/myproj/tools/internal/typeparams/typeparams_go118.go:55
golang.org/x/tools/go/ssa.(*subster).signature(0x1286b00?, 0xc0002de200)
	$home/myproj/tools/go/ssa/subst.go:372 +0xd2
....
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 8, 2022
@gopherbot gopherbot added this to the Unreleased milestone Sep 8, 2022
@mknyszek
Copy link
Contributor

mknyszek commented Sep 8, 2022

CC @golang/tools-team

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 8, 2022
@findleyr
Copy link
Contributor

findleyr commented Sep 9, 2022

CC @timothy-king @adonovan @zpavlinovic

@timothy-king
Copy link
Contributor

I was not able to reproduce this yet. Steps I tried below.

So far the root cause of similar issues has been compiling with a too old version of x/tools. Do you know what version of go you compiled ssadump with and what version of x/tools it was?

Here are the steps I took to reproduce.

% go install golang.org/dl/go1.18.4@latest
% ~/go/bin/go1.18.4 download
% go1.18.4 install golang.org/x/tools/cmd/ssadump@latest
% go version -m /Users/taking/go/bin/ssadump
/Users/taking/go/bin/ssadump: go1.18.4
        path    golang.org/x/tools/cmd/ssadump
        mod     golang.org/x/tools      v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU=
        dep     golang.org/x/mod        v0.6.0-dev.0.20220419223038-86c51ed26bb4        h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
        dep     golang.org/x/sys        v0.0.0-20220722155257-8c9f86f7a55f      h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s=
...
% /Users/taking/go/bin/ssadump ~/sdk/go1.18.4/test/typeparam/issue376214.go
% # works fine

@visualfc
Copy link
Author

visualfc commented Sep 10, 2022 •
edited

ssadump ~/sdk/go1.18.4/test/typeparam/issue376214.go

ssadump -build=FG ~/sdk/go1.18.4/test/typeparam/issue376214.go

-build=G for ssa instantiate generic function

@findleyr
Copy link
Contributor

@visualfc please share the output of go version -m $(which ssadump).

@timothy-king
Copy link
Contributor

Thanks. I was missing -build=G. ssadump -build=G ~/sdk/go1.18.4/test/typeparam/issue376214.go now reproduces.

@visualfc
Copy link
Author

@visualfc please share the output of go version -m $(which ssadump).

go1.18.4
	path	golang.org/x/tools/cmd/ssadump
	mod	golang.org/x/tools	v0.1.12	h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU=
	dep	golang.org/x/mod	v0.6.0-dev.0.20220419223038-86c51ed26bb4	h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
	dep	golang.org/x/sys	v0.0.0-20220722155257-8c9f86f7a55f	h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s=
	build	-compiler=gc
	build	CGO_ENABLED=1
	build	CGO_CFLAGS="-g -O2"
	build	CGO_CPPFLAGS=
	build	CGO_CXXFLAGS="-g -O2"
	build	CGO_LDFLAGS="-g -O2"
	build	GOARCH=amd64
	build	GOOS=darwin
	build	GOAMD64=v1

@timothy-king timothy-king added NeedsFix The path to resolution is known, but the work has not been done. Analysis Issues related to static analysis (vet, x/tools/go/analysis) and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 12, 2022
@timothy-king
Copy link
Contributor

The example in question:

func add[S ~string | ~[]byte](buf *[]byte, s S) {
	*buf = append(*buf, s...)
}

The issue comes from substitution of S to string. The type of append needs to go from a func([]byte, S...) []byte to a func([]byte, string...)[]byte.

I don't believe we can create the signature func([]byte, string...)[]byte from go/types ATM. This can either be solved by go/types being more permissive for this as a special case, offering an interface that lets this Signature be created, or ssa instantiating a different type as a special case, like func([]byte, []byte...)[]byte, and adding conversion, like treating this as append(*buf, []byte(s)...).

@findleyr findleyr mentioned this issue Sep 12, 2022
Closed
@findleyr
Copy link
Contributor

Filed #55030 for discussion of what to do about this in go/types.

@timothy-king
Copy link
Contributor

Fixed at tip. Waiting for a366ed5 to be released with 1.19.2 and e40a130 to be released with 1.18.7 for this to be closed.

@timothy-king
Copy link
Contributor

Fixes have been back-ported.

@bcmills bcmills mentioned this issue May 20, 2023
Closed
@golang golang locked and limited conversation to collaborators Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. 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
@visualfc @timothy-king @mknyszek @gopherbot @findleyr