search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.02k stars 17.54k forks source link

cmd/compile/internal/gc: bad store type when compiling an openshift origin package #42568

Closed derekparker closed 3 years ago

derekparker commented 3 years ago

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

$ go version
go version devel +15f01d6ae9 Thu Oct 29 03:23:51 2020 +0000 linux/amd64

This also reproduces on tip.

Does this issue reproduce with the latest release?

No, only on tip.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/deparker/.cache/go-build"
GOENV="/home/deparker/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/deparker/Code/gopath"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/usr/lib/golang"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/golang/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/deparker/Code/go/src/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-build790675458=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  1. git clone github.com/openshift/origin
  2. cd pkg/templateservicebroker/servicebroker
  3. GO111MODULE=auto go build

What did you expect to see?

Successful compilation.

What did you see instead?

# github.com/openshift/origin/pkg/templateservicebroker/servicebroker
panic: bad store type

goroutine 1 [running]:
cmd/compile/internal/amd64.storeByType(0xc00005dec0, 0xc0006f82d0)
        /home/deparker/Code/go/src/cmd/compile/internal/amd64/ssa.go:78 +0x129
cmd/compile/internal/amd64.loadByType(0xc00005dec0, 0xc001a80813)
        /home/deparker/Code/go/src/cmd/compile/internal/amd64/ssa.go:53 +0x38
cmd/compile/internal/amd64.ssaGenValue(0xc001cfaa80, 0xc001a88340)
        /home/deparker/Code/go/src/cmd/compile/internal/amd64/ssa.go:950 +0x6525
cmd/compile/internal/gc.genssa(0xc001c8c6e0, 0xc001cfaa00)
        /home/deparker/Code/go/src/cmd/compile/internal/gc/ssa.go:6319 +0x6eb
cmd/compile/internal/gc.compileSSA(0xc0001522c0, 0x0)
        /home/deparker/Code/go/src/cmd/compile/internal/gc/pgen.go:329 +0x3a5
cmd/compile/internal/gc.compile(0xc0001522c0)
        /home/deparker/Code/go/src/cmd/compile/internal/gc/pgen.go:277 +0x39e
cmd/compile/internal/gc.funccompile(0xc0001522c0)
        /home/deparker/Code/go/src/cmd/compile/internal/gc/pgen.go:220 +0xc5
cmd/compile/internal/gc.Main(0xcb9768)
        /home/deparker/Code/go/src/cmd/compile/internal/gc/main.go:752 +0x34af
main.main()
        /home/deparker/Code/go/src/cmd/compile/main.go:52 +0xb1

Here is some prelim debugging output I've collected about what is causing this panic to trigger:

panic: bad store type

goroutine 15 [running]:
cmd/compile/internal/amd64.storeByType(0xc000051ec0, 0xc0005a62d0)
        /home/deparker/Code/go/src/cmd/compile/internal/amd64/ssa.go:78 +0x129
cmd/compile/internal/amd64.loadByType(0xc000051ec0, 0xc001b00813)
        /home/deparker/Code/go/src/cmd/compile/internal/amd64/ssa.go:53 +0x38
cmd/compile/internal/amd64.ssaGenValue(0xc0018c2d00, 0xc001b01b90)
        /home/deparker/Code/go/src/cmd/compile/internal/amd64/ssa.go:950 +0x6525
cmd/compile/internal/gc.genssa(0xc001dfc000, 0xc0018c2c00)
        /home/deparker/Code/go/src/cmd/compile/internal/gc/ssa.go:6319 +0x6eb
cmd/compile/internal/gc.compileSSA(0xc0000c82c0, 0x0)
        /home/deparker/Code/go/src/cmd/compile/internal/gc/pgen.go:329 +0x3a5
cmd/compile/internal/gc.compileFunctions.func2(0xc001f464e0, 0xc001e27b30, 0x0)
        /home/deparker/Code/go/src/cmd/compile/internal/gc/pgen.go:384 +0x4d
created by cmd/compile/internal/gc.compileFunctions
        /home/deparker/Code/go/src/cmd/compile/internal/gc/pgen.go:382 +0x129
