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/ssa: emitConv panic on generics #57272

Closed
zpavlinovic opened this issue Dec 13, 2022 · 6 comments
Closed

x/tools/go/ssa: emitConv panic on generics #57272

zpavlinovic opened this issue Dec 13, 2022 · 6 comments
Assignees
@timothy-king @zpavlinovic
Labels
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. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Milestone
Unreleased

Comments

@zpavlinovic
Copy link
Contributor

zpavlinovic commented Dec 13, 2022 •
edited

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

$ go version

go1.19.3

Does this issue reproduce with the latest release?

Yes, also reproduced with go version go1.20-pre3.

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

go env Output
$ go env

GO111MODULE=""

GOARCH="amd64"

GOBIN=""

GOCACHE="[redacted]/.cache/go-build"

GOENV="[redacted]/.config/go/env"

GOEXE=""

GOEXPERIMENT=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="linux"

GOINSECURE=""

GOMODCACHE="[redacted]/go/pkg/mod"

GONOPROXY=".git.corp.google.com"

GONOSUMDB=".git.corp.google.com"

GOOS="linux"

GOPATH="[redacted]/go"

GOPRIVATE="*.git.corp.google.com"

GOPROXY="https://proxy.golang.org,direct"

GOROOT="[redacted]"

GOSUMDB="sum.golang.org"

GOTMPDIR=""

GOTOOLDIR="[redacted]/linux_amd64"

GOVCS=""

GOVERSION="go1.20-pre3 cl/474093167 +a813be86df"

GCCGO="gccgo"

GOAMD64="v1"

AR="ar"

CC="clang"

CXX="clang++"

CGO_ENABLED="1"

GOMOD="[redacted]/minio/go.mod"

GOWORK=""

CGO_CFLAGS="-g -O2"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-g -O2"

CGO_FFLAGS="-g -O2"

CGO_LDFLAGS="-g -O2"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2425109656=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run ssa on https://github.com/minio/minio. Reproducible by running govulncheck, which uses ssa:

govulncheck ./...

Also reproducible if govulncheck uses (the latest) v0.4.1-0.20221212161611-18f76ecd3f38 version of tools.

What did you expect to see?

No panic.

What did you see instead?

panic: in github.com/minio/minio/internal/config.Error: cannot convert *t0 (PT) to T

