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/compile: internal compile error when using sync.Pool: mismatched zero/store sizes #39459

Closed
peterstace opened this issue Jun 8, 2020 · 9 comments
Labels
Milestone

Comments

@peterstace
Copy link

@peterstace peterstace commented Jun 8, 2020

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

$ go version
go version go1.14.4 linux/amd64

Does this issue reproduce with the latest release?

Yes, go1.14.4 is the latest release (released about a week ago). I've also reproduced this using go1.14.1.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/petsta/.cache/go-build"
GOENV="/home/petsta/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY="github.com/nearmap"
GONOSUMDB="github.com/nearmap"
GOOS="linux"
GOPATH="/home/petsta/go"
GOPRIVATE="github.com/nearmap"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/petsta/bug/go.mod"
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-build913647161=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I attempted to compile the following program, which is a minimum working example that exhibits the problem:

https://play.golang.org/p/C5P2Zh6X_i-

package main

import "sync"

type foo struct {
    bars [2]bar
}

type bar struct{}

var pool = sync.Pool{
    New: func() interface{} {
        return new(foo)
    },
}

func newFoo() *foo {
    f := pool.Get().(*foo)
    *f = foo{}
    return f
}

func main() {
    f := newFoo()
    *f = foo{}
}

What did you expect to see?

The program to compile successfully.

What did you see instead?

The following is emitted from the go toolchain:

./main.go:24:13: internal compiler error: 'main': mismatched zero/store sizes: 0 and 1 [v27 = Zero <mem> {foo} v29 v9]

Please file a bug report including a short program that triggers the error.
https://golang.org/issue/new
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 8, 2020

I can reproduce this with Go 1.12, 1.13, and 1.14, but it is fixed on tip.

@ianlancetaylor ianlancetaylor changed the title Internal compile error when using sync.Pool: mismatched zero/store sizes cmd/compile: internal compile error when using sync.Pool: mismatched zero/store sizes Jun 8, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 8, 2020

Do you really need an array of elements of zero size? Given that this is fixed on tip, I'm trying to judge how important it is to backport a fix to 1.14.

@peterstace
Copy link
Author

@peterstace peterstace commented Jun 8, 2020

Do you really need an array of elements of zero size?

bar only has zero size in the minimum working example -- in the 'real' program bar has many fields (sorry, my minimum working example may have been a bit misleading in that regard).

Given that this is fixed on tip, I'm trying to judge how important it is to backport a fix to 1.14.

For me personally, I'm able to work around the problem by replacing *f = foo{} with code that zeros out each field within foo manually (e.g. f.bars = [2]bar{}). In my situation, this is new code as opposed to existing code that worked in a previous version of Go, so a backport is less important for me.

@marigonzes
Copy link

@marigonzes marigonzes commented Jun 9, 2020

According to bisection, this was fixed by 396833c

@randall77
Copy link
Contributor

@randall77 randall77 commented Jun 9, 2020

Thanks for the bisection.
That CL is just an optimization. It's probably optimizing away the code that triggers the bug, but I suspect there may still be some underlying bug here.

@toothrot toothrot added this to the Backlog milestone Jun 12, 2020
@randall77
Copy link
Contributor

@randall77 randall77 commented Jun 25, 2020

The underlying bug is just a mistaken invariant check. For the opcode (Zero {typ} ptr mem), it checks that the base type of the pointer and the aux type field have the same size. That's not always going to be the case, though. The typ field is authoritative. The ptr field can have a different base type in situations like the OP's program, because the pointer comes from the data portion of a type-casted interface{}. We give the interface data field a byte* type because we don't know its actual base type at the time it is loaded. Only when the cast succeeds do we know the size that needs to be zeroed.

I'll see if I can make a repro that works at tip.

The fix is just to remove the invariant check. It isn't generating any wrong code that I can see. Might be worth auditing all our rules to make sure we're not depending on the types of ssa.Value inputs. Reminds me of #37753.

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 25, 2020

Change https://golang.org/cl/239817 mentions this issue: cmd/compile: remove check that Zero's arg has the correct base type

@randall77 randall77 modified the milestones: Backlog, Go1.15 Jun 25, 2020
@randall77
Copy link
Contributor

@randall77 randall77 commented Jun 25, 2020

@gopherbot please open backport issues for 1.14 and 1.13.

This issue can cause the compiler to crash. The compiler was checking an invariant that doesn't, and does not need to, hold.

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 25, 2020

Backport issue(s) opened: #39848 (for 1.13), #39849 (for 1.14).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.