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 <jayconrod@google.com>
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/341936
Reviewed-by: Bryan C. Mills <bcmills@google.com>

4 files changed, 38 insertions, 20 deletions

diff --git a/src/cmd/link/dwarf_test.go b/src/cmd/link/dwarf_test.go
index 3ca59bd47f..f7bbb014d9 100644
--- a/src/cmd/link/dwarf_test.go
+++ b/src/cmd/link/dwarf_test.go
@@ -8,6 +8,7 @@ import (
 	"bytes"
 	cmddwarf "cmd/internal/dwarf"
 	"cmd/internal/objfile"
+	"cmd/internal/str"
 	"debug/dwarf"
 	"internal/testenv"
 	"os"
@@ -67,8 +68,11 @@ func testDWARF(t *testing.T, buildmode string, expectDWARF bool, env ...string)
 			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 894e5afe63..494fea5e72 100644
--- a/src/cmd/link/internal/ld/lib.go
+++ b/src/cmd/link/internal/ld/lib.go
@@ -464,23 +464,24 @@ func loadinternal(ctxt *Link, name string) *sym.Library {
 }
 
 // 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)
@@ -1242,7 +1243,7 @@ func (ctxt *Link) hostlink() {
 	}
 
 	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 @@ func (ctxt *Link) hostlink() {
 			// 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 @@ func (ctxt *Link) hostlink() {
 		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 @@ func (ctxt *Link) hostlink() {
 		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 @@ func (ctxt *Link) hostlink() {
 		}
 	}
 
-	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 @@ func linkerFlagSupported(arch *sys.Arch, linker, altLinker, flag string) bool {
 	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 cba0e3d81f..33b03b5024 100644
--- a/src/cmd/link/internal/ld/main.go
+++ b/src/cmd/link/internal/ld/main.go
@@ -34,6 +34,7 @@ import (
 	"bufio"
 	"cmd/internal/goobj"
 	"cmd/internal/objabi"
+	"cmd/internal/str"
 	"cmd/internal/sys"
 	"cmd/link/internal/benchmark"
 	"flag"
@@ -53,6 +54,8 @@ var (
 
 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 @@ var (
 	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 fed9c7bc3f..2b0b2dc4a1 100644
--- a/src/cmd/link/link_test.go
+++ b/src/cmd/link/link_test.go
@@ -282,8 +282,8 @@ func TestBuildForTvOS(t *testing.T) {
 		"-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 @@ func TestBuildForTvOS(t *testing.T) {
 		"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 {