goroutine 16266 [running]:
golang.org/x/tools/go/ssa.emitConv(0xc06b569680, {0x9436a0, 0xc080fa2ea0}, {0x93fcb8?, 0xc052874180})
       [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/emit.go:295 +0xa2c
golang.org/x/tools/go/ssa.(*builder).stmt(0xc080f9acc0?, 0xc06b569680, {0x941208?, 0xc04c2a2fc0?})
        [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:2220 +0x1145
golang.org/x/tools/go/ssa.(*builder).stmtList(0x40d11d?, 0x849dc0?, {0xc03c94cac0?, 0x3, 0xc080fa41b0?})
        [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:946 +0x45
golang.org/x/tools/go/ssa.(*builder).stmt(0xc06b569680?, 0xc06b569680, {0x940c68?, 0xc04cb0cd80?})
        [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:2277 +0x859
golang.org/x/tools/go/ssa.(*builder).buildFunctionBody(0x656791?, 0xc06b569680)
       [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:2391 +0x437
golang.org/x/tools/go/ssa.(*builder).buildFunction(0x6567e0?, 0xc06b569680)
        [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:2326 +0x2e
golang.org/x/tools/go/ssa.(*builder).buildCreated(0xc076503e18)
        [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:2413 +0x25
golang.org/x/tools/go/ssa.(*Package).build(0xc06b508980)
        [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:2606 +0xc86
sync.(*Once).doSlow(0xc00260cfb8?, 0x608ede?)
        [redacted]/src/sync/once.go:74 +0xc2
sync.(*Once).Do(...)
        [redacted]/src/sync/once.go:65
golang.org/x/tools/go/ssa.(*Package).Build(...)
        [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:2477
golang.org/x/tools/go/ssa.(*Program).Build.func1(0x0?)
       [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:2462 +0x4c
created by golang.org/x/tools/go/ssa.(*Program).Build
        [redacted]/go/pkg/mod/golang.org/x/tools@v0.4.1-0.20221212161611-18f76ecd3f38/go/ssa/builder.go:2461 +0x19c

It looks like this is the function causing the panic. Perhaps a duplicate of #56849?

Thanks to @harshavardhana for reporting this.

cc @timothy-king
@zpavlinovic zpavlinovic added 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. labels Dec 13, 2022
@zpavlinovic zpavlinovic added this to the Unreleased milestone Dec 13, 2022
@timothy-king
Copy link
Contributor

Here is what seem to be the relevant bits:

type I interface { X | Y }

type X struct { v string }
type Y struct { X }

func Foo[T I, PT interface { *T }]() {
	_ = PT(new(T))
}

I don't think this seems to be about supporting arrays conversions or multiple distinct kinds of conversions. So I don't think this is a duplicate of #56849.

@zpavlinovic zpavlinovic self-assigned this Dec 15, 2022
@zpavlinovic zpavlinovic added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Dec 15, 2022
@zpavlinovic
Copy link
Contributor Author

The issue could be in

func emitLoad(f *Function, addr Value) *UnOp {
	v := &UnOp{Op: token.MUL, X: addr}
	v.setType(deref(addr.Type()))
	f.emit(v)
	return v
}

Likely, deref is not picking the type under the pointer. addr.CoreType could be a solution here.

@gopherbot
Copy link

Change https://go.dev/cl/458235 mentions this issue: go/ssa: deref core type in emitLoad

@gopherbot
Copy link

Change https://go.dev/cl/458255 mentions this issue: all: update dependency to tools

gopherbot pushed a commit to golang/vuln that referenced this issue Dec 17, 2022
@zpavlinovic @gopherbot
all: update dependency to tools
785bada 
Resolves some panics in ssa.

Updates golang/go#57272

Change-Id: Icf2bb1d2e0c54f9cf8443e339dd0935019e887e6
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/458255
Auto-Submit: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
@Antonboom Antonboom mentioned this issue Mar 19, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/477635 mentions this issue: go/ssa: deref core type in emitStore

softdev050 added a commit to softdev050/Golangvuln that referenced this issue Apr 5, 2023
@softdev050
all: update dependency to tools
3fc48c4 
Resolves some panics in ssa.

Updates golang/go#57272

Change-Id: Icf2bb1d2e0c54f9cf8443e339dd0935019e887e6
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/458255
Auto-Submit: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
sayjun0505 added a commit to sayjun0505/Golangvuln that referenced this issue Apr 8, 2023
@sayjun0505
all: update dependency to tools
43d4098 
Resolves some panics in ssa.

Updates golang/go#57272

Change-Id: Icf2bb1d2e0c54f9cf8443e339dd0935019e887e6
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/458255
Auto-Submit: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/492598 mentions this issue: go/ssa: use core type in address

gopherbot pushed a commit to golang/tools that referenced this issue May 5, 2023
@timothy-king
go/ssa: use core type in address
0500fd4 
The type of an address is now the element type of a type whose core type
is a pointer type. Previously this was the element type of a type whose
underlying type was a pointer type. emitStore now uses the core type as
well.
	func f[M any, P *M](p P) {
		var m M
		*p = m
	}

is emitted as:

	func f[M any, P *M](p P):
	0:
		*p = *new(M):M
		return

Related to golang/go#57272
Fixes golang/go#58633

Change-Id: I35d4345a9b3f69bcd28cf8342f7fec550329eba4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/492598
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tim King <taking@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
stanislavkononiuk added a commit to stanislavkononiuk/Golangvuln that referenced this issue Jun 26, 2023
@stanislavkononiuk
all: update dependency to tools
1bc6252 
Resolves some panics in ssa.

Updates golang/go#57272

Change-Id: Icf2bb1d2e0c54f9cf8443e339dd0935019e887e6
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/458255
Auto-Submit: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@timothy-king timothy-king

@zpavlinovic zpavlinovic

Labels
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. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@timothy-king @zpavlinovic @gopherbot