cmd/go: possible incorrect lock usage in GetExitStatus and ExitIfErrors functions

[perillo]

What version of Go are you using (go version)?

$ go version
go version go1.16 linux/amd64

Does this issue reproduce with the latest release?

The code is incorrect in the current tip.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE="*.local"
GOMODCACHE="/home/manlio/.local/lib/go/pkg/mod"
GONOPROXY=""
GONOSUMDB="*.local"
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/manlio/src/contrib/go/go.googlesource.com/go/src/cmd/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build924785553=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.16 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.16
uname -sr: Linux 5.11.2-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) release release version 2.33.
gdb --version: GNU gdb (GDB) 10.1

What did you do?

In the internal/base/base.go file in the cmd/go package, there is a mutex that protects the access to the global exitStatus variable. The mutex is locked in the SetExitStatus function, but exitStatus is not protected in the ExitIfErrors and GetExitStatus functions.

Looking at the code, GetExitStatus is only used in the test but ExitIfErrors is used in several sub packages. It is possible that there are only concurrent write access and no read/write concurrent access.

GetExitStatus should lock the exitStatus variable and ExitIfErrors should be changed to call GetExitStatus internally.

[perillo]

After some git archeology, the result is that:

1. The original exitIfErrors was added in 6e2c3ef.
2. SetExitStatus was added in 64a73b0, to fix a data race when accessing the global exitStatus global variable.
3. GetExitStatus was added in 3def99a.

[gopherbot]

Change https://golang.org/cl/309349 mentions this issue: cmd/go/internal/base: lock exitMu in GetExitStatus