[go] cmd: support space and quotes in CC and CXX

20 views
Skip to first unread message

Jay Conrod (Gerrit)

unread,
Aug 12, 2021, 6:42:24 PM8/12/21
to Michael Matloob, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Michael Matloob.

Jay Conrod would like Michael Matloob to review this change.

View Change

cmd: support space and quotes in CC and CXX

The CC and CXX environment variables now support spaces and quotes
(both double and single). This fixes two issues: first, if CC is a
single path that contains spaces (like 'c:\Program
Files\gcc\bin\gcc.exe'), that should now work if the space is quoted
or escaped (#41400). Second, if CC or CXX has multiple arguments (like
'gcc -O2'), they are now split correctly, and the arguments are passed
before other arguments when invoking the C compiler. Previously,
strings.Fields was used to split arguments, and the arguments were
placed later in the command line. (#43078).

Fixes golang/go#41400
Fixes golang/go#43078

NOTE: This change also includes a fix (CL 341929) for a test that was
broken by the original CL. Commit message for the fix is below.

[dev.cmdgo] cmd/link: fix TestBuildForTvOS

This test was broken in CL 334732 on darwin.

The test invokes 'go build' with a CC containing the arguments
-framework CoreFoundation. Previously, the go command split CC on
whitespace, and inserted the arguments after the command line when
running CC directly. Those arguments weren't passed to cgo though,
so cgo ran CC without -framework CoreFoundation (or any of the other
flags).

In CL 334732, we pass CC through to cgo, and cgo splits arguments
using str.SplitQuotedFields. So -framework CoreFoundation actually
gets passed to the C compiler. It appears that -framework flags are
only meant to be used in linking operations, so when cgo invokes clang
with -E (run preprocessor only), clang emits an error that -framework
is unused.

This change fixes the test by moving -framework CoreFoundation out of
CC and into CGO_LDFLAGS.

Change-Id: I2d5d89ddb19c94adef65982a8137b01f037d5c11
Reviewed-on: https://go-review.googlesource.com/c/go/+/334732
Trust: Jay Conrod <jayc...@google.com>
Trust: Michael Matloob <mat...@golang.org>
Run-TryBot: Jay Conrod <jayc...@google.com>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Michael Matloob <mat...@golang.org>
---
M src/cmd/cgo/gcc.go
M src/cmd/cgo/main.go
M src/cmd/compile/internal/ssa/stmtlines_test.go
M src/cmd/dist/buildtool.go
M src/cmd/go/internal/envcmd/env.go
M src/cmd/go/internal/work/exec.go
M src/cmd/go/internal/work/gc.go
M src/cmd/go/internal/work/init.go
M src/cmd/go/script_test.go
A src/cmd/go/testdata/script/cgo_path_space_quote.txt
M src/cmd/internal/dwarf/dwarf.go
M src/cmd/link/dwarf_test.go
M src/cmd/link/internal/ld/lib.go
M src/cmd/link/internal/ld/main.go
M src/cmd/link/link_test.go
15 files changed, 209 insertions(+), 104 deletions(-)

diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go
index a73e998..92adb1e 100644
--- a/src/cmd/cgo/gcc.go
+++ b/src/cmd/cgo/gcc.go
@@ -23,10 +23,13 @@
"internal/xcoff"
"math"
"os"
+ "os/exec"
"strconv"
"strings"
"unicode"
"unicode/utf8"
+
+ "cmd/internal/str"
)

var debugDefine = flag.Bool("debug-define", false, "print relevant #defines")
@@ -382,7 +385,7 @@
stderr = p.gccErrors(b.Bytes())
}
if stderr == "" {
- fatalf("%s produced no output\non input:\n%s", p.gccBaseCmd()[0], b.Bytes())
+ fatalf("%s produced no output\non input:\n%s", gccBaseCmd[0], b.Bytes())
}

completed := false
@@ -457,7 +460,7 @@
}

if !completed {
- fatalf("%s did not produce error at completed:1\non input:\n%s\nfull error output:\n%s", p.gccBaseCmd()[0], b.Bytes(), stderr)
+ fatalf("%s did not produce error at completed:1\non input:\n%s\nfull error output:\n%s", gccBaseCmd[0], b.Bytes(), stderr)
}

for i, n := range names {
@@ -488,7 +491,7 @@
// to users debugging preamble mistakes. See issue 8442.
preambleErrors := p.gccErrors([]byte(f.Preamble))
if len(preambleErrors) > 0 {
- error_(token.NoPos, "\n%s errors for preamble:\n%s", p.gccBaseCmd()[0], preambleErrors)
+ error_(token.NoPos, "\n%s errors for preamble:\n%s", gccBaseCmd[0], preambleErrors)
}

fatalf("unresolved names")
@@ -1545,20 +1548,37 @@
return fmt.Sprintf("/*line :%d:%d*/%s", p.Line, p.Column, s)
}

-// gccBaseCmd returns the start of the compiler command line.
+// checkGCCBaseCmd returns the start of the compiler command line.
// It uses $CC if set, or else $GCC, or else the compiler recorded
// during the initial build as defaultCC.
// defaultCC is defined in zdefaultcc.go, written by cmd/dist.
-func (p *Package) gccBaseCmd() []string {
+//
+// The compiler command line is split into arguments on whitespace. Quotes
+// are understood, so arguments may contain whitespace.
+//
+// checkGCCBaseCmd confirms that the compiler exists in PATH, returning
+// an error if it does not.
+func checkGCCBaseCmd() ([]string, error) {
// Use $CC if set, since that's what the build uses.
- if ret := strings.Fields(os.Getenv("CC")); len(ret) > 0 {
- return ret
+ value := os.Getenv("CC")
+ if value == "" {
+ // Try $GCC if set, since that's what we used to use.
+ value = os.Getenv("GCC")
}
- // Try $GCC if set, since that's what we used to use.
- if ret := strings.Fields(os.Getenv("GCC")); len(ret) > 0 {
- return ret
+ if value == "" {
+ value = defaultCC(goos, goarch)
}
- return strings.Fields(defaultCC(goos, goarch))
+ args, err := str.SplitQuotedFields(value)
+ if err != nil {
+ return nil, err
+ }
+ if len(args) == 0 {
+ return nil, errors.New("CC not set and no default found")
+ }
+ if _, err := exec.LookPath(args[0]); err != nil {
+ return nil, fmt.Errorf("C compiler %q not found: %v", args[0], err)
+ }
+ return args[:len(args):len(args)], nil
}

// gccMachine returns the gcc -m flag to use, either "-m32", "-m64" or "-marm".
@@ -1604,7 +1624,7 @@
// gccCmd returns the gcc command line to use for compiling
// the input.
func (p *Package) gccCmd() []string {
- c := append(p.gccBaseCmd(),
+ c := append(gccBaseCmd,
"-w", // no warnings
"-Wno-error", // warnings are not errors
"-o"+gccTmp(), // write object to tmp
@@ -2005,7 +2025,7 @@
// #defines that gcc encountered while processing the input
// and its included files.
func (p *Package) gccDefines(stdin []byte) string {
- base := append(p.gccBaseCmd(), "-E", "-dM", "-xc")
+ base := append(gccBaseCmd, "-E", "-dM", "-xc")
base = append(base, p.gccMachine()...)
stdout, _ := runGcc(stdin, append(append(base, p.GccOptions...), "-"))
return stdout
diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go
index c6a0c52..14642b7 100644
--- a/src/cmd/cgo/main.go
+++ b/src/cmd/cgo/main.go
@@ -21,7 +21,6 @@
"io"
"io/ioutil"
"os"
- "os/exec"
"path/filepath"
"reflect"
"runtime"
@@ -248,6 +247,7 @@
var trimpath = flag.String("trimpath", "", "applies supplied rewrites or trims prefixes to recorded source file paths")

var goarch, goos, gomips, gomips64 string
+var gccBaseCmd []string

func main() {
objabi.AddVersionFlag() // -V
@@ -305,10 +305,10 @@
p := newPackage(args[:i])

// We need a C compiler to be available. Check this.
- gccName := p.gccBaseCmd()[0]
- _, err := exec.LookPath(gccName)
+ var err error
+ gccBaseCmd, err = checkGCCBaseCmd()
if err != nil {
- fatalf("C compiler %q not found: %v", gccName, err)
+ fatalf("%v", err)
os.Exit(2)
}

diff --git a/src/cmd/compile/internal/ssa/stmtlines_test.go b/src/cmd/compile/internal/ssa/stmtlines_test.go
index a510d0b..843db8c 100644
--- a/src/cmd/compile/internal/ssa/stmtlines_test.go
+++ b/src/cmd/compile/internal/ssa/stmtlines_test.go
@@ -2,6 +2,7 @@

import (
cmddwarf "cmd/internal/dwarf"
+ "cmd/internal/str"
"debug/dwarf"
"debug/elf"
"debug/macho"
@@ -57,7 +58,11 @@
if extld == "" {
extld = "gcc"
}
- enabled, err := cmddwarf.IsDWARFEnabledOnAIXLd(extld)
+ extldArgs, err := str.SplitQuotedFields(extld)
+ if err != nil {
+ t.Fatal(err)
+ }
+ enabled, err := cmddwarf.IsDWARFEnabledOnAIXLd(extldArgs)
if err != nil {
t.Fatal(err)
}
diff --git a/src/cmd/dist/buildtool.go b/src/cmd/dist/buildtool.go
index 26b33e3..320c62f 100644
--- a/src/cmd/dist/buildtool.go
+++ b/src/cmd/dist/buildtool.go
@@ -47,6 +47,7 @@
"cmd/internal/objabi",
"cmd/internal/pkgpath",
"cmd/internal/src",
+ "cmd/internal/str",
"cmd/internal/sys",
"cmd/link",
"cmd/link/internal/...",
diff --git a/src/cmd/go/internal/envcmd/env.go b/src/cmd/go/internal/envcmd/env.go
index 1553d26..483ce2a 100644
--- a/src/cmd/go/internal/envcmd/env.go
+++ b/src/cmd/go/internal/envcmd/env.go
@@ -26,6 +26,7 @@
"cmd/go/internal/load"
"cmd/go/internal/modload"
"cmd/go/internal/work"
+ "cmd/internal/str"
)

var CmdEnv = &base.Command{
@@ -104,13 +105,13 @@
env = append(env, cfg.EnvVar{Name: key, Value: val})
}

- cc := cfg.DefaultCC(cfg.Goos, cfg.Goarch)
- if env := strings.Fields(cfg.Getenv("CC")); len(env) > 0 {
- cc = env[0]
+ cc := cfg.Getenv("CC")
+ if cc == "" {
+ cc = cfg.DefaultCC(cfg.Goos, cfg.Goarch)
}
- cxx := cfg.DefaultCXX(cfg.Goos, cfg.Goarch)
- if env := strings.Fields(cfg.Getenv("CXX")); len(env) > 0 {
- cxx = env[0]
+ cxx := cfg.Getenv("CXX")
+ if cxx == "" {
+ cxx = cfg.DefaultCXX(cfg.Goos, cfg.Goarch)
}
env = append(env, cfg.EnvVar{Name: "AR", Value: envOr("AR", "ar")})
env = append(env, cfg.EnvVar{Name: "CC", Value: cc})
@@ -457,10 +458,23 @@
if !filepath.IsAbs(val) && val != "" {
return fmt.Errorf("GOPATH entry is relative; must be absolute path: %q", val)
}
- // Make sure CC and CXX are absolute paths
- case "CC", "CXX", "GOMODCACHE":
- if !filepath.IsAbs(val) && val != "" && val != filepath.Base(val) {
- return fmt.Errorf("%s entry is relative; must be absolute path: %q", key, val)
+ case "GOMODCACHE":
+ if !filepath.IsAbs(val) && val != "" {
+ return fmt.Errorf("GOMODCACHE entry is relative; must be absolute path: %q", val)
+ }
+ case "CC", "CXX":
+ if val == "" {
+ break
+ }
+ args, err := str.SplitQuotedFields(val)
+ if err != nil {
+ return fmt.Errorf("invalid %s: %v", key, err)
+ }
+ if len(args) == 0 {
+ return fmt.Errorf("%s entry cannot contain only space", key)
+ }
+ if !filepath.IsAbs(args[0]) && args[0] != filepath.Base(args[0]) {
+ return fmt.Errorf("%s entry is relative; must be absolute path: %q", key, args[0])
}
}

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 2aa099b..f7fae9f 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -1487,6 +1487,8 @@
return nil, nil, errPrintedOutput
}
if len(out) > 0 {
+ // NOTE: we don't attempt to parse quotes and unescapes here. pkg-config
+ // is typically used within shell backticks, which treats quotes literally.
ldflags = strings.Fields(string(out))
if err := checkLinkerFlags("LDFLAGS", "pkg-config --libs", ldflags); err != nil {
return nil, nil, err
@@ -2429,12 +2431,6 @@
return err
}

-// Grab these before main helpfully overwrites them.
-var (
- origCC = cfg.Getenv("CC")
- origCXX = cfg.Getenv("CXX")
-)
-
// gccCmd returns a gcc command line prefix
// defaultCC is defined in zdefaultcc.go, written by cmd/dist.
func (b *Builder) GccCmd(incdir, workdir string) []string {
@@ -2454,40 +2450,23 @@

// ccExe returns the CC compiler setting without all the extra flags we add implicitly.
func (b *Builder) ccExe() []string {
- return b.compilerExe(origCC, cfg.DefaultCC(cfg.Goos, cfg.Goarch))
+ return envList("CC", cfg.DefaultCC(cfg.Goos, cfg.Goarch))
}

// cxxExe returns the CXX compiler setting without all the extra flags we add implicitly.
func (b *Builder) cxxExe() []string {
- return b.compilerExe(origCXX, cfg.DefaultCXX(cfg.Goos, cfg.Goarch))
+ return envList("CXX", cfg.DefaultCXX(cfg.Goos, cfg.Goarch))
}

// fcExe returns the FC compiler setting without all the extra flags we add implicitly.
func (b *Builder) fcExe() []string {
- return b.compilerExe(cfg.Getenv("FC"), "gfortran")
-}
-
-// compilerExe returns the compiler to use given an
-// environment variable setting (the value not the name)
-// and a default. The resulting slice is usually just the name
-// of the compiler but can have additional arguments if they
-// were present in the environment value.
-// For example if CC="gcc -DGOPHER" then the result is ["gcc", "-DGOPHER"].
-func (b *Builder) compilerExe(envValue string, def string) []string {
- compiler := strings.Fields(envValue)
- if len(compiler) == 0 {
- compiler = strings.Fields(def)
- }
- return compiler
+ return envList("FC", "gfortran")
}

// compilerCmd returns a command line prefix for the given environment
// variable and using the default command when the variable is empty.
func (b *Builder) compilerCmd(compiler []string, incdir, workdir string) []string {
- // NOTE: env.go's mkEnv knows that the first three
- // strings returned are "gcc", "-I", incdir (and cuts them off).
- a := []string{compiler[0], "-I", incdir}
- a = append(a, compiler[1:]...)
+ a := append(compiler, "-I", incdir)

// Definitely want -fPIC but on Windows gcc complains
// "-fPIC ignored for target (all code is position independent)"
@@ -2658,12 +2637,20 @@

// envList returns the value of the given environment variable broken
// into fields, using the default value when the variable is empty.
+//
+// The environment variable must be quoted correctly for
+// str.SplitQuotedFields. This should be done before building
+// anything, for example, in BuildInit.
func envList(key, def string) []string {
v := cfg.Getenv(key)
if v == "" {
v = def
}
- return strings.Fields(v)
+ args, err := str.SplitQuotedFields(v)
+ if err != nil {
+ panic(fmt.Sprintf("could not parse environment variable %s with value %q: %v", key, v, err))
+ }
+ return args
}

// CFlags returns the flags to use when invoking the C, C++ or Fortran compilers, or cgo.
diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go
index 70ca5d6..74e14d0 100644
--- a/src/cmd/go/internal/work/gc.go
+++ b/src/cmd/go/internal/work/gc.go
@@ -545,33 +545,18 @@
}

// setextld sets the appropriate linker flags for the specified compiler.
-func setextld(ldflags []string, compiler []string) []string {
+func setextld(ldflags []string, compiler []string) ([]string, error) {
for _, f := range ldflags {
if f == "-extld" || strings.HasPrefix(f, "-extld=") {
// don't override -extld if supplied
- return ldflags
+ return ldflags, nil
}
}
- ldflags = append(ldflags, "-extld="+compiler[0])
- if len(compiler) > 1 {
- extldflags := false
- add := strings.Join(compiler[1:], " ")
- for i, f := range ldflags {
- if f == "-extldflags" && i+1 < len(ldflags) {
- ldflags[i+1] = add + " " + ldflags[i+1]
- extldflags = true
- break
- } else if strings.HasPrefix(f, "-extldflags=") {
- ldflags[i] = "-extldflags=" + add + " " + ldflags[i][len("-extldflags="):]
- extldflags = true
- break
- }
- }
- if !extldflags {
- ldflags = append(ldflags, "-extldflags="+add)
- }
+ joined, err := str.JoinAndQuoteFields(compiler)
+ if err != nil {
+ return nil, err
}
- return ldflags
+ return append(ldflags, "-extld="+joined), nil
}

// pluginPath computes the package path for a plugin main package.
@@ -658,7 +643,10 @@
}
ldflags = append(ldflags, forcedLdflags...)
ldflags = append(ldflags, root.Package.Internal.Ldflags...)
- ldflags = setextld(ldflags, compiler)
+ ldflags, err := setextld(ldflags, compiler)
+ if err != nil {
+ return err
+ }

// On OS X when using external linking to build a shared library,
// the argument passed here to -o ends up recorded in the final
@@ -702,7 +690,10 @@
} else {
compiler = envList("CC", cfg.DefaultCC(cfg.Goos, cfg.Goarch))
}
- ldflags = setextld(ldflags, compiler)
+ ldflags, err := setextld(ldflags, compiler)
+ if err != nil {
+ return err
+ }
for _, d := range toplevelactions {
if !strings.HasSuffix(d.Target, ".a") { // omit unsafe etc and actions for other shared libraries
continue
diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go
index 37a3e2d..0221373 100644
--- a/src/cmd/go/internal/work/init.go
+++ b/src/cmd/go/internal/work/init.go
@@ -11,6 +11,7 @@
"cmd/go/internal/cfg"
"cmd/go/internal/fsys"
"cmd/go/internal/modload"
+ "cmd/internal/str"
"cmd/internal/sys"
"flag"
"fmt"
@@ -39,9 +40,18 @@
cfg.BuildPkgdir = p
}

- // Make sure CC and CXX are absolute paths
- for _, key := range []string{"CC", "CXX"} {
- if path := cfg.Getenv(key); !filepath.IsAbs(path) && path != "" && path != filepath.Base(path) {
+ // Make sure CC, CXX, and FC are absolute paths.
+ for _, key := range []string{"CC", "CXX", "FC"} {
+ value := cfg.Getenv(key)
+ args, err := str.SplitQuotedFields(value)
+ if err != nil {
+ base.Fatalf("go %s: %s environment variable could not be parsed: %v", flag.Args()[0], key, err)
+ }
+ if len(args) == 0 {
+ continue
+ }
+ path := args[0]
+ if !filepath.IsAbs(path) && path != filepath.Base(path) {
base.Fatalf("go %s: %s environment variable is relative; must be absolute path: %s\n", flag.Args()[0], key, path)
}
}
diff --git a/src/cmd/go/script_test.go b/src/cmd/go/script_test.go
index 9ca297e..8a7c77a 100644
--- a/src/cmd/go/script_test.go
+++ b/src/cmd/go/script_test.go
@@ -184,6 +184,7 @@
"devnull=" + os.DevNull,
"goversion=" + goVersion(ts),
":=" + string(os.PathListSeparator),
+ "/=" + string(os.PathSeparator),
}
if !testenv.HasExternalNetwork() {
ts.env = append(ts.env, "TESTGONETWORK=panic", "TESTGOVCS=panic")
diff --git a/src/cmd/go/testdata/script/cgo_path_space_quote.txt b/src/cmd/go/testdata/script/cgo_path_space_quote.txt
new file mode 100644
index 0000000..3b89bfb
--- /dev/null
+++ b/src/cmd/go/testdata/script/cgo_path_space_quote.txt
@@ -0,0 +1,56 @@
+# This test checks that the CC environment variable may contain quotes and
+# spaces. Arguments are normally split on spaces, tabs, newlines. If an
+# argument contains these characters, the entire argument may be quoted
+# with single or double quotes. This is the same as -gcflags and similar
+# options.
+
+[short] skip
+[!exec:clang] [!exec:gcc] skip
+
+env GOENV=$WORK/go.env
+mkdir 'program files'
+go build -o 'program files' './which cc/which cc.go'
+[exec:clang] env CC='"'$PWD${/}program' 'files${/}which' 'cc"' 'clang
+[!exec:clang] env CC='"'$PWD${/}program' 'files${/}which' 'cc"' 'gcc
+go env CC
+stdout 'program files[/\\]which cc" (clang|gcc)$'
+go env -w CC=$CC
+env CC=
+go env CC
+stdout 'program files[/\\]which cc" (clang|gcc)$'
+
+go run .
+
+-- go.mod --
+module test
+
+go 1.17
+-- which cc/which cc.go --
+package main
+
+import (
+ "fmt"
+ "os"
+ "os/exec"
+)
+
+func main() {
+ args := append([]string{"-DWRAPPER_WAS_USED=1"}, os.Args[2:]...)
+ cmd := exec.Command(os.Args[1], args...)
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ if err := cmd.Run(); err != nil {
+ fmt.Fprintln(os.Stderr, err)
+ os.Exit(1)
+ }
+}
+-- hello.go --
+package main
+
+// int x = WRAPPER_WAS_USED;
+import "C"
+import "fmt"
+
+func main() {
+ fmt.Println(C.x)
+}
diff --git a/src/cmd/internal/dwarf/dwarf.go b/src/cmd/internal/dwarf/dwarf.go
index 860c7d6..4e163db 100644
--- a/src/cmd/internal/dwarf/dwarf.go
+++ b/src/cmd/internal/dwarf/dwarf.go
@@ -1617,8 +1617,10 @@
// current extld.
// AIX ld doesn't support DWARF with -bnoobjreorder with version
// prior to 7.2.2.
-func IsDWARFEnabledOnAIXLd(extld string) (bool, error) {
- out, err := exec.Command(extld, "-Wl,-V").CombinedOutput()
+func IsDWARFEnabledOnAIXLd(extld []string) (bool, error) {
+ name, args := extld[0], extld[1:]
+ args = append(args, "-Wl,-V")
+ out, err := exec.Command(name, args...).CombinedOutput()
if err != nil {
// The normal output should display ld version and
// then fails because ".main" is not defined:
diff --git a/src/cmd/link/dwarf_test.go b/src/cmd/link/dwarf_test.go
index 3ca59bd..f7bbb01 100644
--- a/src/cmd/link/dwarf_test.go
+++ b/src/cmd/link/dwarf_test.go
@@ -8,6 +8,7 @@
"bytes"
cmddwarf "cmd/internal/dwarf"
"cmd/internal/objfile"
+ "cmd/internal/str"
"debug/dwarf"
"internal/testenv"
"os"
@@ -67,8 +68,11 @@
if extld == "" {
extld = "gcc"
}
- var err error
- expectDWARF, err = cmddwarf.IsDWARFEnabledOnAIXLd(extld)
+ extldArgs, err := str.SplitQuotedFields(extld)
+ if err != nil {
+ t.Fatal(err)
+ }
+ expectDWARF, err = cmddwarf.IsDWARFEnabledOnAIXLd(extldArgs)
if err != nil {
t.Fatal(err)
}
diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index 644faeb..4cfee4a 100644
--- a/src/cmd/link/internal/ld/lib.go
+++ b/src/cmd/link/internal/ld/lib.go
@@ -464,23 +464,24 @@
}

// extld returns the current external linker.
-func (ctxt *Link) extld() string {
- if *flagExtld == "" {
- *flagExtld = "gcc"
+func (ctxt *Link) extld() []string {
+ if len(flagExtld) == 0 {
+ flagExtld = []string{"gcc"}
}
- return *flagExtld
+ return flagExtld
}

// findLibPathCmd uses cmd command to find gcc library libname.
// It returns library full path if found, or "none" if not found.
func (ctxt *Link) findLibPathCmd(cmd, libname string) string {
extld := ctxt.extld()
- args := hostlinkArchArgs(ctxt.Arch)
+ name, args := extld[0], extld[1:]
+ args = append(args, hostlinkArchArgs(ctxt.Arch)...)
args = append(args, cmd)
if ctxt.Debugvlog != 0 {
ctxt.Logf("%s %v\n", extld, args)
}
- out, err := exec.Command(extld, args...).Output()
+ out, err := exec.Command(name, args...).Output()
if err != nil {
if ctxt.Debugvlog != 0 {
ctxt.Logf("not using a %s file because compiler failed\n%v\n%s\n", libname, err, out)
@@ -1240,7 +1241,7 @@
}

var argv []string
- argv = append(argv, ctxt.extld())
+ argv = append(argv, ctxt.extld()...)
argv = append(argv, hostlinkArchArgs(ctxt.Arch)...)

if *FlagS || debug_s {
@@ -1401,7 +1402,9 @@
// If gold is not installed, gcc will silently switch
// back to ld.bfd. So we parse the version information
// and provide a useful error if gold is missing.
- cmd := exec.Command(*flagExtld, "-fuse-ld=gold", "-Wl,--version")
+ name, args := flagExtld[0], flagExtld[1:]
+ args = append(args, "-fuse-ld=gold", "-Wl,--version")
+ cmd := exec.Command(name, args...)
if out, err := cmd.CombinedOutput(); err == nil {
if !bytes.Contains(out, []byte("GNU gold")) {
log.Fatalf("ARM external linker must be gold (issue #15696), but is not: %s", out)
@@ -1414,7 +1417,9 @@
altLinker = "bfd"

// Provide a useful error if ld.bfd is missing.
- cmd := exec.Command(*flagExtld, "-fuse-ld=bfd", "-Wl,--version")
+ name, args := flagExtld[0], flagExtld[1:]
+ args = append(args, "-fuse-ld=bfd", "-Wl,--version")
+ cmd := exec.Command(name, args...)
if out, err := cmd.CombinedOutput(); err == nil {
if !bytes.Contains(out, []byte("GNU ld")) {
log.Fatalf("ARM64 external linker must be ld.bfd (issue #35197), please install devel/binutils")
@@ -1482,10 +1487,11 @@
argv = append(argv, "/lib/crt0_64.o")

extld := ctxt.extld()
+ name, args := extld[0], extld[1:]
// Get starting files.
getPathFile := func(file string) string {
- args := []string{"-maix64", "--print-file-name=" + file}
- out, err := exec.Command(extld, args...).CombinedOutput()
+ args := append(args, "-maix64", "--print-file-name="+file)
+ out, err := exec.Command(name, args...).CombinedOutput()
if err != nil {
log.Fatalf("running %s failed: %v\n%s", extld, err, out)
}
@@ -1567,14 +1573,18 @@
}
}

- for _, p := range strings.Fields(*flagExtldflags) {
+ for _, p := range flagExtldflags {
argv = append(argv, p)
checkStatic(p)
}
if ctxt.HeadType == objabi.Hwindows {
// Determine which linker we're using. Add in the extldflags in
// case used has specified "-fuse-ld=...".
- cmd := exec.Command(*flagExtld, *flagExtldflags, "-Wl,--version")
+ extld := ctxt.extld()
+ name, args := extld[0], extld[1:]
+ args = append(args, flagExtldflags...)
+ args = append(args, "-Wl,--version")
+ cmd := exec.Command(name, args...)
usingLLD := false
if out, err := cmd.CombinedOutput(); err == nil {
if bytes.Contains(out, []byte("LLD ")) {
@@ -1718,8 +1728,7 @@
flags := hostlinkArchArgs(arch)
keep := false
skip := false
- extldflags := strings.Fields(*flagExtldflags)
- for _, f := range append(extldflags, ldflag...) {
+ for _, f := range append(flagExtldflags, ldflag...) {
if keep {
flags = append(flags, f)
keep = false
diff --git a/src/cmd/link/internal/ld/main.go b/src/cmd/link/internal/ld/main.go
index cba0e3d..33b03b5 100644
--- a/src/cmd/link/internal/ld/main.go
+++ b/src/cmd/link/internal/ld/main.go
@@ -34,6 +34,7 @@
"bufio"
"cmd/internal/goobj"
"cmd/internal/objabi"
+ "cmd/internal/str"
"cmd/internal/sys"
"cmd/link/internal/benchmark"
"flag"
@@ -53,6 +54,8 @@

func init() {
flag.Var(&rpath, "r", "set the ELF dynamic linker search `path` to dir1:dir2:...")
+ flag.Var(&flagExtld, "extld", "use `linker` when linking in external mode")
+ flag.Var(&flagExtldflags, "extldflags", "pass `flags` to external linker")
}

// Flags used by the linker. The exported flags are used by the architecture-specific packages.
@@ -72,8 +75,8 @@
flagLibGCC = flag.String("libgcc", "", "compiler support lib for internal linking; use \"none\" to disable")
flagTmpdir = flag.String("tmpdir", "", "use `directory` for temporary files")

- flagExtld = flag.String("extld", "", "use `linker` when linking in external mode")
- flagExtldflags = flag.String("extldflags", "", "pass `flags` to external linker")
+ flagExtld str.QuotedStringListFlag
+ flagExtldflags str.QuotedStringListFlag
flagExtar = flag.String("extar", "", "archive program for buildmode=c-archive")

flagA = flag.Bool("a", false, "no-op (deprecated)")
diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go
index fed9c7b..2b0b2dc 100644
--- a/src/cmd/link/link_test.go
+++ b/src/cmd/link/link_test.go
@@ -282,8 +282,8 @@
"-isysroot", strings.TrimSpace(string(sdkPath)),
"-mtvos-version-min=12.0",
"-fembed-bitcode",
- "-framework", "CoreFoundation",
}
+ CGO_LDFLAGS := []string{"-framework", "CoreFoundation"}
lib := filepath.Join("testdata", "testBuildFortvOS", "lib.go")
tmpDir := t.TempDir()

@@ -295,12 +295,14 @@
"GOARCH=arm64",
"CC="+strings.Join(CC, " "),
"CGO_CFLAGS=", // ensure CGO_CFLAGS does not contain any flags. Issue #35459
+ "CGO_LDFLAGS="+strings.Join(CGO_LDFLAGS, " "),
)
if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("%v: %v:\n%s", cmd.Args, err, out)
}

link := exec.Command(CC[0], CC[1:]...)
+ link.Args = append(link.Args, CGO_LDFLAGS...)
link.Args = append(link.Args, "-o", filepath.Join(tmpDir, "a.out")) // Avoid writing to package directory.
link.Args = append(link.Args, ar, filepath.Join("testdata", "testBuildFortvOS", "main.m"))
if out, err := link.CombinedOutput(); err != nil {

To view, visit change 341936. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I2d5d89ddb19c94adef65982a8137b01f037d5c11
Gerrit-Change-Number: 341936
Gerrit-PatchSet: 1
Gerrit-Owner: Jay Conrod <jayc...@google.com>
Gerrit-Reviewer: Michael Matloob <mat...@golang.org>
Gerrit-Attention: Michael Matloob <mat...@golang.org>
Gerrit-MessageType: newchange

Jay Conrod (Gerrit)

unread,
Aug 12, 2021, 6:43:49 PM8/12/21
to goph...@pubsubhelper.golang.org, Bryan C. Mills, Michael Matloob, golang-co...@googlegroups.com

Attention is currently required from: Bryan C. Mills, Michael Matloob.

Patch set 1:Run-TryBot +1Trust +1

View Change

    To view, visit change 341936. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I2d5d89ddb19c94adef65982a8137b01f037d5c11
    Gerrit-Change-Number: 341936
    Gerrit-PatchSet: 1
    Gerrit-Owner: Jay Conrod <jayc...@google.com>
    Gerrit-Reviewer: Bryan C. Mills <bcm...@google.com>
    Gerrit-Reviewer: Jay Conrod <jayc...@google.com>
    Gerrit-Reviewer: Michael Matloob <mat...@golang.org>
    Gerrit-Attention: Bryan C. Mills <bcm...@google.com>
    Gerrit-Attention: Michael Matloob <mat...@golang.org>
    Gerrit-Comment-Date: Thu, 12 Aug 2021 22:43:44 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Jay Conrod (Gerrit)

    unread,
    Aug 12, 2021, 8:23:30 PM8/12/21
    to goph...@pubsubhelper.golang.org, Go Bot, Bryan C. Mills, Michael Matloob, golang-co...@googlegroups.com

    Attention is currently required from: Bryan C. Mills, Michael Matloob.

    View Change

    1 comment:

    To view, visit change 341936. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I2d5d89ddb19c94adef65982a8137b01f037d5c11
    Gerrit-Change-Number: 341936
    Gerrit-PatchSet: 1
    Gerrit-Owner: Jay Conrod <jayc...@google.com>
    Gerrit-Reviewer: Bryan C. Mills <bcm...@google.com>
    Gerrit-Reviewer: Go Bot <go...@golang.org>
    Gerrit-Reviewer: Jay Conrod <jayc...@google.com>
    Gerrit-Reviewer: Michael Matloob <mat...@golang.org>
    Gerrit-Attention: Bryan C. Mills <bcm...@google.com>
    Gerrit-Attention: Michael Matloob <mat...@golang.org>
    Gerrit-Comment-Date: Fri, 13 Aug 2021 00:23:23 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Go Bot <go...@golang.org>
    Gerrit-MessageType: comment

    Bryan C. Mills (Gerrit)

    unread,
    Aug 12, 2021, 9:56:38 PM8/12/21
    to Jay Conrod, goph...@pubsubhelper.golang.org, Bryan C. Mills, Go Bot, Michael Matloob, golang-co...@googlegroups.com

    Attention is currently required from: Jay Conrod, Michael Matloob.

    Patch set 1:Code-Review +2

    View Change

      To view, visit change 341936. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I2d5d89ddb19c94adef65982a8137b01f037d5c11
      Gerrit-Change-Number: 341936
      Gerrit-PatchSet: 1
      Gerrit-Owner: Jay Conrod <jayc...@google.com>
      Gerrit-Reviewer: Bryan C. Mills <bcm...@google.com>
      Gerrit-Reviewer: Go Bot <go...@golang.org>
      Gerrit-Reviewer: Jay Conrod <jayc...@google.com>
      Gerrit-Reviewer: Michael Matloob <mat...@golang.org>
      Gerrit-Attention: Jay Conrod <jayc...@google.com>
      Gerrit-Attention: Michael Matloob <mat...@golang.org>
      Gerrit-Comment-Date: Fri, 13 Aug 2021 01:56:33 +0000

      Jay Conrod (Gerrit)

      unread,
      Aug 16, 2021, 3:01:00 PM8/16/21
      to goph...@pubsubhelper.golang.org, Bryan C. Mills, Go Bot, Michael Matloob, golang-co...@googlegroups.com

      Attention is currently required from: Michael Matloob.

      Patch set 2:Run-TryBot +1Trust +1

      View Change

        To view, visit change 341936. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I2d5d89ddb19c94adef65982a8137b01f037d5c11
        Gerrit-Change-Number: 341936
        Gerrit-PatchSet: 2
        Gerrit-Owner: Jay Conrod <jayc...@google.com>
        Gerrit-Reviewer: Bryan C. Mills <bcm...@google.com>
        Gerrit-Reviewer: Go Bot <go...@golang.org>
        Gerrit-Reviewer: Jay Conrod <jayc...@google.com>
        Gerrit-Reviewer: Michael Matloob <mat...@golang.org>
        Gerrit-Attention: Michael Matloob <mat...@golang.org>
        Gerrit-Comment-Date: Mon, 16 Aug 2021 19:00:55 +0000

        Jay Conrod (Gerrit)

        unread,
        Aug 16, 2021, 4:23:17 PM8/16/21
        to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go Bot, Bryan C. Mills, Michael Matloob, golang-co...@googlegroups.com

        Jay Conrod submitted this change.

        View Change

        Approvals: Bryan C. Mills: Looks good to me, approved Jay Conrod: Trusted; Run TryBots Go Bot: TryBots succeeded
        Reviewed-on: https://go-review.googlesource.com/c/go/+/341936
        Reviewed-by: Bryan C. Mills <bcm...@google.com>
        index 894e5af..494fea5 100644
        @@ -1242,7 +1243,7 @@

        }

        var argv []string
        - argv = append(argv, ctxt.extld())
        + argv = append(argv, ctxt.extld()...)
        argv = append(argv, hostlinkArchArgs(ctxt.Arch)...)

        if *FlagS || debug_s {
        @@ -1403,7 +1404,9 @@

        // If gold is not installed, gcc will silently switch
        // back to ld.bfd. So we parse the version information
        // and provide a useful error if gold is missing.
        - cmd := exec.Command(*flagExtld, "-fuse-ld=gold", "-Wl,--version")
        + name, args := flagExtld[0], flagExtld[1:]
        + args = append(args, "-fuse-ld=gold", "-Wl,--version")
        + cmd := exec.Command(name, args...)
        if out, err := cmd.CombinedOutput(); err == nil {
        if !bytes.Contains(out, []byte("GNU gold")) {
        log.Fatalf("ARM external linker must be gold (issue #15696), but is not: %s", out)
        @@ -1416,7 +1419,9 @@

        altLinker = "bfd"

        // Provide a useful error if ld.bfd is missing.
        - cmd := exec.Command(*flagExtld, "-fuse-ld=bfd", "-Wl,--version")
        + name, args := flagExtld[0], flagExtld[1:]
        + args = append(args, "-fuse-ld=bfd", "-Wl,--version")
        + cmd := exec.Command(name, args...)
        if out, err := cmd.CombinedOutput(); err == nil {
        if !bytes.Contains(out, []byte("GNU ld")) {
        log.Fatalf("ARM64 external linker must be ld.bfd (issue #35197), please install devel/binutils")
        @@ -1484,10 +1489,11 @@

        argv = append(argv, "/lib/crt0_64.o")

        extld := ctxt.extld()
        + name, args := extld[0], extld[1:]
        // Get starting files.
        getPathFile := func(file string) string {
        - args := []string{"-maix64", "--print-file-name=" + file}
        - out, err := exec.Command(extld, args...).CombinedOutput()
        + args := append(args, "-maix64", "--print-file-name="+file)
        + out, err := exec.Command(name, args...).CombinedOutput()
        if err != nil {
        log.Fatalf("running %s failed: %v\n%s", extld, err, out)
        }
        @@ -1569,14 +1575,18 @@

        }
        }

        - for _, p := range strings.Fields(*flagExtldflags) {
        + for _, p := range flagExtldflags {
        argv = append(argv, p)
        checkStatic(p)
        }
        if ctxt.HeadType == objabi.Hwindows {
        // Determine which linker we're using. Add in the extldflags in
        // case used has specified "-fuse-ld=...".
        - cmd := exec.Command(*flagExtld, *flagExtldflags, "-Wl,--version")
        + extld := ctxt.extld()
        + name, args := extld[0], extld[1:]
        + args = append(args, flagExtldflags...)
        + args = append(args, "-Wl,--version")
        + cmd := exec.Command(name, args...)
        usingLLD := false
        if out, err := cmd.CombinedOutput(); err == nil {
        if bytes.Contains(out, []byte("LLD ")) {
        @@ -1720,8 +1730,7 @@
        1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one.

        To view, visit change 341936. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I2d5d89ddb19c94adef65982a8137b01f037d5c11
        Gerrit-Change-Number: 341936
        Gerrit-PatchSet: 3
        Gerrit-Owner: Jay Conrod <jayc...@google.com>
        Gerrit-Reviewer: Bryan C. Mills <bcm...@google.com>
        Gerrit-Reviewer: Go Bot <go...@golang.org>
        Gerrit-Reviewer: Jay Conrod <jayc...@google.com>
        Gerrit-Reviewer: Michael Matloob <mat...@golang.org>
        Gerrit-MessageType: merged
        Reply all
        Reply to author
        Forward
        0 new messages