cmd/compile: ICE when calling ABI0 function via func value #47317

cherrymui · 2021-07-21T16:54:52Z

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

tip (49402be)

Does this issue reproduce with the latest release?

Reproducible with 1.17RC1
Not with 1.16.

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

darwin/amd64

What did you do?

Compiling code that calls an ABI0 function using a func value, on a register-ABI platform.

x.go

package p

func F() interface{} {
	g := G
	g(1)
	return G
}

func G(x int) [2]int

a.s

TEXT	·G(SB),4,$0-24
	RET

What did you expect to see?

Build successfully.

What did you see instead?

./x.go:6:2: internal compiler error: offset for x at ./x.go:9:8 changed from 0 to 16

goroutine 8 [running]:
runtime/debug.Stack()
	/Users/cherryyz/src/go-tmp/src/runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0x262c0, 0xc0}, {0x19017a3, 0x29}, {0xc0005dd310, 0x4, 0x4})
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/base/print.go:227 +0x154
cmd/compile/internal/base.Fatalf(...)
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/abi.(*ABIConfig).updateOffset(0x0, 0x251ffff, 0xc0003c0910, {0xc00037e380, {0x1a35260, 0xc0003bf110}, {0xc000026228, 0x1, 0x8}, 0x0}, ...)
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/abi/abiutils.go:477 +0x298
cmd/compile/internal/abi.(*ABIConfig).ABIAnalyze(0xc0003bef70, 0x1a4b188, 0x70)
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/abi/abiutils.go:441 +0x232
cmd/compile/internal/ssagen.(*state).call(0xc0005ce000, 0xc000176360, 0x0, 0x0)
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/ssagen/ssa.go:5012 +0x926
cmd/compile/internal/ssagen.(*state).callResult(0xc000176360, 0x0, 0x70)
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/ssagen/ssa.go:4915 +0x1b
cmd/compile/internal/ssagen.(*state).stmt(0xc0005ce000, {0x1a49d38, 0xc000176360})
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/ssagen/ssa.go:1453 +0xf05
cmd/compile/internal/ssagen.(*state).stmtList(0xc0005ce000, {0xc0000ae500, 0x6, 0x1a2f620})
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/ssagen/ssa.go:1414 +0x5d
cmd/compile/internal/ssagen.buildssa(0xc00013c2c0, 0x1)
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/ssagen/ssa.go:622 +0x1d33
cmd/compile/internal/ssagen.Compile(0xc00013c2c0, 0xc0005cdf90)
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/ssagen/pgen.go:165 +0x4c
cmd/compile/internal/gc.compileFunctions.func4.1(0x0)
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/gc/compile.go:153 +0x3a
cmd/compile/internal/gc.compileFunctions.func3.1()
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/gc/compile.go:140 +0x4d
created by cmd/compile/internal/gc.compileFunctions.func3
	/Users/cherryyz/src/go-tmp/src/cmd/compile/internal/gc/compile.go:138 +0x7f

In the case above, G is an ABI0 function. It is called via a func value (g). As far as I can tell, the ABI analysis for the call (g(1)) correctly uses ABIInternal (for ABI config), but the function's *Type being analyzed is the same *Type as the definition of ABI0 G. Perhaps the line g := G just makes g the same *Type as G.

@aclements @thanm @dr2chase what ensures that we always put the ABIInternal reference to a func value? What do we do with its *Type? The ABI analysis associates parameter/result offsets to the fields of function *Type. So if an ABI0 reference and an ABIInternal reference have the same *Type pointer, it blows up.

If I add an explicit type definition, var g func(int) [2]int = G, it compiles fine.

Originally found by @mdempsky and @cuonglm when investigating #47227. See also CL https://go-review.googlesource.com/c/go/+/334883 .

The text was updated successfully, but these errors were encountered:

thanm · 2021-07-21T17:47:42Z

What's weird about this bug is that when I make a cursory look through the call-related code in ssagen and the back end, I don't actually see any places where we take a types.Field corresponding to a function parameter and look at its Offset field (instead the code is looking at the corresponding ABIParamAssignment object fo r the parameter. Do we really need to be setting f.Offset any more for these fields?

mdempsky · 2021-07-21T17:57:07Z

Do we really need to be setting f.Offset any more for these fields?

FWIW, I've been wanting to decouple parameter layout from types.Type/types.Field for a while, as they're not really type system details. If we can avoid touching f.Offset, I think that would be great. Nothing in the frontend should depend on f.Offset for parameters.

cherrymui · 2021-07-21T20:33:38Z

FWIW, I've been wanting to decouple parameter layout from types.Type/types.Field

I also think that's the right direction. If nothing depends on field.Offset, we could try not changing that. And have everything in the backend to use ABIParamAssignment.FrameOffset or something alike.

thanm · 2021-07-21T20:38:53Z

OK, I'll poke at this and see if I can prune away the ABI field offset update. I did find one suspicious use at

https://go.googlesource.com/go/+/3e48c0381fd1beb78e993e940c3b46ca9898ce6d/src/cmd/compile/internal/ssagen/ssa.go#5068

but I am pretty sure I can work around it.

cherrymui · 2021-07-21T20:44:14Z

That code is probably okay. It seems that is only for stack-allocated defer calls. If regabi is used, deferred function is argumentless, so it won't execute that line. Maybe you could condition it on GOEXPERIMENT not being enabled.

thanm · 2021-07-21T20:48:25Z

Yes, exactly.

gopherbot · 2021-07-22T17:08:23Z

Change https://golang.org/cl/336629 mentions this issue: cmd/compile: do not change field offset in ABI analysis

gopherbot · 2021-07-22T17:28:03Z

Change https://golang.org/cl/336610 mentions this issue: [dev.typeparams] cmd/compile: remove outdate TODO in escape analysis

We now understand the root cause of #47227, it will be fixed in #47317. Change-Id: Ifcd44f887a0bd3195818df33e409bd3e818e0b27 Reviewed-on: https://go-review.googlesource.com/c/go/+/336610 Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org>

mdempsky · 2021-07-23T00:44:54Z

If nothing depends on field.Offset, we could try not changing that. And have everything in the backend to use ABIParamAssignment.FrameOffset or something alike.

Out of curiosity, where does 052da57 leave us w.r.t. this? Is f.Offset still needed by the backend for function parameters? It looks like maybe not?

cherrymui · 2021-07-23T02:40:19Z

Out of curiosity, where does 052da57 leave us w.r.t. this? Is f.Offset still needed by the backend for function parameters? It looks like maybe not?

It was used in exactly one place in the backend, which I changed to not use that. Now it should not be used.

On the dev.typeparams branch it should already be not used in the backend. The code I changed in the CL is already gone.

mdempsky · 2021-07-23T19:16:18Z

Ah, great. Thanks for the clarification.

cherrymui added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 21, 2021

cherrymui added this to the Go1.17 milestone Jul 21, 2021

gopherbot closed this as completed in 052da57 Jul 22, 2021

golang locked and limited conversation to collaborators Jul 23, 2022

gopherbot added the FrozenDueToAge label Jul 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/compile: ICE when calling ABI0 function via func value #47317

cmd/compile: ICE when calling ABI0 function via func value #47317

cherrymui commented Jul 21, 2021

thanm commented Jul 21, 2021

mdempsky commented Jul 21, 2021

cherrymui commented Jul 21, 2021

thanm commented Jul 21, 2021

cherrymui commented Jul 21, 2021

thanm commented Jul 21, 2021

gopherbot commented Jul 22, 2021

gopherbot commented Jul 22, 2021

mdempsky commented Jul 23, 2021

cherrymui commented Jul 23, 2021

mdempsky commented Jul 23, 2021

cmd/compile: ICE when calling ABI0 function via func value #47317

cmd/compile: ICE when calling ABI0 function via func value #47317

Comments

cherrymui commented Jul 21, 2021

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

thanm commented Jul 21, 2021

mdempsky commented Jul 21, 2021

cherrymui commented Jul 21, 2021

thanm commented Jul 21, 2021

cherrymui commented Jul 21, 2021

thanm commented Jul 21, 2021

gopherbot commented Jul 22, 2021

gopherbot commented Jul 22, 2021

mdempsky commented Jul 23, 2021

cherrymui commented Jul 23, 2021

mdempsky commented Jul 23, 2021

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

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