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/go: spurious error message when external linking a pure Go program #31544

Closed
cherrymui opened this issue Apr 18, 2019 · 18 comments
Closed

cmd/go: spurious error message when external linking a pure Go program #31544

cherrymui opened this issue Apr 18, 2019 · 18 comments
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@cherrymui
Copy link
Member

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

tip (3235f7c)

Does this issue reproduce with the latest release?

Yes, happens with Go 1.11, 1.12, and tip, but not with Go 1.10.

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

linux/amd64

What did you do?

$ mkdir -p /tmp/src/hello
$ cat >hello.go
package main
func main() { println("hello") }
$ GO111MODULE=off GOPATH=/tmp go build -ldflags=-linkmode=external
# hello
loadinternal: cannot find runtime/cgo
$ echo $?
0
$ ./hello
hello

When using external linking for a pure Go program, the linker prints loadinternal: cannot find runtime/cgo message. The linking actually succeeded, and the generated binary works fine.

What did you expect to see?

No error message, like with Go 1.10.

$ GOPATH=/tmp go1.10 build -ldflags=-linkmode=external 
$ ./hello 
hello

What did you see instead?

Spurious loadinternal: cannot find runtime/cgo error message.

@cherrymui
Copy link
Member Author

This also happens in module mode:

$ go mod init hello
go: creating new go.mod: module hello
$ GO111MODULE=on go build -ldflags=-linkmode=external 
# hello
loadinternal: cannot find runtime/cgo

But it does not happen if the file name (hello.go) is given explicitly.

$ go build -ldflags=-linkmode=external hello.go 
$

@julieqiu julieqiu changed the title cmd/go or cmd/link: spurious error message when external linking a pure Go program cmd/go: spurious error message when external linking a pure Go program Apr 22, 2019
@julieqiu julieqiu added this to the Go1.13 milestone Apr 22, 2019
@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 22, 2019
@julieqiu
Copy link
Member

/cc @jayconrod @bcmills

@bcmills
Copy link
Contributor

bcmills commented May 8, 2019

I can reproduce the failure. I'm bisecting it now to try to get some idea of how it broke.

@bcmills bcmills self-assigned this May 8, 2019
@bcmills
Copy link
Contributor

bcmills commented May 8, 2019

git bisect points to CL 129059 (CC @rsc @alandonovan).

@bcmills
Copy link
Contributor

bcmills commented May 8, 2019

This is a result of the -ldflags=-linkmode=external flag now (correctly) being passed when compiling the hello package, which reveals a typestate bug in load.PackagesAndErrors.

The decision in the LinkerDeps function about whether to add the runtime/cgo dependency is based on whether the -ldflags=-linkmode=external flag is set for the package, which itself depends on whether CmdlinePkg flag is set:

match := func(p *Package) bool { return p.Internal.CmdlinePkg || p.Internal.CmdlineFiles } // default predicate with no pattern

That field is set after the call to loadImport:

p := loadImport(pre, pkg, base.Cwd, nil, &stk, nil, 0)
p.Match = append(p.Match, m.Pattern)
p.Internal.CmdlinePkg = true

However, by that point loadImport has already called load on the package:

p.load(stk, bp, err)

...and load is what's supposed to figure out the implicit linker dependencies:

