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: runtime failure with unified IR ("unreachable method called") #51519

Closed
FiloSottile opened this issue Mar 7, 2022 · 13 comments
Closed
Labels
NeedsFix Soon
Milestone

Comments

@FiloSottile
Copy link
Contributor

@FiloSottile FiloSottile commented Mar 7, 2022

https://go.dev/cl/360015 is still failing on linux-amd64-unified after #49536 was fixed.

https://storage.googleapis.com/go-build-log/6a77cb42/linux-amd64-unified_ff342a8e.log

fatal error: unreachable method called. linker bug?

goroutine 20 [running]:
runtime.throw({0x5b3397?, 0x7f4ed9a005b8?})
	/workdir/go/src/runtime/panic.go:992 +0x71 fp=0xc000045dd8 sp=0xc000045da8 pc=0x434d91
runtime.unreachableMethod()
	/workdir/go/src/runtime/iface.go:532 +0x25 fp=0xc000045df8 sp=0xc000045dd8 pc=0x40bc05
crypto/elliptic.matchesSpecificCurve(...)
	/workdir/go/src/crypto/elliptic/params.go:291
crypto/elliptic.(*CurveParams).IsOnCurve(0xc0000e6800, 0xc0000dc6a0, 0xc0000dc6c0)
	/workdir/go/src/crypto/elliptic/params.go:49 +0xda fp=0xc000045e98 sp=0xc000045df8 pc=0x53be1a

/cc @mdempsky

@cagedmantis cagedmantis added the NeedsInvestigation label Mar 7, 2022
@cagedmantis cagedmantis added this to the Backlog milestone Mar 7, 2022
@mdempsky mdempsky self-assigned this Mar 7, 2022
@mdempsky mdempsky added the Soon label Mar 7, 2022
@mdempsky mdempsky removed this from the Backlog milestone Mar 7, 2022
@mdempsky mdempsky added this to the Go1.19 milestone Mar 7, 2022
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 7, 2022