(dlv) c
> [unrecovered-panic] runtime.fatalpanic() /home/deparker/Code/go/src/runtime/panic.go:1185 (hits goroutine(15):1 total:1) (PC: 0x4375a0)
Warning: debugging optimized function
        runtime.curg._panic.arg: interface {}(string) "bad store type"
Current event: 21604
  1180: // fatalpanic implements an unrecoverable panic. It is like fatalthrow, except
  1181: // that if msgs != nil, fatalpanic also prints panic messages and decrements
  1182: // runningPanicDefers once main is blocked from exiting.
  1183: //
  1184: //go:nosplit
=>1185: func fatalpanic(msgs *_panic) {
  1186:         pc := getcallerpc()
  1187:         sp := getcallersp()
  1188:         gp := getg()
  1189:         var docrash bool
  1190:         // Switch to the system stack to avoid any stack growth, which
(dlv) b /home/deparker/Code/go/src/cmd/compile/internal/amd64/ssa.go:78
Breakpoint 1 set at 0xbadc2d for cmd/compile/internal/amd64.storeByType() /home/deparker/Code/go/src/cmd/compile/internal/amd64/ssa.go:78
(dlv) rev continue
> cmd/compile/internal/amd64.storeByType() /home/deparker/Code/go/src/cmd/compile/internal/amd64/ssa.go:78 (hits goroutine(15):1 total:1) (PC: 0xbadc2d)
Warning: debugging optimized function
Current event: 21604
    73:                         return x86.AMOVL
    74:                 case 8:
    75:                         return x86.AMOVQ
    76:                 }
    77:         }
