go/loader: fix a data race
The loader was calling (*types.Checker).Files on the "unsafe" package,
a global variable. Even with zero files, this operation is not a no-op
because it sets the package's "complete" flag, leading to a data race.
(Because Unsafe.complete is already set at construction, the
race is benign, but is reported by -race nonetheless.)
Fixes golang/go#20718
Change-Id: I5a4f95be5ab4c60ea3b6c2a7fb6f1b67acbf42bc
Reviewed-on: https://go-review.googlesource.com/46071
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/go/loader/loader.go b/go/loader/loader.go
index f69a668..1cd4d6e 100644
--- a/go/loader/loader.go
+++ b/go/loader/loader.go
@@ -1009,15 +1009,19 @@
time.Since(imp.start), info.Pkg.Path(), len(files))
}
- if info.Pkg == types.Unsafe && len(files) > 0 {
- panic(`addFiles("unsafe") not permitted`)
+ // Don't call checker.Files on Unsafe, even with zero files,
+ // because it would mutate the package, which is a global.
+ if info.Pkg == types.Unsafe {
+ if len(files) > 0 {
+ panic(`"unsafe" package contains unexpected files`)
+ }
+ } else {
+ // Ignore the returned (first) error since we
+ // already collect them all in the PackageInfo.
+ info.checker.Files(files)
+ info.Files = append(info.Files, files...)
}
- // Ignore the returned (first) error since we
- // already collect them all in the PackageInfo.
- info.checker.Files(files)
- info.Files = append(info.Files, files...)
-
if imp.conf.AfterTypeCheck != nil {
imp.conf.AfterTypeCheck(info, files)
}
diff --git a/go/loader/loader_test.go b/go/loader/loader_test.go
index 807f798..02630eb 100644
--- a/go/loader/loader_test.go
+++ b/go/loader/loader_test.go
@@ -800,3 +800,17 @@
}
return strings.Join(pkgs, " ")
}
+
+// Load package "io" twice in parallel.
+// When run with -race, this is a regression test for Go issue 20718, in
+// which the global "unsafe" package was modified concurrently.
+func TestLoad1(t *testing.T) { loadIO(t) }
+func TestLoad2(t *testing.T) { loadIO(t) }
+
+func loadIO(t *testing.T) {
+ t.Parallel()
+ conf := &loader.Config{ImportPkgs: map[string]bool{"io": false}}
+ if _, err := conf.Load(); err != nil {
+ t.Fatal(err)
+ }
+}
