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 compiler error: Op...LECall and OpDereference have mismatched mem #49282

Closed
ALTree opened this issue Nov 2, 2021 · 9 comments
Labels
NeedsInvestigation release-blocker
Milestone

Comments

@ALTree
Copy link
Member

@ALTree ALTree commented Nov 2, 2021

$ gotip version
go version devel go1.18-088bb4bf4a Tue Nov 2 06:25:39 2021 +0000 windows/amd64
package p

func f[G uint]() {
	var a, m []int
	var s struct {
		a, b, c, d, e int
	}

	func(G) {
		_ = a
		_ = m
		_ = s
		func() {
			for i := 0; i < 5; i++ {
				_ = a
				_ = m
				_, _ = s, s
			}
		}()
	}(G(1.0))

	defer func() uint {
		return 0
	}()
}

func g() {
	f[uint]()
}
$ gotip tool compile crash.go

crash.go:22:2: internal compiler error: 'f[%2eshape.uint_0]': Op...LECall and OpDereference have mismatched mem, v26 = StaticLECall <mem> {AuxCall{"".f[%2eshape.uint_0].func1}} [104] v9 v10 v10 v15 v19 v59 and v15 = Dereference <struct { a int; b int; c int; d int; e int }> v14 v13

goroutine 1 [running]:
runtime/debug.Stack()
        D:/users/f65362c/alberto/other/gotip/src/runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0x1a6cc00?, 0x0?}, {0xc00001c910, 0x42}, {0xc0003f7620, 0x3, 0x3})
        D:/users/f65362c/alberto/other/gotip/src/cmd/compile/internal/base/print.go:227 +0x1ca
cmd/compile/internal/base.Fatalf(...)
        D:/users/f65362c/alberto/other/gotip/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/ssagen.(*ssafn).Fatalf(0x8?, {0x8e70108?, 0x2ad?}, {0x1b161bb, 0x3c}, {0xc0003c9e20, 0x2, 0x0?})
        D:/users/f65362c/alberto/other/gotip/src/cmd/compile/internal/ssagen/ssa.go:7650 +0x17d
cmd/compile/internal/ssa.(*Func).Fatalf(0xc0004061c0, {0x1b161bb, 0x3c}, {0xc0003c9e20, 0x2, 0x2})
        D:/users/f65362c/alberto/other/gotip/src/cmd/compile/internal/ssa/func.go:773 +0x279
cmd/compile/internal/ssa.(*expandState).rewriteArgs(0xc0000797e8, 0xc000480b60, 0x0)
        D:/users/f65362c/alberto/other/gotip/src/cmd/compile/internal/ssa/expand_calls.go:1099 +0x53e
...

Tentatively labeling as a release blocker since it's a compiler crasher on a valid program with typeparameters.

cc @randall77 @danscales

@ALTree ALTree added NeedsInvestigation release-blocker labels Nov 2, 2021
@ALTree ALTree added this to the Go1.18 milestone Nov 2, 2021
@randall77
Copy link
Contributor

@randall77 randall77 commented Nov 2, 2021

Looks like there is a vardef/varlive pair that is masking the fact that the two memory args are the same. The expand_calls pass doesn't like that for some reason.

@drchase can you look at this? I don't think it is directly related to generics.

@cuonglm
Copy link
Member

@cuonglm cuonglm commented Nov 4, 2021

Looks like there is a vardef/varlive pair that is masking the fact that the two memory args are the same. The expand_calls pass doesn't like that for some reason.

@drchase can you look at this? I don't think it is directly related to generics.

Seems right, look at the SSA:

