go/types: vet and test fail to typecheck valid package using unsafe.Sizeof

[zephyrtronium]

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

$ go version
go version go1.20.4 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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/branden/.cache/go-build"
GOENV="/home/branden/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/branden/go/pkg/mod"
GONOPROXY="*.example.net"
GONOSUMDB="*.example.net"
GOOS="linux"
GOPATH="/home/branden/go"
GOPRIVATE="*.example.net"
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.20.4"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/branden/work/example/arapi/go.mod"
GOWORK="/home/branden/work/example/go.work"
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2431293305=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Statically asserted the size of a generated struct to ensure that I wouldn't forget to update tests if the definition from which it is generated changes:

package model

type Invoice struct {
	Num       int                  `json:"num"`       // num
	Ot        sql.NullString       `json:"ot"`        // ot
	Oi        sql.NullString       `json:"oi"`        // oi
	Cust      int64                `json:"cust"`      // cust
	Store     sql.NullString       `json:"store"`     // store
	Status    sql.NullString       `json:"status"`    // status
	Afe       sql.NullString       `json:"afe"`       // afe
	Cc        sql.NullString       `json:"cc"`        // cc
	Pono      sql.NullString       `json:"pono"`      // pono
	Invdate   sql.NullTime         `json:"invdate"`   // invdate
	Svcdate   sql.NullTime         `json:"svcdate"`   // svcdate
	Reqdate   sql.NullTime         `json:"reqdate"`   // reqdate
	Shipdate  sql.NullTime         `json:"shipdate"`  // shipdate
	Siteid    NullUniqueidentifier `json:"siteid"`    // siteid
	Site      sql.NullString       `json:"site"`      // site
	Deptid    NullUniqueidentifier `json:"deptid"`    // deptid
	Dept      sql.NullString       `json:"dept"`      // dept
	Pbid      NullUniqueidentifier `json:"pbid"`      // pbid
	Pbno      sql.NullString       `json:"pbno"`      // pbno
	Pb        sql.NullString       `json:"pb"`        // pb
	Shiptoid  sql.NullString       `json:"shiptoid"`  // shiptoid
	Shipto    sql.NullString       `json:"shipto"`    // shipto
	Shipaddr1 sql.NullString       `json:"shipaddr1"` // shipaddr1
	Shipaddr2 sql.NullString       `json:"shipaddr2"` // shipaddr2
	Shipaddr3 sql.NullString       `json:"shipaddr3"` // shipaddr3
	Shipaddr4 sql.NullString       `json:"shipaddr4"` // shipaddr4
	Total     sql.NullFloat64      `json:"total"`     // total
	Subtotal  sql.NullFloat64      `json:"subtotal"`  // subtotal
	Statetax  sql.NullFloat64      `json:"statetax"`  // statetax
	Countytax sql.NullFloat64      `json:"countytax"` // countytax
	Citytax   sql.NullFloat64      `json:"citytax"`   // citytax
	Othertax  sql.NullFloat64      `json:"othertax"`  // othertax
	Pbcheck   sql.NullTime         `json:"pbcheck"`   // pbcheck
	Descr     sql.NullString       `json:"descr"`     // descr
	// xo fields
	_exists, _deleted bool
}

package front

var _ = [1]struct{}{}[unsafe.Sizeof(model.Invoice{})-587]

Then build, vet, and test:

$ go build ./front/
$ go vet ./front/
$ go test ./front/

What did you expect to see?

PASS.

What did you see instead?

• go build succeeds.
• go vet reports vet: front/order_test.go:262:23: invalid argument: index 160 out of bounds [0:1].
• go test reports front/order_test.go:262:23: invalid argument: index 197 out of bounds [0:1].

Likely relates to #40322, in particular #40322 (comment).

[mknyszek]

CC @griesemer @findleyr

[findleyr]

I'm surprised that we get different results for vet and test; presumably they should be using the same Sizes implementation.

[zephyrtronium]

I found some more time to work on a better reproducer and realized that go build ./front/ works because it doesn't include front/order_test.go. Including the size check in a file that is built with the package makes go build produce the same message as go test, because it is an error while compiling the package, while go vet continues to report the same error it had.

The correct constant is 784 rather than 587; using this makes go build succeed again, while go vet and go test then both report constant -37 ... overflows uintptr. Presumably this comes from vet in both invocations.

For the issue with vet, here is a smaller reproducer:

package issue60431

import (
	"unsafe"
)

type Bug struct {
	_ int32
	_ bool
}

var _ = [1]struct{}{}[unsafe.Sizeof(Bug{})-8]

Here go build succeeds while go vet and go test both report unsafe.Sizeof(Bug{}) - 8 (constant -3 of type uintptr) overflows uintptr. In other words, the issue is that go vet's types.Sizes implementation is not including end-of-struct padding to alignment in Sizeof. Considering #40322 (comment), I think this is indeed a duplicate of #40322.

[griesemer]

The default size computation of go/types doesn't match the compiler, and one of the things that is different is the missing end-of-struct padding, as you have found out (I've argued in the past that that padding can lead to quite a bit of waste if such structs are used as elements in array or slices).

Running the following program in the type checker testing framework (which understands the trace built-in):

package p

import "unsafe"

type S struct {
	_ int32
	_ bool
}

var _ = trace(unsafe.Sizeof(S{}))

prints:

testdata/manual.go:10:15: unsafe.Sizeof(S{}) (constant 5 of type uintptr)

That is, the size of S is 5 bytes as one would expect for a 32bit int and a bool field.

The "correct" fix is for go vet to provide a go/types.Sizes implementation that matches the compiler size computations.

[gopherbot]

Change https://go.dev/cl/501495 mentions this issue: go/analysis: add Sizes that matches size computations

[gopherbot]

Change https://go.dev/cl/501475 mentions this issue: go/types,types2: fix calc sizeof struct type

[gopherbot]

Change https://go.dev/cl/506715 mentions this issue: go/types, types2: add Sizes that match actual gc behavior

[gopherbot]

Change https://go.dev/cl/506856 mentions this issue: go/types, types2: add Sizes that match actual gc behavior