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: aliasing constant which is not registered #35157

Closed
ALTree opened this issue Oct 25, 2019 · 8 comments
Closed

Comments

@ALTree
Copy link
Member

@ALTree ALTree commented Oct 25, 2019

$ gotip version
go version devel +dad512f6df Fri Oct 25 14:31:54 2019 +0000 linux/amd64

The following program:

package p

func f() {
	var i int
	var b *bool
	var s0, s1, s2 string

	if *b {
		s2 = s2[:1]
		i = 1
	}
	s1 = s1[i:-i+i] + s1[-i+i:i+2]
	s1 = s0[i:-i]
}

Crashes the tip compiler with this error:

$ gotip tool compile crash.go

crash.go:8:5: internal compiler error: 'f': panic during prove while compiling f:

aliasing constant which is not registered

goroutine 1 [running]:
cmd/compile/internal/ssa.Compile.func1(0xc0003cc790, 0xc0002be580)
	/home/alberto/go/src/cmd/compile/internal/ssa/compile.go:47 +0xa5
panic(0xda9a00, 0xfd7670)
	/home/alberto/go/src/runtime/panic.go:909 +0x155
cmd/compile/internal/ssa.(*poset).aliasnode(0xc0003c07e0, 0xc0003729a0, 0xc0003721c0)
	/home/alberto/go/src/cmd/compile/internal/ssa/poset.go:450 +0x71a
cmd/compile/internal/ssa.(*poset).collapsepath(0xc0003c07e0, 0xc0003729a0, 0xc0003721c0, 0x15b9d01)
	/home/alberto/go/src/cmd/compile/internal/ssa/poset.go:618 +0x102
cmd/compile/internal/ssa.(*poset).setOrder(0xc0003c07e0, 0xc0003721c0, 0xc0003729a0, 0x1fc51b00, 0xbef5dd81c848f0f1)
	/home/alberto/go/src/cmd/compile/internal/ssa/poset.go:1034 +0x226
cmd/compile/internal/ssa.(*poset).SetOrderOrEqual(0xc0003c07e0, 0xc0003721c0, 0xc0003729a0, 0xc0003e0100)
	/home/alberto/go/src/cmd/compile/internal/ssa/poset.go:1074 +0x85
cmd/compile/internal/ssa.(*factsTable).update(0xc000371900, 0xc0003a9e00, 0xc0003721c0, 0xc0003729a0, 0x2, 0x3)
	/home/alberto/go/src/cmd/compile/internal/ssa/prove.go:235 +0xfa
cmd/compile/internal/ssa.addRestrictions(0xc0003a9e00, 0xc000371900, 0x3, 0xc0003721c0, 0xc0003729a0, 0x3)
	/home/alberto/go/src/cmd/compile/internal/ssa/prove.go:1051 +0x7f
cmd/compile/internal/ssa.addBranchRestrictions(0xc000371900, 0xc0003a9e00, 0x1)
	/home/alberto/go/src/cmd/compile/internal/ssa/prove.go:1025 +0x224
cmd/compile/internal/ssa.prove(0xc0002be580)
	/home/alberto/go/src/cmd/compile/internal/ssa/prove.go:890 +0x1141
cmd/compile/internal/ssa.Compile(0xc0002be580)
	/home/alberto/go/src/cmd/compile/internal/ssa/compile.go:92 +0x9a5
cmd/compile/internal/gc.buildssa(0xc0002be2c0, 0x0, 0x0)
	/home/alberto/go/src/cmd/compile/internal/gc/ssa.go:444 +0xcd8
cmd/compile/internal/gc.compileSSA(0xc0002be2c0, 0x0)
	/home/alberto/go/src/cmd/compile/internal/gc/pgen.go:298 +0x5d
cmd/compile/internal/gc.compile(0xc0002be2c0)
	/home/alberto/go/src/cmd/compile/internal/gc/pgen.go:277 +0x33b
cmd/compile/internal/gc.funccompile(0xc0002be2c0)
	/home/alberto/go/src/cmd/compile/internal/gc/pgen.go:222 +0xc1
cmd/compile/internal/gc.Main(0xe49b30)
	/home/alberto/go/src/cmd/compile/internal/gc/main.go:699 +0x3174
main.main()
	/home/alberto/go/src/cmd/compile/main.go:50 +0xac


goroutine 1 [running]:
runtime/debug.Stack(0xfe1040, 0xc000090008, 0x0)
	/home/alberto/go/src/runtime/debug/stack.go:24 +0x9d
cmd/compile/internal/gc.Fatalf(0xc00030e1c0, 0x32, 0xc0000967d0, 0x5, 0x5)
	/home/alberto/go/src/cmd/compile/internal/gc/subr.go:193 +0x291
cmd/compile/internal/gc.(*ssafn).Fatalf(0xc00007b9e0, 0x805000000002, 0xe3f5a5, 0x2c, 0xc000339580, 0x4, 0x4)
	/home/alberto/go/src/cmd/compile/internal/gc/ssa.go:6783 +0x1b0
cmd/compile/internal/ssa.(*Func).Fatalf(...)
	/home/alberto/go/src/cmd/compile/internal/ssa/func.go:625
cmd/compile/internal/ssa.Compile.func1(0xc0003cc790, 0xc0002be580)
	/home/alberto/go/src/cmd/compile/internal/ssa/compile.go:49 +0x216
panic(0xda9a00, 0xfd7670)
	/home/alberto/go/src/runtime/panic.go:909 +0x155
