From 3803c858800b90fb8ae1669a3ac1f337be45f886 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Wed, 18 Oct 2023 16:49:50 -0400 Subject: [PATCH] cmd/go: work around race in fmtCmd CL 529219 made an existing race with accessing Builder.scriptDir from Builder.fmtcmd (and now also Builder.Showcmd) much more likely by dropping a theoretically unnecessary condition from the call from Builder.run to Builder.fmtcmd. For an example race report, see https://build.golang.org/log/c3cad62d0fc33a8381d2091661c685ea1fc525c4 The race is between (*Builder).cover2() -> (*Builder).run() -> (*Builder).fmtcmd() and various other call paths of the form (*Builder).build() -> (*gcToolchain).* (*Builder).Showcmd() -> (*Builder).fmtcmd() The race can be reproduced with go install -race cmd/go stress -p 1 go test -x -cover -a log Return this race to its existing likelihood by putting the condition back. This isn't a "correct" solution because the race could still happen if the "cover" tool invoked by Builder.cover2 emits output. But this will do for a temporary fix. Change-Id: Ifd811dea07f05e1422fd02b63cd958627727aa12 Reviewed-on: https://go-review.googlesource.com/c/go/+/536355 Auto-Submit: Austin Clements Reviewed-by: Bryan Mills LUCI-TryBot-Result: Go LUCI --- src/cmd/go/internal/work/exec.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index f115b5dc47..d66ffb7b86 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -2389,7 +2389,7 @@ var cgoTypeSigRe = lazyregexp.New(`\b_C2?(type|func|var|macro)_\B`) // and returns a non-nil error. func (b *Builder) run(a *Action, dir string, desc string, env []string, cmdargs ...any) error { out, err := b.runOut(a, dir, env, cmdargs...) - if desc == "" { + if len(out) > 0 && desc == "" { desc = b.fmtcmd(dir, "%s", strings.Join(str.StringList(cmdargs...), " ")) } return b.reportCmd(a, desc, dir, out, err) -- 2.48.1