diff options
author | Jay Conrod <jayconrod@google.com> | 2021-01-22 14:27:24 -0500 |
---|---|---|
committer | Jay Conrod <jayconrod@google.com> | 2021-01-22 21:16:10 +0000 |
commit | a2cef9b544708ecae983ed8836ee2425a28aab68 (patch) | |
tree | 80ebd937e34ba17a9e66005f5eadf9ee441f6bc6 | |
parent | dab3e5affefb7e0b157ad27fe4797c4c6f3c4ea6 (diff) | |
download | go-a2cef9b544708ecae983ed8836ee2425a28aab68.tar.gz go-a2cef9b544708ecae983ed8836ee2425a28aab68.zip |
cmd/go: don't lookup the path for CC when invoking cgo
Previously, if CC was a path without separators (like gcc or clang),
we'd look it up in PATH in cmd/go using internal/execabs.LookPath,
then pass the resolved path to cgo in CC.
This caused a regression: if the directory in PATH containing CC has a
space, cgo splits it and interprets it as multiple arguments.
With this change, cmd/go no longer resolves CC before invoking
cgo. cgo does the path lookup on each invocation. This reverts the
security fix CL 284780, but that was redundant with the addition of
internal/execabs (CL 955304), which still protects us.
Fixes #43808
Updates #41400
Change-Id: I65d91a1e303856df8653881eb6e2e75a3bf95c49
Reviewed-on: https://go-review.googlesource.com/c/go/+/285873
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
-rw-r--r-- | src/cmd/go/internal/work/action.go | 3 | ||||
-rw-r--r-- | src/cmd/go/internal/work/exec.go | 27 | ||||
-rw-r--r-- | src/cmd/go/testdata/script/cgo_path.txt | 5 | ||||
-rw-r--r-- | src/cmd/go/testdata/script/cgo_path_space.txt | 55 |
4 files changed, 64 insertions, 26 deletions
diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go index b071ed1400..9d141ae233 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -57,9 +57,6 @@ type Builder struct { id sync.Mutex toolIDCache map[string]string // tool name -> tool ID buildIDCache map[string]string // file name -> build ID - - cgoEnvOnce sync.Once - cgoEnvCache []string } // NOTE: Much of Action would not need to be exported if not for test. diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index cacb4c05df..422e83c224 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -1165,7 +1165,10 @@ func (b *Builder) vet(ctx context.Context, a *Action) error { } // TODO(rsc): Why do we pass $GCCGO to go vet? - env := b.cgoEnv() + env := b.cCompilerEnv() + if cfg.BuildToolchainName == "gccgo" { + env = append(env, "GCCGO="+BuildToolchain.compiler()) + } p := a.Package tool := VetTool @@ -2111,24 +2114,6 @@ func (b *Builder) cCompilerEnv() []string { return []string{"TERM=dumb"} } -// cgoEnv returns environment variables to set when running cgo. -// Some of these pass through to cgo running the C compiler, -// so it includes cCompilerEnv. -func (b *Builder) cgoEnv() []string { - b.cgoEnvOnce.Do(func() { - cc, err := exec.LookPath(b.ccExe()[0]) - if err != nil || filepath.Base(cc) == cc { // reject relative path - cc = "/missing-cc" - } - gccgo := GccgoBin - if filepath.Base(gccgo) == gccgo { // reject relative path - gccgo = "/missing-gccgo" - } - b.cgoEnvCache = append(b.cCompilerEnv(), "CC="+cc, "GCCGO="+gccgo) - }) - return b.cgoEnvCache -} - // mkdir makes the named directory. func (b *Builder) Mkdir(dir string) error { // Make Mkdir(a.Objdir) a no-op instead of an error when a.Objdir == "". @@ -2729,7 +2714,7 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo // along to the host linker. At this point in the code, cgoLDFLAGS // consists of the original $CGO_LDFLAGS (unchecked) and all the // flags put together from source code (checked). - cgoenv := b.cgoEnv() + cgoenv := b.cCompilerEnv() if len(cgoLDFLAGS) > 0 { flags := make([]string, len(cgoLDFLAGS)) for i, f := range cgoLDFLAGS { @@ -2966,7 +2951,7 @@ func (b *Builder) dynimport(a *Action, p *load.Package, objdir, importGo, cgoExe if p.Standard && p.ImportPath == "runtime/cgo" { cgoflags = []string{"-dynlinker"} // record path to dynamic linker } - return b.run(a, base.Cwd, p.ImportPath, b.cgoEnv(), cfg.BuildToolexec, cgoExe, "-dynpackage", p.Name, "-dynimport", dynobj, "-dynout", importGo, cgoflags) + return b.run(a, base.Cwd, p.ImportPath, b.cCompilerEnv(), cfg.BuildToolexec, cgoExe, "-dynpackage", p.Name, "-dynimport", dynobj, "-dynout", importGo, cgoflags) } // Run SWIG on all SWIG input files. diff --git a/src/cmd/go/testdata/script/cgo_path.txt b/src/cmd/go/testdata/script/cgo_path.txt index 0d15998426..98c56ff40e 100644 --- a/src/cmd/go/testdata/script/cgo_path.txt +++ b/src/cmd/go/testdata/script/cgo_path.txt @@ -2,11 +2,12 @@ env GOCACHE=$WORK/gocache # Looking for compile flags, so need a clean cache. [!windows] env PATH=.:$PATH -[!windows] chmod 0777 p/gcc p/clang +[!windows] chmod 0755 p/gcc p/clang [!windows] exists -exec p/gcc p/clang [windows] exists -exec p/gcc.bat p/clang.bat ! exists p/bug.txt -go build -x +! go build -x +stderr '^cgo: exec (clang|gcc): (clang|gcc) resolves to executable relative to current directory \(.[/\\](clang|gcc)(.bat)?\)$' ! exists p/bug.txt -- go.mod -- diff --git a/src/cmd/go/testdata/script/cgo_path_space.txt b/src/cmd/go/testdata/script/cgo_path_space.txt new file mode 100644 index 0000000000..6d203b04d6 --- /dev/null +++ b/src/cmd/go/testdata/script/cgo_path_space.txt @@ -0,0 +1,55 @@ +# Check that if the PATH directory containing the C compiler has a space, +# we can still use that compiler with cgo. +# Verifies #43808. + +[!cgo] skip + +# Check if default CC was set by make.bash. +# If it was, this test is not valid. +go env CC +stdout '^(clang|gcc)$' + +[!windows] chmod 0755 $WORK/'program files'/clang +[!windows] chmod 0755 $WORK/'program files'/gcc +[!windows] exists -exec $WORK/'program files'/clang +[!windows] exists -exec $WORK/'program files'/gcc +[!windows] env PATH=$WORK/'program files':$PATH +[windows] exists -exec $WORK/'program files'/gcc.bat +[windows] exists -exec $WORK/'program files'/clang.bat +[windows] env PATH=$WORK\'program files';%PATH% + +! exists log.txt +? go build -x +exists log.txt +rm log.txt + +# TODO(#41400, #43078): when CC is set explicitly, it should be allowed to +# contain spaces separating arguments, and it should be possible to quote +# arguments with spaces (including the path), as in CGO_CFLAGS and other +# variables. For now, this doesn't work. +[!windows] env CC=$WORK/'program files'/gcc +[windows] env CC=$WORK\'program files'\gcc.bat +! go build -x +! exists log.txt + +-- go.mod -- +module m + +-- m.go -- +package m + +// #define X 1 +import "C" + +-- $WORK/program files/gcc -- +#!/bin/sh + +echo ok >log.txt +-- $WORK/program files/clang -- +#!/bin/sh + +echo ok >log.txt +-- $WORK/program files/gcc.bat -- +echo ok >log.txt +-- $WORK/program files/clang.bat -- +echo ok >log.txt |