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/analysis: divide by zero in atomicalign.check64BitAlignment #30962

Closed
zeebo opened this issue Mar 20, 2019 · 10 comments
Closed

x/tools/go/analysis: divide by zero in atomicalign.check64BitAlignment #30962

zeebo opened this issue Mar 20, 2019 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@zeebo
Copy link
Contributor

zeebo commented Mar 20, 2019

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

$ go version
go version go1.12.1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jeff/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jeff/go"
GOPROXY=""
GORACE=""
GOROOT="/nix/store/4kvx51wk8sjglkns8bnrragw4hqvssiw-go-1.12.1/share/go"
GOTMPDIR=""
GOTOOLDIR="/nix/store/4kvx51wk8sjglkns8bnrragw4hqvssiw-go-1.12.1/share/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build548840513=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I'm using gopls and I opened a file in a module that contained

package foo

import "sync/atomic"

type T struct {
	f struct {
		y uint64
	}
}

var _ = atomic.LoadUint64(&(&T{}).f.y)

The key part is that it's attempting to access a field inside of an anonymous struct with sync/atomic.

What did you expect to see?

No crash.

What did you see instead?

panic: runtime error: integer divide by zero

goroutine 14 [running]:
go/types.align(...)
	/goroot/src/go/types/sizes.go:256
go/types.(*StdSizes).Offsetsof(0xc00021c370, 0xc0002260e0, 0x1, 0x1, 0x1, 0xc0002260e0, 0x0)
	/goroot/src/go/types/sizes.go:97 +0x144
golang.org/x/tools/go/analysis/passes/atomicalign.check64BitAlignment(0xc0002221b0, 0xc000018e30, 0xa, 0x8fb0c0, 0xc00000ebe0)
	/build/go/src/golang.org/x/tools/go/analysis/passes/atomicalign/atomicalign.go:105 +0x1cd
golang.org/x/tools/go/analysis/passes/atomicalign.run.func1(0x8f50a0, 0xc000030d00)
	/build/go/src/golang.org/x/tools/go/analysis/passes/atomicalign/atomicalign.go:65 +0x16e
golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0xc0000ebd60, 0xc00004ed40, 0x1, 0x1, 0xc0001eed30)
	/build/go/src/golang.org/x/tools/go/ast/inspector/inspector.go:77 +0x9f
golang.org/x/tools/go/analysis/passes/atomicalign.run(0xc0002221b0, 0x0, 0x0, 0x0, 0x0)
	/build/go/src/golang.org/x/tools/go/analysis/passes/atomicalign/atomicalign.go:42 +0x155
golang.org/x/tools/internal/lsp/source.(*Action).execOnce(0xc0000e4420, 0xc0000f0dc0)
	/build/go/src/golang.org/x/tools/internal/lsp/source/analysis.go:156 +0x727
golang.org/x/tools/internal/lsp/source.(*Action).exec.func1()
	/build/go/src/golang.org/x/tools/internal/lsp/source/analysis.go:93 +0x33
sync.(*Once).Do(0xc0000e4420, 0xc00004ef88)
	/goroot/src/sync/once.go:44 +0xb3
golang.org/x/tools/internal/lsp/source.(*Action).exec(0xc0000e4420, 0xc0000f0dc0)
	/build/go/src/golang.org/x/tools/internal/lsp/source/analysis.go:92 +0x63
golang.org/x/tools/internal/lsp/source.execAll.func1(0xc0000e4420)
	/build/go/src/golang.org/x/tools/internal/lsp/source/analysis.go:83 +0x3d
created by golang.org/x/tools/internal/lsp/source.execAll
	/build/go/src/golang.org/x/tools/internal/lsp/source/analysis.go:86 +0x5a
@gopherbot gopherbot added this to the Unreleased milestone Mar 20, 2019
@josharian
Copy link
Contributor

@mvdan

@agnivade agnivade changed the title x/tools: divide by zero in atomicalign.check64BitAlignment x/tools/go/analysis: divide by zero in atomicalign.check64BitAlignment Mar 21, 2019
@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 21, 2019
@mvdan
Copy link
Member

mvdan commented Mar 21, 2019

@arl worked on the analyzer, so perhaps he wants to look into this.

@arl
Copy link
Contributor

arl commented Mar 21, 2019

Ok, you can assign it to me

@mvdan
Copy link
Member

mvdan commented Mar 21, 2019

@zeebo does it happen with go vet too? If so, we might want to consider a backport to keep vet from crashing on valid programs.

@zeebo
Copy link
Contributor Author

zeebo commented Mar 21, 2019

It doesn’t crash go vet.

@arl
Copy link
Contributor

arl commented Jun 30, 2019

Indeed it doesn't crash vet so this should probably be reported to gopls.
However, with the latest gopls I wasn't able to reproduce either (fb37f6ba82613749b0b522aa509da78361849fc3 in x/tools), though I've never used gopls before so I might have done something wrong: I created a project with that file and ensured gopls (via VSCode) was providing type info for it, saw nothing in gopls log.

@zeebo could you check if this is still valid please?

@stamblerre
Copy link
Contributor

stamblerre commented Jul 1, 2019

gopls doesn't work correctly with build tags, so I think that's probably what is happening here. #32413 is likely related.

@arl
Copy link
Contributor

arl commented Jul 1, 2019

gopls doesn't work correctly with build tags

atomicalign vet analyser does use build tags to make the analyser a no-op on non-affected architectures, so yeah that's probably what is happening.

@zeebo
Copy link
Contributor Author

zeebo commented Jul 2, 2019

I can confirm that it doesn't crash for me anymore.

@arl
Copy link
Contributor

arl commented Jul 7, 2019

I believe this issue can be close

@mvdan mvdan closed this as completed Jul 7, 2019
@golang golang locked and limited conversation to collaborators Jul 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants