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

Jump to bottom

cmd/compile: internal compile error when using sync.Pool: mismatched zero/store sizes #39459

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

cmd/compile: internal compile error when using sync.Pool: mismatched zero/store sizes #39459

peterstace opened this issue Jun 8, 2020 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.15

Comments

@peterstace
Copy link

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

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

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

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.

@mariecurried
Copy link

According to bisection, this was fixed by 396833c

@randall77
Copy link
Contributor

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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 12, 2020
@toothrot toothrot added this to the Backlog milestone Jun 12, 2020
@randall77
Copy link
Contributor

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

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

@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.

This was referenced Jun 25, 2020
Closed
Closed
@gopherbot
Copy link

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.

@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 Jun 25, 2020
@golang golang locked and limited conversation to collaborators Jun 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.15
Development

No branches or pull requests
7 participants
@toothrot @dmitshur @ianlancetaylor @peterstace @randall77 @gopherbot @mariecurried