cmd/compile/internal/ssa.(*poset).aliasnode(0xc0003c07e0, 0xc0003729a0, 0xc0003721c0)
	/home/alberto/go/src/cmd/compile/internal/ssa/poset.go:450 +0x71a
cmd/compile/internal/ssa.(*poset).collapsepath(0xc0003c07e0, 0xc0003729a0, 0xc0003721c0, 0x15b9d01)
	/home/alberto/go/src/cmd/compile/internal/ssa/poset.go:618 +0x102
cmd/compile/internal/ssa.(*poset).setOrder(0xc0003c07e0, 0xc0003721c0, 0xc0003729a0, 0x1fc51b00, 0xbef5dd81c848f0f1)
	/home/alberto/go/src/cmd/compile/internal/ssa/poset.go:1034 +0x226
cmd/compile/internal/ssa.(*poset).SetOrderOrEqual(0xc0003c07e0, 0xc0003721c0, 0xc0003729a0, 0xc0003e0100)
	/home/alberto/go/src/cmd/compile/internal/ssa/poset.go:1074 +0x85
cmd/compile/internal/ssa.(*factsTable).update(0xc000371900, 0xc0003a9e00, 0xc0003721c0, 0xc0003729a0, 0x2, 0x3)
	/home/alberto/go/src/cmd/compile/internal/ssa/prove.go:235 +0xfa
cmd/compile/internal/ssa.addRestrictions(0xc0003a9e00, 0xc000371900, 0x3, 0xc0003721c0, 0xc0003729a0, 0x3)
	/home/alberto/go/src/cmd/compile/internal/ssa/prove.go:1051 +0x7f
cmd/compile/internal/ssa.addBranchRestrictions(0xc000371900, 0xc0003a9e00, 0x1)
	/home/alberto/go/src/cmd/compile/internal/ssa/prove.go:1025 +0x224
cmd/compile/internal/ssa.prove(0xc0002be580)
	/home/alberto/go/src/cmd/compile/internal/ssa/prove.go:890 +0x1141
cmd/compile/internal/ssa.Compile(0xc0002be580)
	/home/alberto/go/src/cmd/compile/internal/ssa/compile.go:92 +0x9a5
cmd/compile/internal/gc.buildssa(0xc0002be2c0, 0x0, 0x0)
	/home/alberto/go/src/cmd/compile/internal/gc/ssa.go:444 +0xcd8
cmd/compile/internal/gc.compileSSA(0xc0002be2c0, 0x0)
	/home/alberto/go/src/cmd/compile/internal/gc/pgen.go:298 +0x5d
cmd/compile/internal/gc.compile(0xc0002be2c0)
	/home/alberto/go/src/cmd/compile/internal/gc/pgen.go:277 +0x33b
cmd/compile/internal/gc.funccompile(0xc0002be2c0)
	/home/alberto/go/src/cmd/compile/internal/gc/pgen.go:222 +0xc1
cmd/compile/internal/gc.Main(0xe49b30)
	/home/alberto/go/src/cmd/compile/internal/gc/main.go:699 +0x3174
main.main()
	/home/alberto/go/src/cmd/compile/main.go:50 +0xac
@cuonglm

This comment has been minimized.

Copy link
Contributor

@cuonglm cuonglm commented Oct 25, 2019

Seems related to recent changes to poset

@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented Oct 25, 2019

@cuonglm

This comment has been minimized.

Copy link
Contributor

@cuonglm cuonglm commented Oct 25, 2019

git bisect points to c3a871f

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 25, 2019

Change https://golang.org/cl/203438 mentions this issue: cmd/compile: fix poset constant alias handling

@rasky

This comment has been minimized.

Copy link
Member

@rasky rasky commented Oct 25, 2019

The bug happens because of a logic snafu in the master's version poset.

poset.constants is a map from int64 constant values to poset indices, so if constants[5] is 32 it means that the 32th node of the poset contains the constant 5. This normally means that its correspondent SSA node is a constant.

When poset learns that two nodes are equal, it aliases them (which means that it collapses them to the same node, adjusting in/out edges). When collapsing two nodes one of each is a constant, either node is kept, and the other is removed (it doesn't really matter which). If the removed node is in the constant map, the constant map should be updated to point to the other node.

The bug happens because the aliasnode functions doesn't explicitly check whether the removed node is reference in the constant map to avoid a O(n) loop through it; instead, it checks whether the corresponding SSA node is a constant (isGenericConstInt()). This is not always the same, because the corresponding SSA node could have been previously aliased to a constant; so it would be found in the constant map, even if it's not a constant itself.

I have a pending CL 200860 that change aliasnode into a more generic aliasnodes; in doing so, it fixes this bug, by changing the isGenericConstInt() call to an explicit loop through the constant map. The CL has been already +2 pending a comment, and I'm going to submit it soon.

If @randall77 agrees, we can just keep this bug open one day or two until I submit that CL. We can then add the testcase to fixedbugs.

@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented Oct 25, 2019

@rasky That sounds fine.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 26, 2019

Change https://golang.org/cl/200861 mentions this issue: cmd/compile: in poset, implement path collapsing

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 26, 2019

Change https://golang.org/cl/203598 mentions this issue: test: add test for fixed internal compiler error

@gopherbot gopherbot closed this in c70e547 Oct 26, 2019
gopherbot pushed a commit that referenced this issue Oct 26, 2019
Updates #35157 (the bug there was fixed by CL200861)

Change-Id: I67069207b4cdc2ad4a475dd0bbc8555ecc5f534f
Reviewed-on: https://go-review.googlesource.com/c/go/+/203598
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.