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

go/types: nil pointer dereference in go/types.(*StdSizes).Sizeof #62167

Closed
aarzilli opened this issue Aug 20, 2023 · 7 comments
Closed

go/types: nil pointer dereference in go/types.(*StdSizes).Sizeof #62167

aarzilli opened this issue Aug 20, 2023 · 7 comments
Labels
FrozenDueToAge

Comments

@aarzilli
Copy link
Contributor

aarzilli commented Aug 20, 2023 •
edited
Loading

Does not affect go1.21, only go built from tip.

go env Output
$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/Users/builduser/Library/Caches/go-build'
GOENV='/Users/builduser/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/builduser/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/builduser/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/builduser/blah/go-tip'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/builduser/blah/go-tip/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='devel go1.22-5fa4aac0ce Wed Aug 16 21:00:48 2023 +0000'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/builduser/blah/delve/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/2d/tdsmbmms1czc87g6g54t022w0000gp/T/go-build2349765789=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

$ git clone github.com/go-delve/delve
$ cd delve
$ go run _scripts/gen-starlark-bindings.go go -
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x17d178a]

goroutine 13 [running]:
go/types.(*Checker).handleBailout(0xc0009e6000, 0xc000729c30)
        /Users/builduser/blah/go-tip/src/go/types/check.go:339 +0x88
panic({0x18e8de0?, 0x1a75700?})
        /Users/builduser/blah/go-tip/src/runtime/panic.go:765 +0x132
go/types.(*StdSizes).Sizeof(0x0, {0x1926ac0?, 0x1a76360})
        /Users/builduser/blah/go-tip/src/go/types/sizes.go:228 +0x2ea
go/types.(*Config).sizeof(...)
        /Users/builduser/blah/go-tip/src/go/types/sizes.go:331
go/types.representableConst.func1({0x1926ac0?, 0x1a76360?})
        /Users/builduser/blah/go-tip/src/go/types/const.go:76 +0x9e
go/types.representableConst({0x1927ac0, 0x1a6e098}, 0xc0009e6000, 0x1a76360, 0x0)
        /Users/builduser/blah/go-tip/src/go/types/const.go:92 +0x192
go/types.(*Checker).arrayLength(0xc0009e6000, {0x19276c8?, 0xc0004434c0?})
        /Users/builduser/blah/go-tip/src/go/types/typexpr.go:490 +0x2e5
go/types.(*Checker).typInternal(0xc0009e6000, {0x1926f78?, 0xc000a620f0?}, 0x0)
        /Users/builduser/blah/go-tip/src/go/types/typexpr.go:299 +0xc39
go/types.(*Checker).definedType(0x1926ac0?, {0x1926f78?, 0xc000a620f0}, 0x1a76340?)
        /Users/builduser/blah/go-tip/src/go/types/typexpr.go:180 +0x39
go/types.(*Checker).varType(0xc0009e6000, {0x1926f78?, 0xc000a620f0})
        /Users/builduser/blah/go-tip/src/go/types/typexpr.go:145 +0x33
go/types.(*Checker).structType(0xc0009e6000, 0xc0009e2c60, 0xc0008b97a0?)
        /Users/builduser/blah/go-tip/src/go/types/struct.go:113 +0x1aa
go/types.(*Checker).typInternal(0xc0009e6000, {0x1926fa8?, 0xc0000f4228?}, 0xc00097f1f0)
        /Users/builduser/blah/go-tip/src/go/types/typexpr.go:316 +0xeb4
go/types.(*Checker).definedType(0xc000109000?, {0x1926fa8?, 0xc0000f4228}, 0xc000729838?)
        /Users/builduser/blah/go-tip/src/go/types/typexpr.go:180 +0x39
go/types.(*Checker).typeDecl(0xc0009e6000, 0xc000316d70, 0xc0000a2640, 0x0)
        /Users/builduser/blah/go-tip/src/go/types/decl.go:595 +0x630
go/types.(*Checker).objDecl(0xc0009e6000, {0x1928e38, 0xc000316d70}, 0x0?)
        /Users/builduser/blah/go-tip/src/go/types/decl.go:197 +0x9a5
go/types.(*Checker).packageObjects(0xc0009e6000)
        /Users/builduser/blah/go-tip/src/go/types/resolver.go:675 +0x3a5
go/types.(*Checker).checkFiles(0xc0009e6000, {0xc0007f99a0, 0x2, 0x2})
        /Users/builduser/blah/go-tip/src/go/types/check.go:387 +0x21e
go/types.(*Checker).Files(...)
        /Users/builduser/blah/go-tip/src/go/types/check.go:344
golang.org/x/tools/go/packages.(*loader).loadPackage(0xc000124000, 0xc000536e40)
        /Users/builduser/blah/delve/vendor/golang.org/x/tools/go/packages/packages.go:1001 +0x8c5
golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
        /Users/builduser/blah/delve/vendor/golang.org/x/tools/go/packages/packages.go:838 +0x1a9
sync.(*Once).doSlow(0xc000140f98?, 0x1648157?)
        /Users/builduser/blah/go-tip/src/sync/once.go:74 +0xbf