for _, dep := range LinkerDeps(p) {

@rsc
Copy link
Contributor

rsc commented May 9, 2019

@cherrymui, unless this is important for you for some reason, we'd like to postpone the fix to Go 1.14. It's subtle code.

/cc @bcmills

@bcmills bcmills modified the milestones: Go1.13, Go1.14 May 9, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/176217 mentions this issue: cmd/go/internal/work: set CmdlinePkg in loadImport instead of after

@cherrymui
Copy link
Member Author

@cherrymui, unless this is important for you for some reason, we'd like to postpone the fix to Go 1.14. It's subtle code.

Sure, it is fine to postpone.

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@michael-obermueller
Copy link

We would like to propose to reconsider the priority fixing this issue. The implication of it is, that such Go binaries have no runtime/cgo dependency which means they do not create threads using pthread_create but instead the clone system call.

Preloaded (LD_PRELOAD) shared objects may rely on the fact that Go creates POSIX threads.

@ianlancetaylor
Copy link
Contributor

I certainly think this should be fixed, but I'm not following the concern with LD_PRELOAD shared objects. I don't see how it would be possible to use an LD_PRELOAD with a pure Go program.

@bcmills bcmills modified the milestones: Backlog, Go1.15 Dec 10, 2019
@bcmills bcmills removed their assignment Dec 10, 2019
@Hollerberg
Copy link

@ianlancetaylor: The scenario is very specific, but has a serious impact on existing applications.

A shared object of our (application performance monitoring) product is pre-loaded into Go processes to hook certain Go functions. Customers use -ldflags=-linkmode=external in some cases to enforce dynamically built applications.

As the issue causes iscgo set to false, Go runtime will create threads with clone and not with pthread_create. Thus, pre-loaded C code executed in Go runtime thread context (invoked from hooked Go functions) will crash the application accessing e.g. TLS.

@ianlancetaylor
Copy link
Contributor

OK, that seems not only specific but completely unsupported.

Again, I think this bug should be fixed, but the Go team can't take any responsibility for keeping that kind of pre-loaded library working.

@bcmills bcmills added compiler/runtime Issues related to the Go compiler and/or runtime. GoCommand cmd/go labels Jan 23, 2023
@dr2chase dr2chase removed the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 1, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/472515 mentions this issue: cmd/link/internal/ld: move more of mustLinkExternal into internal/platform

gopherbot pushed a commit that referenced this issue Mar 2, 2023
…tform

internal/platform.MustLinkExternal is used in various places to
determine whether external linking is required. It should always
match what the linker actually requires, but today does not match
because the linker imposes additional constraints.

Updates #31544.

Change-Id: I0cc6ad587e95c607329dea5d60d29a5fb2a9e722
Reviewed-on: https://go-review.googlesource.com/c/go/+/472515
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/477195 mentions this issue: cmd/go: import runtime/cgo when externally linking

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/477397 mentions this issue: cmd/go: skip TestScript/build_static on Darwin

@bcmills bcmills reopened this Mar 20, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/477839 mentions this issue: cmd/go: import runtime/cgo when externally linking

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 18, 2023
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Aug 18, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/522239 mentions this issue: cmd/go: error out of linking package main if cgo is required but not enabled

gopherbot pushed a commit that referenced this issue Aug 23, 2023
…enabled

Fixes #46330.
Fixes #62123.
Updates #31544.

Change-Id: I023aa2bdb5a24e126a0de5192a077e8cf1a0a67c
Reviewed-on: https://go-review.googlesource.com/c/go/+/522239
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
cellularmitosis pushed a commit to cellularmitosis/go that referenced this issue Aug 24, 2023
…enabled

Fixes golang#46330.
Fixes golang#62123.
Updates golang#31544.

Change-Id: I023aa2bdb5a24e126a0de5192a077e8cf1a0a67c
Reviewed-on: https://go-review.googlesource.com/c/go/+/522239
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/569296 mentions this issue: _content/doc: mention cgo requirements for external linking in release notes for Go 1.21 and 1.22

gopherbot pushed a commit to golang/website that referenced this issue Mar 11, 2024
…e notes for Go 1.21 and 1.22

Fixes golang/go#65887.
Updates golang/go#31544.
Updates golang/go#46330.
Updates golang/go#64875.

Change-Id: Ibb035e2287ad0efdbe875c5dd16ffd938ec7a956
Reviewed-on: https://go-review.googlesource.com/c/website/+/569296
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
andaaron added a commit to andaaron/zot that referenced this issue Mar 19, 2024
Looks like they made some cleanup in the logic allowing buildmode pie on various platforms.

Related to golang/go#31544
See the code at: https://cs.opensource.google/go/go/+/master:src/internal/platform/supported.go;l=222-231;drc=d7fcb5cf80953f1d63246f1ae9defa60c5ce2d76;bpv=1;bpt=0

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
andaaron added a commit to andaaron/zot that referenced this issue Mar 19, 2024
Looks like they made some cleanup in the logic allowing buildmode pie on various platforms.

Related to golang/go#31544
See the code at: https://cs.opensource.google/go/go/+/master:src/internal/platform/supported.go;l=222-231;drc=d7fcb5cf80953f1d63246f1ae9defa60c5ce2d76;bpv=1;bpt=0

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
andaaron added a commit to andaaron/zot that referenced this issue Mar 20, 2024
Looks like they made some cleanup in the logic allowing buildmode pie on various platforms.

Related to golang/go#31544
See the code at: https://cs.opensource.google/go/go/+/master:src/internal/platform/supported.go;l=222-231;drc=d7fcb5cf80953f1d63246f1ae9defa60c5ce2d76;bpv=1;bpt=0

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
rchincha pushed a commit to project-zot/zot that referenced this issue Mar 20, 2024
* chore: update to go 1.22

Only go toolchain version is updated.
We compile with go 1.22, but we allow others to compile using language version 1.21 if they wish to.
If we also updated the go version in go.mod everyone would be forced to update, as that is enforced as a minimum allowed version.

This comment explains the difference well enough https://news.ycombinator.com/item?id=36455759

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>

* chore: fix freeBSD AMD64 build

Looks like they made some cleanup in the logic allowing buildmode pie on various platforms.

Related to golang/go#31544
See the code at: https://cs.opensource.google/go/go/+/master:src/internal/platform/supported.go;l=222-231;drc=d7fcb5cf80953f1d63246f1ae9defa60c5ce2d76;bpv=1;bpt=0

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>

---------

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests