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: relocation error when building some programs on linux/amd64 after dev.regabi merge #44330

Closed
siebenmann opened this issue Feb 17, 2021 · 14 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

Comments

@siebenmann
Copy link

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

$ go version
go version devel +2f0da6d9e2 Wed Feb 17 01:29:54 2021 +0000 linux/amd64

This is the current git tip, but the problem happens from commit 8482559 onward (which merges the dev.regabi branch into main).

Does this issue reproduce with the latest release?

No.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/homes/hawklords/cks/.cache/go-build"
GOENV="/homes/hawklords/cks/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/homes/hawklords/cks/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/homes/hawklords/cks/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/data/code/go-lang/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/data/code/go-lang/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel +2f0da6d9e2 Wed Feb 17 01:29:54 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/homes/hawklords/cks/go/src/github.com/monsterxx03/gospy/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmpfs/go-build1010952997=/tmp/go-build -gno-record-gcc-switches"

What did you do?

The simplest reproduction is:

cd /tmp
git clone https://github.com/monsterxx03/gospy
cd gospy
go build

(Unfortunately the program doesn't build as a module build with 'go install' due to a 'replace' directive in its go.mod.)

What did you expect to see?

The build works.

What did you see instead?

# github.com/monsterxx03/gospy
github.com/monsterxx03/gospy/pkg/term.NewTerm.func1·f: relocation target github.com/monsterxx03/gospy/pkg/term.NewTerm.func1 not defined
@ALTree ALTree changed the title relocation error when building some programs on linux/amd64 after dev.regabi merge cmd/compile: relocation error when building some programs on linux/amd64 after dev.regabi merge Feb 17, 2021
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 17, 2021
@ALTree ALTree added this to the Go1.17 milestone Feb 17, 2021
@bcmills
Copy link
Contributor

bcmills commented Feb 17, 2021

CC @cherrymui @jeremyfaller

@thanm
Copy link
Contributor

thanm commented Feb 17, 2021

Looks like a missing closure -- could this have something to do with the recent changes to the order of closure processing in walk? @mdempsky

@mdempsky
Copy link
Member

Yeah, that seems plausibly related to the change in closure processing.

I don't immediately see any function literals in NewTerm in https://github.com/monsterxx03/gospy/blob/master/pkg/term/term.go though, so I'm not sure where NewTerm.func1 is coming from.

I vaguely remember having to deal with some more of these .f issues on dev.typeparams. It would be worthwhile to check if the issue still reproduces on that branch (also, testing both with -gcflags=all=-G=3 and without).

Otherwise, minimizing down the failure case would be helpful.

@cuonglm
Copy link
Member

cuonglm commented Feb 17, 2021

@mdempsky I bet this issue is the same as many other issues opened recently, which related to inlining of OCLOSURE. If you build the repo with go build -gcflags=all=-l, then it's compiled ok.

@cuonglm
Copy link
Member

cuonglm commented Feb 21, 2021

@mdempsky also, FYI, go build -gcflags=all=-G=3 works!

@siebenmann
Copy link
Author

Here is a more minimal reproduction that still requires a second package, along with a theory about what field is involved.

package main

import "github.com/gizak/termui/v3/widgets"

// widgets.Table contains a 'ColumnResizer func()' field that is
// initialized in NewTable() to a 'func() {}'. However, I don't see
// the issue if I duplicate that within a single package, here.
type Term struct {
	top *widgets.Table
}

func NewFred() *Term {
	table := widgets.NewTable()
	return &Term{top: table}
}

func main() {
	NewFred()
}

Building this with git tip produces main.main.func1·f: relocation target main.main.func1 not defined. Both -gcflags= settings above produced a successful build.

@siebenmann
Copy link
Author

Unsurprisingly this has stopped happening after the just landed commit 8027343, "disable inlining functions with closures for now". That was done for bug #44370, so possibly the fix for it will also fix this issue.

@tosi3k
Copy link

tosi3k commented Feb 26, 2021

FYI: this stopped working, most probably after https://go-review.googlesource.com/c/go/+/296649/ got merged.

@siebenmann
Copy link
Author

I can confirm that git tip from commit 9a555fc onward now has this issue again; building either gospy or my test example now fails again.

@mdempsky
Copy link
Member

(I thought @danscales had been CC'd on this issue already, but it looks like he has not.)

@danscales
Copy link
Contributor

danscales commented Feb 26, 2021

Yes, I didn't know about this bug, else I wouldn't have checked in 9a555fc without seeing if this issue was fixed. If I can't figure this out by Monday, I'll disable inlining functions with closures again. @siebenmann - in the meantime, you can build using:

go build -gcflags="all=-d=inlfuncswithclosures=0"

Thanks for the repro cases!

@danscales
Copy link
Contributor

The symptom ("github.com/monsterxx03/gospy/pkg/term.NewTerm.func1·f: relocation target github.com/monsterxx03/gospy/pkg/term.NewTerm.func1 not defined") is the same as a case for our "-G=3" noder where we were missing a call to typecheck.NeedRuntimeType(t) for an anonymous type t. Not sure yet why inlining a function with a closure would cause us to miss a case - maybe some difference about the way we typecheck imported function bodies (which is when we normally call typecheck.NeedRuntimeType).

@danscales
Copy link
Contributor

OK, I have the simple fix. The issue specifically relates to importing an empty closure. I just need to create the simple test case, then will put the fix out for review.

@gopherbot
Copy link

Change https://golang.org/cl/297569 mentions this issue: cmd/compile: import empty closure function correctly

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

No branches or pull requests

9 participants