Skip to content

Commit c71eedf

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored andcommitted
cmd/go: accept clang versions with vendor prefixes
To better diagnose bugs like this one in the future, I think we should also refuse to use a C compiler if we can't identify a sensible version for it. I did not do that in this CL because I want it to be small and low-risk for possible backporting. Fixes #64423. Change-Id: I21e44fc55f6fcf76633e4fecf6400c226a742351 Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest Reviewed-on: https://go-review.googlesource.com/c/go/+/547998 Auto-Submit: Bryan Mills <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
1 parent 77e76c4 commit c71eedf

File tree

2 files changed

+136
-2
lines changed

2 files changed

+136
-2
lines changed

src/cmd/go/internal/work/buildid.go

+15-2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"os"
1111
"os/exec"
12+
"regexp"
1213
"strings"
1314

1415
"cmd/go/internal/base"
@@ -236,10 +237,22 @@ func (b *Builder) gccToolID(name, language string) (id, exe string, err error) {
236237
}
237238

238239
version := ""
240+
gccVersionRE := regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+`)
239241
lines := strings.Split(string(out), "\n")
240242
for _, line := range lines {
241-
if fields := strings.Fields(line); len(fields) > 1 && fields[1] == "version" || len(fields) > 2 && fields[2] == "version" {
242-
version = line
243+
fields := strings.Fields(line)
244+
for i, field := range fields {
245+
if strings.HasSuffix(field, ":") {
246+
// Avoid parsing fields of lines like "Configured with: …", which may
247+
// contain arbitrary substrings.
248+
break
249+
}
250+
if field == "version" && i < len(fields)-1 && gccVersionRE.MatchString(fields[i+1]) {
251+
version = line
252+
break
253+
}
254+
}
255+
if version != "" {
243256
break
244257
}
245258
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
# Regression test for https://go.dev/issue/64423:
2+
#
3+
# When we parse the version for a Clang binary, we should accept
4+
# an arbitrary vendor prefix, which (as of 2023) may be injected
5+
# by defining CLANG_VENDOR when building clang itself.
6+
#
7+
# Since we don't want to actually rebuild the Clang toolchain in
8+
# this test, we instead simulate it by injecting a fake "clang"
9+
# binary that runs the real one as a subprocess.
10+
11+
[!cgo] skip
12+
[short] skip 'builds and links a fake clang binary'
13+
[!cc:clang] skip 'test is specific to clang version parsing'
14+
15+
# Save the location of the real clang command for our fake one to use.
16+
go run ./which clang
17+
cp stdout $WORK/.realclang
18+
19+
# Build a fake clang and ensure that it is the one in $PATH.
20+
mkdir $WORK/bin
21+
go build -o $WORK/bin/clang$GOEXE ./fakeclang
22+
[!GOOS:plan9] env PATH=$WORK${/}bin
23+
[GOOS:plan9] env path=$WORK${/}bin
24+
25+
# Force CGO_ENABLED=1 so that the following commands should error
26+
# out if the fake clang doesn't work.
27+
env CGO_ENABLED=1
28+
29+
# The bug in https://go.dev/issue/64423 resulted in cache keys that
30+
# didn't contain any information about the C compiler.
31+
# Since the bug was in cache key computation, isolate the cache:
32+
# if we change the way caching works, we want the test to fail
33+
# instead of accidentally reusing the cached information from a
34+
# previous test run.
35+
env GOCACHE=$WORK${/}.cache
36+
mkdir $GOCACHE
37+
38+
go build -x runtime/cgo
39+
40+
# Tell our fake clang to stop working.
41+
# Previously, 'go build -x runtime/cgo' would continue to
42+
# succeed because both the broken clang and the non-broken one
43+
# resulted in a cache key with no clang version information.
44+
env GO_BREAK_CLANG=1
45+
! go build -x runtime/cgo
46+
stderr '# runtime/cgo\nGO_BREAK_CLANG is set'
47+
48+
-- go.mod --
49+
module example/issue64423
50+
go 1.20
51+
-- which/main.go --
52+
package main
53+
54+
import (
55+
"os"
56+
"os/exec"
57+
)
58+
59+
func main() {
60+
path, err := exec.LookPath(os.Args[1])
61+
if err != nil {
62+
panic(err)
63+
}
64+
os.Stdout.WriteString(path)
65+
}
66+
-- fakeclang/main.go --
67+
package main
68+
69+
import (
70+
"bufio"
71+
"bytes"
72+
"log"
73+
"os"
74+
"os/exec"
75+
"path/filepath"
76+
"strings"
77+
)
78+
79+
func main() {
80+
if os.Getenv("GO_BREAK_CLANG") != "" {
81+
os.Stderr.WriteString("GO_BREAK_CLANG is set\n")
82+
os.Exit(1)
83+
}
84+
85+
b, err := os.ReadFile(filepath.Join(os.Getenv("WORK"), ".realclang"))
86+
if err != nil {
87+
log.Fatal(err)
88+
}
89+
clang := string(bytes.TrimSpace(b))
90+
cmd := exec.Command(clang, os.Args[1:]...)
91+
cmd.Stdout = os.Stdout
92+
stderr, err := cmd.StderrPipe()
93+
if err != nil {
94+
log.Fatal(err)
95+
}
96+
97+
if err := cmd.Start(); err != nil {
98+
log.Fatal(err)
99+
}
100+
101+
r := bufio.NewReader(stderr)
102+
for {
103+
line, err := r.ReadString('\n')
104+
if line != "" {
105+
if strings.Contains(line, "clang version") {
106+
// Simulate a clang version string with an arbitrary vendor prefix.
107+
const vendorString = "Gopher Solutions Unlimited "
108+
os.Stderr.WriteString(vendorString)
109+
}
110+
os.Stderr.WriteString(line)
111+
}
112+
if err != nil {
113+
break
114+
}
115+
}
116+
os.Stderr.Close()
117+
118+
if err := cmd.Wait(); err != nil {
119+
os.Exit(1)
120+
}
121+
}

0 commit comments

Comments
 (0)