From: Bryan C. Mills Date: Wed, 6 Dec 2023 20:19:59 +0000 (-0500) Subject: cmd/go: accept clang versions with vendor prefixes X-Git-Tag: go1.22rc1~66 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=c71eedf90aff3fc73a645b88d2e5166b8a0179fd;p=gostls13.git 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Matloob --- diff --git a/src/cmd/go/internal/work/buildid.go b/src/cmd/go/internal/work/buildid.go index 276f524afa..0769443712 100644 --- a/src/cmd/go/internal/work/buildid.go +++ b/src/cmd/go/internal/work/buildid.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "os/exec" + "regexp" "strings" "cmd/go/internal/base" @@ -236,10 +237,22 @@ func (b *Builder) gccToolID(name, language string) (id, exe string, err error) { } 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 0000000000..f1bc2c3108 --- /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) + } +}