From 5ef95666946dd744c0392166981783f72687993f Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Thu, 16 Dec 2021 14:33:13 -0500 Subject: [release-branch.go1.16] cmd/link: force eager binding when using plugins on darwin When building/using plugins on darwin, we need to use flat namespace so the same symbol from the main executable and the plugin can be resolved to the same address. Apparently, when using flat namespace the dynamic linker can hang at forkExec when resolving a lazy binding. Work around it by forcing early bindings. Updates #38824. Fixes #50245. Change-Id: I983aa0a0960b15bf3f7871382e8231ee244655f4 Reviewed-on: https://go-review.googlesource.com/c/go/+/372798 Trust: Cherry Mui Reviewed-by: Than McIntosh Reviewed-by: Ian Lance Taylor Run-TryBot: Cherry Mui TryBot-Result: Gopher Robot (cherry picked from commit c5fee935bbb8f02406eb653cfed550593755a1a4) Reviewed-on: https://go-review.googlesource.com/c/go/+/373095 --- misc/cgo/testplugin/plugin_test.go | 28 +++++++++++++++++++++++++ misc/cgo/testplugin/testdata/forkexec/main.go | 30 +++++++++++++++++++++++++++ src/cmd/link/internal/ld/lib.go | 5 ++++- 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 misc/cgo/testplugin/testdata/forkexec/main.go diff --git a/misc/cgo/testplugin/plugin_test.go b/misc/cgo/testplugin/plugin_test.go index 8869528015..7b4df3f8ed 100644 --- a/misc/cgo/testplugin/plugin_test.go +++ b/misc/cgo/testplugin/plugin_test.go @@ -216,3 +216,31 @@ func TestIssue44956(t *testing.T) { goCmd(t, "build", "-o", "issue44956.exe", "./issue44956/main.go") run(t, "./issue44956.exe") } + +func TestForkExec(t *testing.T) { + // Issue 38824: importing the plugin package causes it hang in forkExec on darwin. + + t.Parallel() + goCmd(t, "build", "-o", "forkexec.exe", "./forkexec/main.go") + + var cmd *exec.Cmd + done := make(chan int, 1) + + go func() { + for i := 0; i < 100; i++ { + cmd = exec.Command("./forkexec.exe", "1") + err := cmd.Run() + if err != nil { + t.Errorf("running command failed: %v", err) + break + } + } + done <- 1 + }() + select { + case <-done: + case <-time.After(5 * time.Minute): + cmd.Process.Kill() + t.Fatalf("subprocess hang") + } +} diff --git a/misc/cgo/testplugin/testdata/forkexec/main.go b/misc/cgo/testplugin/testdata/forkexec/main.go new file mode 100644 index 0000000000..3169ff5f04 --- /dev/null +++ b/misc/cgo/testplugin/testdata/forkexec/main.go @@ -0,0 +1,30 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "os" + "os/exec" + _ "plugin" + "sync" +) + +func main() { + if os.Args[1] != "1" { + return + } + + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + // does not matter what we exec, just exec itself + cmd := exec.Command("./forkexec.exe", "0") + cmd.Run() + }() + } + wg.Wait() +} diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go index 18db567041..d56a51e375 100644 --- a/src/cmd/link/internal/ld/lib.go +++ b/src/cmd/link/internal/ld/lib.go @@ -1269,7 +1269,10 @@ func (ctxt *Link) hostlink() { if ctxt.DynlinkingGo() && objabi.GOOS != "ios" { // -flat_namespace is deprecated on iOS. // It is useful for supporting plugins. We don't support plugins on iOS. - argv = append(argv, "-Wl,-flat_namespace") + // -flat_namespace may cause the dynamic linker to hang at forkExec when + // resolving a lazy binding. See issue 38824. + // Force eager resolution to work around. + argv = append(argv, "-Wl,-flat_namespace", "-Wl,-bind_at_load") } if !combineDwarf { argv = append(argv, "-Wl,-S") // suppress STAB (symbolic debugging) symbols -- cgit v1.2.3-54-g00ecf