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

cmd/vet, x/tools/go/analysis/passes/testinggoroutine: panic during processing of hash/crc32 package #48124

Closed
dmitshur opened this issue Sep 1, 2021 · 6 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. release-blocker
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Sep 1, 2021

There is a problem on tip when the vendored x/tools module is updated to its latest master pseudo-version (as part of #36905).

What did you do?

$ git clone https://go.googlesource.com/go gotip && cd gotip/src
$ ./make.bash
$ export PATH="$(pwd)/../bin:$PATH"
$ (cd cmd; go get -d golang.org/x/tools@master && go mod tidy && go mod vendor)
$ go install cmd/vet
$ go vet hash/crc32

What did you expect to see?

$ go vet hash/crc32
$ echo $?
0

What did you see instead?

$ go vet hash/crc32 
# hash/crc32
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x12463b8]

goroutine 72 [running]:
cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine.goStmtFun(...)
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine/testinggoroutine.go:130
cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine.checkGoStmt(0xc0001e48f0, 0xc000220a50)
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine/testinggoroutine.go:142 +0x58
cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine.run.func1.1({0x1310528, 0xc000220a50})
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine/testinggoroutine.go:76 +0x36
go/ast.inspector.Visit(0xc000053200, {0x1310528, 0xc000220a50})
	/Users/gopher/gotip/src/go/ast/walk.go:387 +0x31
go/ast.Walk({0x130c7c0, 0xc000053200}, {0x1310528, 0xc000220a50})
	/Users/gopher/gotip/src/go/ast/walk.go:52 +0x62
go/ast.walkStmtList({0x130c7c0, 0xc000053200}, {0xc0002252c0, 0x3, 0x0})
	/Users/gopher/gotip/src/go/ast/walk.go:32 +0x91
go/ast.Walk({0x130c7c0, 0xc000053200}, {0x1310140, 0xc000228420})
	/Users/gopher/gotip/src/go/ast/walk.go:235 +0x1945
go/ast.Walk({0x130c7c0, 0xc000053200}, {0x1310460, 0xc000228450})
	/Users/gopher/gotip/src/go/ast/walk.go:358 +0x85c
go/ast.Inspect(...)
	/Users/gopher/gotip/src/go/ast/walk.go:399
cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine.run.func1({0x1310460, 0xc000228450}, 0xaa)
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine/testinggoroutine.go:70 +0xa5
cmd/vendor/golang.org/x/tools/go/ast/inspector.(*Inspector).Nodes(0xc0000a4810, {0xc000538cc8, 0x7, 0x105c47c}, 0xc000538cb8)
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/ast/inspector/inspector.go:100 +0x9c
cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine.run(0xc0001e48f0)
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine/testinggoroutine.go:58 +0xc5
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func5.1()
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:352 +0x82e
sync.(*Once).doSlow(0x1270b00, 0xc0004f1ad0)
	/Users/gopher/gotip/src/sync/once.go:68 +0xc2
sync.(*Once).Do(...)
	/Users/gopher/gotip/src/sync/once.go:59
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func5(0x14914e0)
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:304 +0x1a5
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func6.1(0x0)
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:364 +0x29
created by cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func6
	/Users/gopher/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:363 +0x47
@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Analysis Issues related to static analysis (vet, x/tools/go/analysis) labels Sep 1, 2021
@dmitshur dmitshur added this to the Go1.18 milestone Sep 1, 2021
@dmitshur
Copy link
Contributor Author

dmitshur commented Sep 1, 2021

Bisection shows this problem is introduced in CL 338529:

$ (cd cmd; go get -d golang.org/x/tools@3fce476f0a782aeb5034d592c189e63be4ba6c9e && go mod tidy && go mod vendor)  # its parent commit
$ go install cmd/vet
$ go vet hash/crc32
$ # ok
$ (cd cmd; go get -d golang.org/x/tools@8fae06a8855e36cf89c791f437aafe2cf3933be5 && go mod tidy && go mod vendor)
$ go install cmd/vet
$ go vet hash/crc32
# hash/crc32
panic: runtime error: [...]

CC @cuonglm, @taking, @odeke-em.

@cuonglm cuonglm self-assigned this Sep 1, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347089 mentions this issue: go/analysis/passes/testinggoroutine: fix panic in goStmtFun

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347191 mentions this issue: all: update vendored dependencies for Go 1.18

gopherbot pushed a commit that referenced this issue Sep 2, 2021
Go 1.18 development is well underway. This is a time to update all
golang.org/x/... module versions that contribute packages to the std
and cmd modules in the standard library to latest master versions.

	gotip $ updatestd -goroot=$(pwd) -branch=master
	> go version
	go version devel go1.18-2872496ba5 Wed Sep 1 23:41:53 2021 +0000 darwin/amd64
	> go env GOROOT
	/Users/dmitshur/gotip
	> go version -m /Users/dmitshur/go/bin/bundle
	/Users/dmitshur/go/bin/bundle: go1.17
		path	golang.org/x/tools/cmd/bundle
		mod	golang.org/x/tools	v0.1.5	h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA=
		dep	golang.org/x/mod	v0.4.2	h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
		dep	golang.org/x/sys	v0.0.0-20210510120138-977fb7262007	h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
		dep	golang.org/x/xerrors	v0.0.0-20200804184101-5ec99f83aff1	h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=

	skipping github.com/chzyer/logex (out of scope, it's not a golang.org/x dependency)
	skipping github.com/chzyer/readline (out of scope, it's not a golang.org/x dependency)
	skipping github.com/chzyer/test (out of scope, it's not a golang.org/x dependency)
	skipping github.com/google/pprof (out of scope, it's not a golang.org/x dependency)
	skipping github.com/ianlancetaylor/demangle (out of scope, it's not a golang.org/x dependency)
	skipping github.com/yuin/goldmark (out of scope, it's not a golang.org/x dependency)
	skipping golang.org/x/tools (temporarily out of scope due to golang.org/issue/48124)
	skipping rsc.io/pdf (out of scope, it's not a golang.org/x dependency)
	updating module cmd in /Users/dmitshur/gotip/src/cmd
	> go mod edit -go=1.18
	> go get -d golang.org/x/arch@ebb09ed340f18f7e2a2200f1adf792992c448346 golang.org/x/crypto@32db794688a5a24a23a43f2a984cecd5b3d8da58 golang.org/x/mod@1b1db11ec8f43eeafa9418698423dc637655ff0c golang.org/x/net@e898025ed96aa6d08e98132b8dca210e9e7a0cd2 golang.org/x/sync@036812b2e83c0ddf193dd5a34e034151da389d09 golang.org/x/sys@f4d43177bf5e2ee98617956e417d0555d4b69c17 golang.org/x/term@6886f2dfbf5b25f595b4fe4279c49956e867c59b golang.org/x/text@383b2e75a7a4198c42f8f87833eefb772868a56f golang.org/x/xerrors@5ec99f83aff198f5fbd629d6c8d8eb38a04218ca
	go get: upgraded golang.org/x/arch v0.0.0-20210502124803-cbf565b21d1e => v0.0.0-20210901143047-ebb09ed340f1
	go get: upgraded golang.org/x/mod v0.5.1-0.20210827163434-4029241eb1d5 => v0.5.1-0.20210830214625-1b1db11ec8f4
	go get: upgraded golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 => v0.0.0-20210825183410-e898025ed96a
	go get: upgraded golang.org/x/term v0.0.0-20210503060354-a79de5458b56 => v0.0.0-20210615171337-6886f2dfbf5b
	go get: upgraded golang.org/x/text v0.3.3 => v0.3.7
	> go mod tidy
	> go mod vendor

	skipping golang.org/x/tools (temporarily out of scope due to golang.org/issue/48124)
	updating module std in /Users/dmitshur/gotip/src
	> go mod edit -go=1.18
	> go get -d golang.org/x/crypto@32db794688a5a24a23a43f2a984cecd5b3d8da58 golang.org/x/net@e898025ed96aa6d08e98132b8dca210e9e7a0cd2 golang.org/x/sys@f4d43177bf5e2ee98617956e417d0555d4b69c17 golang.org/x/term@6886f2dfbf5b25f595b4fe4279c49956e867c59b golang.org/x/text@383b2e75a7a4198c42f8f87833eefb772868a56f
	go get: upgraded golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 => v0.0.0-20210615171337-6886f2dfbf5b
	go get: upgraded golang.org/x/text v0.3.7-0.20210503195748-5c7c50ebbd4f => v0.3.7
	> go mod tidy
	> go mod vendor

	updating bundles in /Users/dmitshur/gotip/src
	> go generate -run=bundle std cmd

The x/tools module will be updated in a following CL,
after issue #48124 is resolved.

The module in GOROOT/src/crypto/ed25519/internal/edwards25519/field/_asm
directory is not updated in this CL.

For #36905.

Change-Id: I728000e8465c0fbf6976629e6da42cc4f9be20fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/347191
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
@timothy-king
Copy link
Contributor

@dmitshur I believe 338529 is 1.18 only so we will not need to backport a fix. Can you confirm this?

@dmitshur
Copy link
Contributor Author

dmitshur commented Sep 2, 2021

@timothy-king Correct. Go 1.17 and 1.16 do not have this problem.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 2, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/348409 mentions this issue: all: update vendored golang.org/x/tools

gopherbot pushed a commit that referenced this issue Sep 8, 2021
Now that issue #48124 is resolved, ran the
following commands inside the cmd module:

	go get -d golang.org/x/tools@36045662144327e4475f9d356f49ab32ce730049  # main branch
	go mod tidy
	go mod vendor

For #36905.
Updates #48124.

Change-Id: I9dc736c2c5256f7d9e80fd9c52c6725ecf0b8001
Reviewed-on: https://go-review.googlesource.com/c/go/+/348409
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Alexander Rakoczy <alex@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
@rsc rsc unassigned cuonglm Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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. release-blocker
Projects
None yet
Development

No branches or pull requests

4 participants