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 <[email protected]>

go/loader/loader.go

@@ -1009,15 +1009,19 @@ func (imp *importer) addFiles(info *PackageInfo, files []*ast.File, cycleCheck b
 			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)
 	}

go/loader/loader_test.go

@@ -800,3 +800,17 @@ func created(prog *loader.Program) string {
 	}
 	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)
+	}
+}