f[go.shape.uint_0] func(uintptr)
  b1: DEAD
    BlockInvalid
  b2: DEAD
    BlockInvalid
  b5: DEAD
    BlockInvalid
  b6: DEAD
    BlockInvalid
  b7:
    (?) v1 = InitMem <mem>
    (?) v2 = SP <uintptr>
    (?) v3 = SB <uintptr>
    (?) v4 = Const8 <uint8> [0]
    (-3) v5 = LocalAddr <*uint8> {.autotmp_10} v2 v1
    (+3) v6 = Store <mem> {uint8} v5 v4 v1
    (-3) v7 = VarLive <mem> {.autotmp_10} v6
    (-3) v9 = Arg <uintptr> {.dict} (.dict[uintptr])
    (-5) v11 = VarDef <mem> {s} v7
    (5) v12 = LocalAddr <*struct { a int; b int; c int; d int; e int }> {s} v2 v11
    (+5) v13 = Zero <mem> {struct { a int; b int; c int; d int; e int }} [40] v12 v11
    (21) v14 = LocalAddr <*struct { a int; b int; c int; d int; e int }> {s} v2 v13
    (+21) v15 = Dereference <struct { a int; b int; c int; d int; e int }> v14 v13
    (?) v30 = Const64 <uintptr> [0]
    (?) v35 = Addr <uintptr> {"".f[go.shape.uint_0].func2} v3
    (?) v45 = ConstNil <func() uint>
    (?) v50 = Addr <uintptr> {"".f[go.shape.uint_0].func3} v3
    (?) v58 = VarDef <mem> {.autotmp_11} v13
    (?) v59 = VarLive <mem> {.autotmp_11} v58
    (?) v60 = LocalAddr <*func()> {.autotmp_11} v2 v59
    (?) v72 = ConstNil <*int> (a.ptr[*int], m.ptr[*int])
    (?) v67 = Const64 <int> [0] (a.cap[int], a.len[int], m.cap[int], m.len[int])
    (21) v19 = Const64 <go.shape.uint_0> [1]
    (23) v64 = Const8 <uint8> [1]
    (?) v54 = Const64 <int64> [0] DEAD
    (-3) v51 = ArgIntReg <uintptr> {.dict+0} [0] DEAD
    (?) v10 = SliceMake <[]int> v72 v67 v67
    (21) v26 = StaticLECall <mem> {AuxCall{"".f[go.shape.uint_0].func1}} [104] v9 v10 v10 v15 v19 v59

The mismatch between v59 (is the m0) and v13 (is the a.MemoryArg()) causes the ICE. We do elide the VarDef for results, but not for args.

Can we add this code right after L1084 to find the correct m0?

	for m0.Op == OpVarLive || m0.Op == OpVarDef {
		m0 = m0.MemoryArg()
	}

@hanchaoqun
Copy link
Contributor

@hanchaoqun hanchaoqun commented Nov 5, 2021

I found the cause of the problem: Before the pass of insert phi, the mem chain of the StaticLECall args in the first block is broken by the vardef/varlive pair inserted by openDeferRecord, trying to fix it.

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 5, 2021

Change https://golang.org/cl/361410 mentions this issue: cmd/compile: avoid adding LECall to the entry block when has opendefers

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented Nov 5, 2021

FYI, a no-generics reproducer:

package p

//go:noinline
func g(d uintptr, a, m []int, s struct {
	a, b, c, d, e int
}, u uint) {
	_ = a
	_ = m
	_ = s
	func() {
		for i := 0; i < 5; i++ {
			_ = a
			_ = m
			_, _ = s, s
		}
	}()
}

var One float64 = 1.0

func f(d uintptr) {
	var a, m []int
	var s struct {
		a, b, c, d, e int
	}

	g(d, a, m, s, uint(One)) // Uint of not-a-constant inserts a conditional, necessary to bug

	defer func() uint {
		return 0
	}()
}

var d uintptr

func h() {
	f(d)
}

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented Nov 5, 2021

which also fails in 1.16

@hanchaoqun
Copy link
Contributor

@hanchaoqun hanchaoqun commented Nov 6, 2021

FYI, a no-generics reproducer:

package p

//go:noinline
func g(d uintptr, a, m []int, s struct {
	a, b, c, d, e int
}, u uint) {
	_ = a
	_ = m
	_ = s
	func() {
		for i := 0; i < 5; i++ {
			_ = a
			_ = m
			_, _ = s, s
		}
	}()
}

var One float64 = 1.0

func f(d uintptr) {
	var a, m []int
	var s struct {
		a, b, c, d, e int
	}

	g(d, a, m, s, uint(One)) // Uint of not-a-constant inserts a conditional, necessary to bug

	defer func() uint {
		return 0
	}()
}

var d uintptr

func h() {
	f(d)
}

Thanks for the the non-generic reproducer, i will use your code to replace the test.

@hanchaoqun
Copy link
Contributor

@hanchaoqun hanchaoqun commented Nov 6, 2021

@gopherbot Please open backport to 1.16 and 1.17

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 6, 2021

Backport issue(s) opened: #49412 (for 1.16), #49413 (for 1.17).

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
Labels
NeedsInvestigation release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants