From: Jay Conrod Date: Fri, 22 Jan 2021 19:27:24 +0000 (-0500) Subject: cmd/go: don't lookup the path for CC when invoking cgo X-Git-Tag: go1.16rc1~34 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=a2cef9b544708ecae983ed8836ee2425a28aab68;p=gostls13.git 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 Run-TryBot: Jay Conrod TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills --- 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