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

Closed
ALTree opened this issue Nov 2, 2021 · 9 comments
Closed

cmd/compile: internal compiler error: Op...LECall and OpDereference have mismatched mem #49282

ALTree opened this issue Nov 2, 2021 · 9 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone
Go1.18

Comments

@ALTree
Copy link
Member

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 Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Nov 2, 2021
@ALTree ALTree added this to the Go1.18 milestone Nov 2, 2021
@randall77
Copy link
Contributor

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 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 commented Nov 5, 2021 •
edited

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

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 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 commented Nov 5, 2021

which also fails in 1.16

@hanchaoqun
Copy link
Contributor

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

@gopherbot Please open backport to 1.16 and 1.17

@gopherbot gopherbot mentioned this issue Nov 6, 2021
Closed
@gopherbot
Copy link

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.

@gopherbot gopherbot mentioned this issue Nov 6, 2021
Closed
@golang golang locked and limited conversation to collaborators Nov 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
6 participants
@dr2chase @ALTree @cuonglm @randall77 @gopherbot @hanchaoqun