Sorry about that, looking into it. (FWIW, I did actually test go build crypto/x509 after patching in that CL, but I didn't think to testing running the resulting binary.)

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 7, 2022

Smaller repro:

package main

import (
	"crypto/elliptic"
	"math/big"
)

func main() {
	elliptic.P224()

	var z big.Int
	elliptic.P256().IsOnCurve(&z, &z)
}

@cuonglm
Copy link
Member

@cuonglm cuonglm commented Mar 7, 2022

Smaller repro:

package main

import (
	"crypto/elliptic"
	"math/big"
)

func main() {
	elliptic.P224()

	var z big.Int
	elliptic.P256().IsOnCurve(&z, &z)
}

Hmm, I can't reproduce the bug with this program, using GOEXPERIMENT=unified go run main.go

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 7, 2022

@cuonglm Did you patch in CLs 315273 and 360015?

@cuonglm
Copy link
Member

@cuonglm cuonglm commented Mar 7, 2022

@cuonglm Did you patch in CLs 315273 and 360015?

Ops, I did checkout CL 360015 using git fetch https://go.googlesource.com/go refs/changes/15/360015/6 && git checkout -b change-360015 FETCH_HEAD, but run go build instead of go run. That's why the bug is not triggered.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 8, 2022

The issue seems sensitive to the use of the -p flag, which makes it difficult to repro in the $GOROOT/test framework.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 8, 2022

This is the simplest repro I've come up with so far:

$ cat elliptic.go
package elliptic

type Curve interface{ IsOnCurve() }

type nistCurve[Point any] struct{}

func (curve *nistCurve[Point]) IsOnCurve() {}

var RawP224 = &nistCurve[int]{}
var P224 = Curve(RawP224)

$ cat main.go
package main

import "elliptic"

func main() {
	_ = elliptic.Curve(elliptic.RawP224)
	elliptic.P224.IsOnCurve()
}

$ mkdir pkg
$ GOEXPERIMENT=unified go tool compile -I pkg -p elliptic -o pkg/elliptic.a elliptic.go
$ GOEXPERIMENT=unified go tool compile -I pkg -p main -o pkg/main.a main.go
$ GOEXPERIMENT=unified go tool link -L pkg pkg/main.a
$ ./a.out
fatal error: unreachable method called. linker bug?
[...]

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 8, 2022

@cherrymui suggests looking at cmd/link -dumpdep output. Here's a diff between bad and good output:

$ diff -u bad good
--- bad	2022-03-08 12:09:51.022029489 -0800
+++ good	2022-03-08 12:09:42.933858359 -0800
@@ -4709,12 +4709,12 @@
 example/elliptic.P224 -> example/elliptic..stmp_0
 go.itab.*example/elliptic.nistCurve[int],example/elliptic.Curve -> type.example/elliptic.Curve
 go.itab.*example/elliptic.nistCurve[int],example/elliptic.Curve -> type.*example/elliptic.nistCurve[int] <UsedInIface>
-type.*example/elliptic.nistCurve[int] <UsedInIface> -> type..namedata.*elliptic.nistCurve[int]-
-type.*example/elliptic.nistCurve[int] <UsedInIface> -> type.example/elliptic.nistCurve[int] <UsedInIface>
-type.*example/elliptic.nistCurve[int] <UsedInIface> -> type..importpath.example/elliptic.
-type.*example/elliptic.nistCurve[int] <UsedInIface> -> type..namedata.IsOnCurve.
 type.example/elliptic.Curve -> type..namedata.*elliptic.Curve.
 type.example/elliptic.Curve -> type.*example/elliptic.Curve
+type.example/elliptic.Curve -> type..importpath.example/elliptic.
+type.example/elliptic.Curve -> type..namedata.IsOnCurve.
+type.*example/elliptic.nistCurve[int] <UsedInIface> -> type..namedata.*elliptic.nistCurve[int]-
+type.*example/elliptic.nistCurve[int] <UsedInIface> -> type.example/elliptic.nistCurve[int] <UsedInIface>
 runtime.getpid -> runtime.getpid.args_stackmap
 runtime.getpid -> runtime.getpid.arginfo0
 runtime.tgkill -> runtime.tgkill.args_stackmap

"bad" here means the test case as presented above. "good" is with the blank assignment in main.main commented out (which doesn't trigger the runtime panic).

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 8, 2022

@cherrymui Are you able to help make sense of the above diff?

@cherrymui
Copy link
Member

@cherrymui cherrymui commented Mar 8, 2022

For the diff above, it looks like the RHS of the arrows are the same set of symbols? So it looks like the set of symbols marked live are the same.

It looks like the failing call is example/elliptic.(*nistCurve[int]).IsOnCurve ? It looks like the itab go.itab.*example/elliptic.nistCurve[int],example/elliptic.Curve doesn't point to that method, instead, it points to unreachableMethod. Maybe we build the itab incorrectly? But I don't see why.

@cherrymui
Copy link
Member

@cherrymui cherrymui commented Mar 8, 2022

It looks to me that under unified IR mode, when both the main package and the elliptic package defines the function example/elliptic.(*nistCurve[int]).IsOnCurve, and it is NOT marked as dupOK. When the blank assignment is present, it triggers the itab generation, so both packages contains the itab, and it is dupOK. At link time, they are two symbols of the method, and one symbol of the itab (as the linker dedup them). The itab (happens to) point to the method that is not marked. Another copy of the method symbol is marked, but isn't referenced by the itab.

When unified IR mode is not used, they are also defined in both packages but marked as dupOK so the linker can dedup them.

I think the method symbol should be marked at dupOK in unified mode as well.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 8, 2022

@cherrymui Thanks! Marking the instantiated functions as DUPOK seems to fix it.

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 8, 2022

Change https://go.dev/cl/390956 mentions this issue: cmd/compile: mark instantiated generic functions as DUPOK

@dmitshur dmitshur added NeedsFix and removed NeedsInvestigation labels Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix Soon
Projects
None yet
Development

No branches or pull requests

7 participants