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

x/tools/go/ssa: crashes on code containing generics #54086

Open
MadhavJivrajani opened this issue Jul 27, 2022 · 3 comments
Open

x/tools/go/ssa: crashes on code containing generics #54086

MadhavJivrajani opened this issue Jul 27, 2022 · 3 comments
Assignees
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) 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

Comments

@MadhavJivrajani
Copy link

MadhavJivrajani commented Jul 27, 2022

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

$ go version
go version go1.19rc2 linux/amd64

Does this issue reproduce with the latest release?

Yes, with go1.19rc2.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/nikhita/.cache/go-build"
GOENV="/home/nikhita/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/nikhita/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/nikhita/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org/,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19rc2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/nikhita/go/src/k8s.io/kubernetes/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 -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build4046507775=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Kubernetes runs a pre-submit which makes use of levee. The script for which can be found here: https://github.com/kubernetes/kubernetes/blob/master/hack/verify-govet-levee.sh

What did you expect to see?

Expected no crashes as per #48525 (comment)

What did you see instead?

After bumping the test image to 1.19rc2, the above mentioned job started panicing, panic starting from x/tools/go/analysis/unitchecker and ending in x/tools/go/ssa. The entire stack trace can be found here as part of the logs: https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/directory/pull-kubernetes-verify-govet-levee/1552157434636668928

Reproducing the issue

To reproduce it locally, running ./hack/verify-govet-levee.sh on Kubernetes master after switching to go1.19rc2 should do it.

Misc info

#48525 was an issue for the Go 1.18 timeframe as well, but the failing job above worked fine before. I'm not sure why this happens after switching to go1.19. Could this be because of go1.19 introducing generics in the stdlib?

The reason I list the above reason is because in the logs we see panics in the following two instances:

instances of generics in the stdlib

panic: no concrete method: func (*sync/atomic.Pointer[go/token.File]).CompareAndSwap(old *go/token.File, new *go/token.File) (swapped bool)

and

panic: no concrete method: func (*crypto/elliptic.nistCurve[*crypto/internal/nistec.P224Point]).Add(x1 *math/big.Int, y1 *math/big.Int, x2 *math/big.Int, y2 *math/big.Int) (*math/big.Int, *math/big.Int)

For more details around this failure and the timeline of merges in Kubernetes that led to them, please see: kubernetes/kubernetes#111452


cc @timothy-king
Since you were working on the generics support for SSA
cc @nikhita @dims @liggitt @palnabarun

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jul 27, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jul 27, 2022
@cherrymui cherrymui added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Analysis Issues related to static analysis (vet, x/tools/go/analysis) labels Jul 27, 2022
@cherrymui cherrymui changed the title x/tools/go/ssa: Crashes on code containing generics x/tools/go/ssa: crashes on code containing generics Jul 27, 2022
@timothy-king timothy-king self-assigned this Aug 3, 2022
@timothy-king
Copy link
Contributor

kubernetes/hack/tools is using x/tools at 77aa08bb151a:

# From go/src/k8s.io/kubernetes/hack/tools
$ go list -m golang.org/x/tools
golang.org/x/tools v0.1.11-0.20220316014157-77aa08bb151a

You need to sync past at least the last relevant commit for #48525 (comment).

You can upgrade to the v0.1.12 release as follows:

# From go/src/k8s.io/kubernetes/hack/tools
$ go get golang.org/x/tools@v0.1.12
$ go mod tidy

Running $ ./hack/verify-govet-levee.sh I now see:

# From go/src/k8s.io/kubernetes
$ ./hack/verify-govet-levee.sh
[...]

+++ [0803 23:29:23] Building go targets for linux/amd64
    k8s.io/kubernetes/hack/make-rules/helpers/go2make (non-static)
# sync/atomic
unexpected type received: *types.TypeParam T; please report this issue
unexpected type received: *types.TypeParam T; please report this issue
unexpected type received: *types.TypeParam T; please report this issue
unexpected type received: *types.TypeParam T; please report this issue
# crypto/elliptic
unexpected type received: *types.TypeParam Point; please report this issue

This addresses the crashes you are seeing. This is now google/go-flow-levee#323.

In the long term levee should:

  1. update its x/tools dependency (quick n easy) and
  2. support type parameters. A stop gap to simply ignore type params and generic functions/methods seems easy. More is blocked on x/tools/go/ssa: generics support #48525 (comment) for now.

@dims
Copy link

dims commented Aug 4, 2022

@timothy-king thanks for the analysis.

yarcat added a commit to yarcat/text that referenced this issue Aug 9, 2022
go get -u golang.org/x/tools
go mod tidy

For golang/go#54086
yarcat added a commit to yarcat/text that referenced this issue Aug 9, 2022
go get -u golang.org/x/tools
go mod tidy

x/tools/go/ssa is used by message/pipeline. The version 0.1.11 required
contained an issue related to generics, when generics instantiation was
confused with container index. However, x/tools/go/ssa does have it fixed
at head (see golang/go#52834). This change upgrades the required version
to ensure less users of message/pipelines have issues.

For golang/go#54086
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/422414 mentions this issue: go.mod: update x/tools to HEAD

yarcat added a commit to yarcat/text that referenced this issue Aug 12, 2022
go get -u golang.org/x/tools
go mod tidy

x/tools/go/ssa is used by message/pipeline. Generics instantiation in
x/tools@0.1.11 could be confused with container index. However,
x/tools/go/ssa does have it fixed in x/tools@0.1.12 (see golang/go#52834).
This change upgrades the required version to ensure that fewer users of
message/pipelines have issues.

Updates golang/go#54086
yarcat added a commit to yarcat/text that referenced this issue Aug 12, 2022
go get -u golang.org/x/tools
go mod tidy

x/tools/go/ssa is used by message/pipeline. Generics instantiation in
x/tools@0.1.11 could be confused with container index. However,
x/tools/go/ssa does have it fixed in x/tools@0.1.12 (see golang/go#52834).
This change upgrades the required version to ensure that fewer users of
message/pipelines have issues.

Updates golang/go#54086
gopherbot pushed a commit to golang/text that referenced this issue Aug 12, 2022
go get -u golang.org/x/tools
go mod tidy

x/tools/go/ssa is used by message/pipeline. Generics instantiation in
x/tools@0.1.11 could be confused with container index. However,
x/tools/go/ssa does have it fixed in x/tools@0.1.12 (see golang/go#52834).
This change upgrades the required version to ensure that fewer users of
message/pipelines have issues.

Updates golang/go#54086

Change-Id: I03882a7bb2c75a8f16ef376d06f2cf714d39e7a2
GitHub-Last-Rev: 351404b
GitHub-Pull-Request: #32
Reviewed-on: https://go-review.googlesource.com/c/text/+/422414
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tim King <taking@google.com>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
xhit pushed a commit to xhit/text that referenced this issue Oct 10, 2022
go get -u golang.org/x/tools
go mod tidy

x/tools/go/ssa is used by message/pipeline. Generics instantiation in
x/tools@0.1.11 could be confused with container index. However,
x/tools/go/ssa does have it fixed in x/tools@0.1.12 (see golang/go#52834).
This change upgrades the required version to ensure that fewer users of
message/pipelines have issues.

Updates golang/go#54086

Change-Id: I03882a7bb2c75a8f16ef376d06f2cf714d39e7a2
GitHub-Last-Rev: 351404b
GitHub-Pull-Request: golang#32
Reviewed-on: https://go-review.googlesource.com/c/text/+/422414
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tim King <taking@google.com>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) 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
Development

No branches or pull requests

5 participants