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 <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/src/cmd/go/internal/work/buildid.go b/src/cmd/go/internal/work/buildid.go
index 276f524..0769443 100644
--- a/src/cmd/go/internal/work/buildid.go
+++ b/src/cmd/go/internal/work/buildid.go
@@ -9,6 +9,7 @@
"fmt"
"os"
"os/exec"
+ "regexp"
"strings"
"cmd/go/internal/base"
@@ -236,10 +237,22 @@
}
version := ""
+ gccVersionRE := regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+`)
lines := strings.Split(string(out), "\n")
for _, line := range lines {
- if fields := strings.Fields(line); len(fields) > 1 && fields[1] == "version" || len(fields) > 2 && fields[2] == "version" {
- version = line
+ fields := strings.Fields(line)
+ for i, field := range fields {
+ if strings.HasSuffix(field, ":") {
+ // Avoid parsing fields of lines like "Configured with: …", which may
+ // contain arbitrary substrings.
+ break
+ }
+ if field == "version" && i < len(fields)-1 && gccVersionRE.MatchString(fields[i+1]) {
+ version = line
+ break
+ }
+ }
+ if version != "" {
break
}
}
diff --git a/src/cmd/go/testdata/script/build_cc_cache_issue64423.txt b/src/cmd/go/testdata/script/build_cc_cache_issue64423.txt
new file mode 100644
index 0000000..f1bc2c3
--- /dev/null
+++ b/src/cmd/go/testdata/script/build_cc_cache_issue64423.txt
@@ -0,0 +1,121 @@
+# Regression test for https://go.dev/issue/64423:
+#
+# When we parse the version for a Clang binary, we should accept
+# an arbitrary vendor prefix, which (as of 2023) may be injected
+# by defining CLANG_VENDOR when building clang itself.
+#
+# Since we don't want to actually rebuild the Clang toolchain in
+# this test, we instead simulate it by injecting a fake "clang"
+# binary that runs the real one as a subprocess.
+
+[!cgo] skip
+[short] skip 'builds and links a fake clang binary'
+[!cc:clang] skip 'test is specific to clang version parsing'
+
+# Save the location of the real clang command for our fake one to use.
+go run ./which clang
+cp stdout $WORK/.realclang
+
+# Build a fake clang and ensure that it is the one in $PATH.
+mkdir $WORK/bin
+go build -o $WORK/bin/clang$GOEXE ./fakeclang
+[!GOOS:plan9] env PATH=$WORK${/}bin
+[GOOS:plan9] env path=$WORK${/}bin
+
+# Force CGO_ENABLED=1 so that the following commands should error
+# out if the fake clang doesn't work.
+env CGO_ENABLED=1
+
+# The bug in https://go.dev/issue/64423 resulted in cache keys that
+# didn't contain any information about the C compiler.
+# Since the bug was in cache key computation, isolate the cache:
+# if we change the way caching works, we want the test to fail
+# instead of accidentally reusing the cached information from a
+# previous test run.
+env GOCACHE=$WORK${/}.cache
+mkdir $GOCACHE
+
+go build -x runtime/cgo
+
+ # Tell our fake clang to stop working.
+ # Previously, 'go build -x runtime/cgo' would continue to
+ # succeed because both the broken clang and the non-broken one
+ # resulted in a cache key with no clang version information.
+env GO_BREAK_CLANG=1
+! go build -x runtime/cgo
+stderr '# runtime/cgo\nGO_BREAK_CLANG is set'
+
+-- go.mod --
+module example/issue64423
+go 1.20
+-- which/main.go --
+package main
+
+import (
+ "os"
+ "os/exec"
+)
+
+func main() {
+ path, err := exec.LookPath(os.Args[1])
+ if err != nil {
+ panic(err)
+ }
+ os.Stdout.WriteString(path)
+}
+-- fakeclang/main.go --
+package main
+
+import (
+ "bufio"
+ "bytes"
+ "log"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "strings"
+)
+
+func main() {
+ if os.Getenv("GO_BREAK_CLANG") != "" {
+ os.Stderr.WriteString("GO_BREAK_CLANG is set\n")
+ os.Exit(1)
+ }
+
+ b, err := os.ReadFile(filepath.Join(os.Getenv("WORK"), ".realclang"))
+ if err != nil {
+ log.Fatal(err)
+ }
+ clang := string(bytes.TrimSpace(b))
+ cmd := exec.Command(clang, os.Args[1:]...)
+ cmd.Stdout = os.Stdout
+ stderr, err := cmd.StderrPipe()
+ if err != nil {
+ log.Fatal(err)
+ }
+
+ if err := cmd.Start(); err != nil {
+ log.Fatal(err)
+ }
+
+ r := bufio.NewReader(stderr)
+ for {
+ line, err := r.ReadString('\n')
+ if line != "" {
+ if strings.Contains(line, "clang version") {
+ // Simulate a clang version string with an arbitrary vendor prefix.
+ const vendorString = "Gopher Solutions Unlimited "
+ os.Stderr.WriteString(vendorString)
+ }
+ os.Stderr.WriteString(line)
+ }
+ if err != nil {
+ break
+ }
+ }
+ os.Stderr.Close()
+
+ if err := cmd.Wait(); err != nil {
+ os.Exit(1)
+ }
+}
