Skip to content

Commit a2cef9b

Browse files
author
Jay Conrod
committed
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 <[email protected]> Run-TryBot: Jay Conrod <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]>
1 parent dab3e5a commit a2cef9b

4 files changed

Lines changed: 64 additions & 26 deletions

File tree

‎src/cmd/go/internal/work/action.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,6 @@ type Builder struct {
5757
id sync.Mutex
5858
toolIDCache map[string]string // tool name -> tool ID
5959
buildIDCache map[string]string // file name -> build ID
60-
61-
cgoEnvOnce sync.Once
62-
cgoEnvCache []string
6360
}
6461

6562
// NOTE: Much of Action would not need to be exported if not for test.

‎src/cmd/go/internal/work/exec.go‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,7 +1165,10 @@ func (b *Builder) vet(ctx context.Context, a *Action) error {
11651165
}
11661166

11671167
// TODO(rsc): Why do we pass $GCCGO to go vet?
1168-
env := b.cgoEnv()
1168+
env := b.cCompilerEnv()
1169+
if cfg.BuildToolchainName == "gccgo" {
1170+
env = append(env, "GCCGO="+BuildToolchain.compiler())
1171+
}
11691172

11701173
p := a.Package
11711174
tool := VetTool
@@ -2111,24 +2114,6 @@ func (b *Builder) cCompilerEnv() []string {
21112114
return []string{"TERM=dumb"}
21122115
}
21132116

2114-
// cgoEnv returns environment variables to set when running cgo.
2115-
// Some of these pass through to cgo running the C compiler,
2116-
// so it includes cCompilerEnv.
2117-
func (b *Builder) cgoEnv() []string {
2118-
b.cgoEnvOnce.Do(func() {
2119-
cc, err := exec.LookPath(b.ccExe()[0])
2120-
if err != nil || filepath.Base(cc) == cc { // reject relative path
2121-
cc = "/missing-cc"
2122-
}
2123-
gccgo := GccgoBin
2124-
if filepath.Base(gccgo) == gccgo { // reject relative path
2125-
gccgo = "/missing-gccgo"
2126-
}
2127-
b.cgoEnvCache = append(b.cCompilerEnv(), "CC="+cc, "GCCGO="+gccgo)
2128-
})
2129-
return b.cgoEnvCache
2130-
}
2131-
21322117
// mkdir makes the named directory.
21332118
func (b *Builder) Mkdir(dir string) error {
21342119
// 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
27292714
// along to the host linker. At this point in the code, cgoLDFLAGS
27302715
// consists of the original $CGO_LDFLAGS (unchecked) and all the
27312716
// flags put together from source code (checked).
2732-
cgoenv := b.cgoEnv()
2717+
cgoenv := b.cCompilerEnv()
27332718
if len(cgoLDFLAGS) > 0 {
27342719
flags := make([]string, len(cgoLDFLAGS))
27352720
for i, f := range cgoLDFLAGS {
@@ -2966,7 +2951,7 @@ func (b *Builder) dynimport(a *Action, p *load.Package, objdir, importGo, cgoExe
29662951
if p.Standard && p.ImportPath == "runtime/cgo" {
29672952
cgoflags = []string{"-dynlinker"} // record path to dynamic linker
29682953
}
2969-
return b.run(a, base.Cwd, p.ImportPath, b.cgoEnv(), cfg.BuildToolexec, cgoExe, "-dynpackage", p.Name, "-dynimport", dynobj, "-dynout", importGo, cgoflags)
2954+
return b.run(a, base.Cwd, p.ImportPath, b.cCompilerEnv(), cfg.BuildToolexec, cgoExe, "-dynpackage", p.Name, "-dynimport", dynobj, "-dynout", importGo, cgoflags)
29702955
}
29712956

29722957
// Run SWIG on all SWIG input files.

‎src/cmd/go/testdata/script/cgo_path.txt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22

33
env GOCACHE=$WORK/gocache # Looking for compile flags, so need a clean cache.
44
[!windows] env PATH=.:$PATH
5-
[!windows] chmod 0777 p/gcc p/clang
5+
[!windows] chmod 0755 p/gcc p/clang
66
[!windows] exists -exec p/gcc p/clang
77
[windows] exists -exec p/gcc.bat p/clang.bat
88
! exists p/bug.txt
9-
go build -x
9+
! go build -x
10+
stderr '^cgo: exec (clang|gcc): (clang|gcc) resolves to executable relative to current directory \(.[/\\](clang|gcc)(.bat)?\)$'
1011
! exists p/bug.txt
1112

1213
-- go.mod --

‎src/cmd/go/testdata/script/cgo_path_space.txt‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Check that if the PATH directory containing the C compiler has a space,
2+
# we can still use that compiler with cgo.
3+
# Verifies #43808.
4+
5+
[!cgo] skip
6+
7+
# Check if default CC was set by make.bash.
8+
# If it was, this test is not valid.
9+
go env CC
10+
stdout '^(clang|gcc)$'
11+
12+
[!windows] chmod 0755 $WORK/'program files'/clang
13+
[!windows] chmod 0755 $WORK/'program files'/gcc
14+
[!windows] exists -exec $WORK/'program files'/clang
15+
[!windows] exists -exec $WORK/'program files'/gcc
16+
[!windows] env PATH=$WORK/'program files':$PATH
17+
[windows] exists -exec $WORK/'program files'/gcc.bat
18+
[windows] exists -exec $WORK/'program files'/clang.bat
19+
[windows] env PATH=$WORK\'program files';%PATH%
20+
21+
! exists log.txt
22+
? go build -x
23+
exists log.txt
24+
rm log.txt
25+
26+
# TODO(#41400, #43078): when CC is set explicitly, it should be allowed to
27+
# contain spaces separating arguments, and it should be possible to quote
28+
# arguments with spaces (including the path), as in CGO_CFLAGS and other
29+
# variables. For now, this doesn't work.
30+
[!windows] env CC=$WORK/'program files'/gcc
31+
[windows] env CC=$WORK\'program files'\gcc.bat
32+
! go build -x
33+
! exists log.txt
34+
35+
-- go.mod --
36+
module m
37+
38+
-- m.go --
39+
package m
40+
41+
// #define X 1
42+
import "C"
43+
44+
-- $WORK/program files/gcc --
45+
#!/bin/sh
46+
47+
echo ok >log.txt
48+
-- $WORK/program files/clang --
49+
#!/bin/sh
50+
51+
echo ok >log.txt
52+
-- $WORK/program files/gcc.bat --
53+
echo ok >log.txt
54+
-- $WORK/program files/clang.bat --
55+
echo ok >log.txt

0 commit comments

Comments
 (0)