sync.(*Once).Do(...)
        /Users/builduser/blah/go-tip/src/sync/once.go:65
golang.org/x/tools/go/packages.(*loader).loadRecursive(0x1680ebc?, 0xc000120420?)
        /Users/builduser/blah/delve/vendor/golang.org/x/tools/go/packages/packages.go:826 +0x4a
golang.org/x/tools/go/packages.(*loader).refine.func2(0x0?)
        /Users/builduser/blah/delve/vendor/golang.org/x/tools/go/packages/packages.go:761 +0x26
created by golang.org/x/tools/go/packages.(*loader).refine in goroutine 1
        /Users/builduser/blah/delve/vendor/golang.org/x/tools/go/packages/packages.go:760 +0xc9a
exit status 2

Bisect says the problem started with 5fa4aac however looking at the code I don't see how this could happen and it might be a compiler bug instead.
@dmitshur
Copy link
Contributor

CC @cuonglm.

@mvdan
Copy link
Member

mvdan commented Aug 20, 2023

This is fixed in master after https://go-review.googlesource.com/c/tools/+/516917.

We need an x/tools release to ensure that more downstreams pick up this important fix, however. cc @golang/tools-team

@aarzilli
Copy link
Contributor Author

https://go-review.googlesource.com/c/tools/+/516917 does indeed fix the problem. I'm closing this, tell me if you want it reopened.

@findleyr
Copy link
Member

We need an x/tools release to ensure that more downstreams pick up this important fix, however.

FWIW, we now have automated tagging of x-repos monthly, and it's still pretty early in the Go dev cycle, so IMO is should be OK to wait for the normal tagging to occur in ~the first week of September.

@mvdan
Copy link
Member

mvdan commented Aug 21, 2023

I think that's fair enough in general. I think I just got particularly surprised by this panic, since daily driving Go tip/master is usually fairly stable, and this change broke practically every Go tool in a bad way :) All other bad breakages I can think of in the past few years were direct regressions that led to quick reverts, whereas this is an indirect and intentional breakage that will take weeks or months to get a release and downstream dependency version bumps.

I'm not disagreeing with @findleyr's conclusion, for what it's worth. I'm only clarifying why this breakage might linger on in the form of more bug reports in the coming weeks.

@findleyr
Copy link
Member

@mvdan I agree that the blast radius of this change has been unfortunately large. Arguably it would have been cleaner to tag a fixed version of x/tools (and update x-repos recursively), and then let that change soak a bit before proceeding with the breaking change in Go.

@aarzilli
Copy link
Contributor Author

A few years ago there was talk of moving go/packages into the standard library. It eventually ceased but maybe it was correct and it should have been done?

@cuonglm cuonglm mentioned this issue Dec 20, 2023
Closed
@dancavallaro dancavallaro mentioned this issue Feb 8, 2024
Closed
@flier flier mentioned this issue Feb 20, 2024
Closed
This was referenced Mar 10, 2024
Closed
Merged
@varungandhi-src varungandhi-src mentioned this issue Mar 19, 2024
Merged
@Groxx Groxx mentioned this issue Apr 17, 2024
Merged
Groxx added a commit to cadence-workflow/cadence that referenced this issue Apr 17, 2024
@Groxx
Bugfix for enumer in go 1.22 (#5915)
d0661b7 
Enumer is currently crashing if `make go-generate` is run with Go 1.22: alvaroloes/enumer#71
Temporal helpfully found an easy fix: temporalio/sdk-go#1382
For the underlying cause: golang/go#62167
So I've mimicked that by just doing a `go get golang.org/x/tools@latest` in the tools-module.

And now `make clean` -> `GOTOOLCHAIN=[go1.20.1 or go1.22.1] make go-generate` both work correctly.
(you need Go 1.21 or newer to use GOTOOLCHAIN.  highly recommended!)

Since this only affects build-time tools and doesn't change any generated code, it seems trivially safe, but I have not checked what all has changed in golang.org/x/tools across these versions.
@hrdtbs hrdtbs mentioned this issue Apr 23, 2024
Closed
mhotan added a commit to EngHabu/mockery that referenced this issue May 5, 2024
@mhotan
Upgrade x/tools to v0.20.0
2ef6d74 
* Upgrade to later version of x/tools that does not depend on StdSizes.
* Dependency on StdSizes introduces fragility in client libraries that
  introduces risk of Nil pointer exceptions

Related bugs and issues:
* golang/go#62167
* https://go-review.googlesource.com/c/tools/+/516917
@mhotan mhotan mentioned this issue May 5, 2024
Merged
mhotan added a commit to EngHabu/mockery that referenced this issue May 7, 2024
@mhotan
Upgrade x/tools to v0.20.0 (#3)
4d0a989 
* Upgrade to later version of x/tools that does not depend on StdSizes.
* Dependency on StdSizes introduces fragility in client libraries that
  introduces risk of Nil pointer exceptions

Related bugs and issues:
* golang/go#62167
* https://go-review.googlesource.com/c/tools/+/516917
@estesp estesp mentioned this issue May 13, 2024
Merged
@golang golang locked and limited conversation to collaborators Aug 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@aarzilli @dmitshur @mvdan @gopherbot @findleyr