=>  78:         panic("bad store type")
    79: }
    80:
    81: // moveByType returns the reg->reg move instruction of the given type.
    82: func moveByType(t *types.Type) obj.As {
    83:         if t.IsFloat() {
(dlv) args
t = ("*cmd/compile/internal/types.Type")(0xc000051ec0)
~r1 = (unreadable empty OP stack)
(dlv) p t
*cmd/compile/internal/types.Type {
        Extra: interface {}(*cmd/compile/internal/types.Interface) *{
                Fields: (*"cmd/compile/internal/types.Fields")(0xc00004c670),
                pkg: *cmd/compile/internal/types.Pkg nil,},
        Width: 16,
        methods: cmd/compile/internal/types.Fields {
                s: *[]*cmd/compile/internal/types.Field nil,},
        allMethods: cmd/compile/internal/types.Fields {
                s: *[]*cmd/compile/internal/types.Field nil,},
        Nod: *cmd/compile/internal/types.Node {_: 0},
        Orig: *cmd/compile/internal/types.Type {
                Extra: interface {}(*cmd/compile/internal/types.Interface) ...,
                Width: 16,
                methods: (*"cmd/compile/internal/types.Fields")(0xc000051ed8),
                allMethods: (*"cmd/compile/internal/types.Fields")(0xc000051ee0),
                Nod: *(*"cmd/compile/internal/types.Node")(0xc0005de500),
                Orig: *(*"cmd/compile/internal/types.Type")(0xc000051ec0),
                Cache: (*"struct { cmd/compile/internal/types.ptr *cmd/compile/internal/types.Type; cmd/compile/internal/types.slice *cmd/compile/internal/types.Type }")(0xc000051ef8),
                Sym: *cmd/compile/internal/types.Sym nil,
                Vargen: 0,
                Etype: cmd/compile/internal/gc.TINTER (24),
                Align: 8,
                flags: 0,},
        Cache: struct { cmd/compile/internal/types.ptr *cmd/compile/internal/types.Type; cmd/compile/internal/types.slice *cmd/compile/internal/types.Type } {
                ptr: *(*"cmd/compile/internal/types.Type")(0xc001aa96e0),
                slice: *(*"cmd/compile/internal/types.Type")(0xc001aa97a0),},
        Sym: *cmd/compile/internal/types.Sym nil,
        Vargen: 0,
        Etype: cmd/compile/internal/gc.TINTER (24),
        Align: 8,
        flags: 0,}

I did a little bit of digging and the problem seems to be that the compiler is not properly decomposing this store into two smaller stores for some reason. I've bisected the tree from 1.15.4 where this worked and I've found the commit that introduced this bug to be https://github.com/golang/go/commit/15f01d6ae9853fd51ee8842d9af93d04ce25458c.

I'm happy to continue working on this and submit a fix, I've spent some time tracking this down and figured I'd at least create an issue and see if there are any obvious things to look at where things may be going wrong from folks who are more familiar with the actual SSA code than I am at the present moment.

derekparker commented 3 years ago

cc @cherrymui @dr2chase @randall77

cuonglm commented 3 years ago

Here's smaller reproducible program:

package p

type S struct{}

func (S) M() {}

type I interface {
    M()
}

func f(i I) {
    o := i.(interface{})
    if _, ok := i.(*S); ok {
        o = nil
    }
    println(o)
}

Somehow, expand_calls does not handle OpSelectN correctly.

derekparker commented 3 years ago

Here's some more debug output using the small reproducer above: 

expandsCalls((*S).M)
v2p[v10 = SelectN <mem> [0] v9] = 0

expandsCalls(f)
allOrdered[0] = b3, v10 = SelectN <mem> [0] v9, uses=1
rewriteSelect(v10 = SelectN <mem> [0] v9, v10 = SelectN <mem> [0] v9, 0)
storeArg v7 = Addr <*uint8> {type.interface {}} v3, *uint8, 0
        storeArgOrLoad(v41 = SP <uintptr>;  v7 = Addr <*uint8> {type.interface {}} v3;  v1;  *uint8; 0)
                storeArg returns v28 = Store <mem> {*uint8} v30 v7 v1
storeArg v33 = IMake <interface {}> v27 v9, interface {}, 0
        storeArgOrLoad(v41 = SP <uintptr>;  v33 = IMake <interface {}> v27 v9;  v32;  interface {}; 0)
        storeArgOrLoad(v41 = SP <uintptr>;  v27 = Phi <uintptr> v16 v29 (o.type[uintptr]);  v32;  uintptr; 0)

expandsCalls(main)
                storeArg returns v24 = Store <mem> {uintptr} v40 v27 v32
        storeArgOrLoad(v41 = SP <uintptr>;  v9 = Phi <*uint8> v17 v25 (o.data[*uint8]);  v24;  *uint8; 8)
                storeArg returns v11 = Store <mem> {*uint8} v13 v9 v24
v2p[v5 = Arg <I> {i} (i[I])] = 0
v2p[v5 = Arg <I> {i} (i[I])] = 1
v2p[v8 = ITab <*uint8> v5] = 0
v2p[v5 = Arg <I> {i} (i[I])] = 2
v2p[v17 = IData <interface {}> v5 (o.data[*uint8])] = 0
v2p[v32 = SelectN <mem> [0] v31] = 0
v2p[v35 = SelectN <mem> [0] v34] = 0
v2p[v37 = SelectN <mem> [0] v36] = 0
v2p[v39 = SelectN <mem> [0] v38] = 0
allOrdered[0] = b7, v32 = SelectN <mem> [0] v31, uses=1
rewriteSelect(v32 = SelectN <mem> [0] v31, v32 = SelectN <mem> [0] v31, 0)
allOrdered[1] = b7, v35 = SelectN <mem> [0] v34, uses=1
rewriteSelect(v35 = SelectN <mem> [0] v34, v35 = SelectN <mem> [0] v34, 0)
allOrdered[2] = b7, v37 = SelectN <mem> [0] v36, uses=1
rewriteSelect(v37 = SelectN <mem> [0] v36, v37 = SelectN <mem> [0] v36, 0)
allOrdered[3] = b7, v39 = SelectN <mem> [0] v38, uses=1

expandsCalls(S.M)
storeArg v13 = Addr <*uint8> {type.interface {}} v3, *uint8, 0
        storeArgOrLoad(v2 = SP <uintptr>;  v13 = Addr <*uint8> {type.interface {}} v3;  v5;  *uint8; 0)
                storeArg returns v15 = Store <mem> {*uint8} v7 v13 v5
rewriteSelect(v39 = SelectN <mem> [0] v38, v39 = SelectN <mem> [0] v38, 0)
storeArg v39 = IMake <interface {}> v28 v33, interface {}, 0
allOrdered[4] = b2, v17 = IData <interface {}> v5 (o.data[*uint8]), uses=2
        storeArgOrLoad(v2 = SP <uintptr>;  v39 = IMake <interface {}> v28 v33;  v38;  interface {}; 0)
        storeArgOrLoad(v2 = SP <uintptr>;  v28 = Phi <uintptr> v22 v35 (o.type[uintptr]);  v38;  uintptr; 0)
                storeArg returns v4 = Store <mem> {uintptr} v23 v28 v38
        storeArgOrLoad(v2 = SP <uintptr>;  v33 = Phi <*uint8> v8 v31 (o.data[*uint8]);  v4;  *uint8; 8)
                storeArg returns v14 = Store <mem> {*uint8} v6 v33 v4
v2p[v38 = SelectN <mem> [0] v37] = 0
v2p[v41 = SelectN <mem> [0] v40] = 0
v2p[v43 = SelectN <mem> [0] v42] = 0
v2p[v45 = SelectN <mem> [0] v44] = 0
allOrdered[0] = b9, v38 = SelectN <mem> [0] v37, uses=1
rewriteSelect(v38 = SelectN <mem> [0] v37, v38 = SelectN <mem> [0] v37, 0)
allOrdered[1] = b9, v41 = SelectN <mem> [0] v40, uses=1
rewriteSelect(v41 = SelectN <mem> [0] v40, v41 = SelectN <mem> [0] v40, 0)
allOrdered[2] = b9, v43 = SelectN <mem> [0] v42, uses=1
rewriteSelect(v43 = SelectN <mem> [0] v42, v43 = SelectN <mem> [0] v42, 0)
allOrdered[3] = b9, v45 = SelectN <mem> [0] v44, uses=1
rewriteSelect(v45 = SelectN <mem> [0] v44, v45 = SelectN <mem> [0] v44, 0)

expandsCalls(I.M)
storeArg v14 = Load <*uint8> v15 v1, uintptr, 0
        storeArgOrLoad(v2 = SP <uintptr>;  v14 = Load <*uint8> v15 v1;  v1;  uintptr; 0)
                storeArg returns v16 = Store <mem> {uintptr} v6 v14 v1
v2p[v13 = SelectN <mem> [0] v12] = 0
allOrdered[0] = b1, v13 = SelectN <mem> [0] v12, uses=1
rewriteSelect(v13 = SelectN <mem> [0] v12, v13 = SelectN <mem> [0] v12, 0)
rewriteSelect(v17 = IData <interface {}> v5 (o.data[*uint8]), v17 = IData <interface {}> v5 (o.data[*uint8]), 0)
rewriteSelect(v17 = IData <interface {}> v5 (o.data[*uint8]), v5 = Arg <I> {i} (i[I]), 8)
        new v19 = Arg <interface {}> {i} [8]
allOrdered[5] = b1, v8 = ITab <*uint8> v5, uses=3
rewriteSelect(v8 = ITab <*uint8> v5, v8 = ITab <*uint8> v5, 0)
rewriteSelect(v8 = ITab <*uint8> v5, v5 = Arg <I> {i} (i[I]), 0)
allOrdered[6] = b1, v5 = Arg <I> {i} (i[I]), uses=0
gopherbot commented 3 years ago

Change https://golang.org/cl/270057 mentions this issue: cmd/compile: fix load of interface{}-typed OpIData in expand_calls

gopherbot commented 3 years ago

Change https://golang.org/cl/276952 mentions this issue: cmd/compile: set correct type for OpIData

josharian commented 3 years ago

(Further comment here that I wanted to leave on the CL, but gerrit is almost completely non-functional on Firefox on mobile.)

Specifically I think you should run compilecmp -fn=changed. Also, this reminds me of https://go-review.googlesource.com/c/go/+/229984. Please see Keith’s comment there.

Thanks for digging to find a root cause here.