go/types: avoid race condition with dot-imported objects

It would be nice to have a test, but it requires running
this under the race detector which is a bit complicated
to set up; yet the fix is trivial. Verified manually that
it doesn't trip the race detector.

Fixes #32154.

Change-Id: I20bd746a07945c802f0476a1d8b1dfd83c87dae8
Reviewed-on: https://go-review.googlesource.com/c/go/+/183849
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

1 files changed, 10 insertions, 9 deletions

diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go
index 41741e5882..417e4e79aa 100644
--- a/src/go/types/resolver.go
+++ b/src/go/types/resolver.go
@@ -301,15 +301,15 @@ func (check *Checker) collectObjects() {
 								// A package scope may contain non-exported objects,
 								// do not import them!
 								if obj.Exported() {
-									// TODO(gri) When we import a package, we create
-									// a new local package object. We should do the
-									// same for each dot-imported object. That way
-									// they can have correct position information.
-									// (We must not modify their existing position
-									// information because the same package - found
-									// via Config.Packages - may be dot-imported in
-									// another package!)
-									check.declare(fileScope, nil, obj, token.NoPos)
+									// declare dot-imported object
+									// (Do not use check.declare because it modifies the object
+									// via Object.setScopePos, which leads to a race condition;
+									// the object may be imported into more than one file scope
+									// concurrently. See issue #32154.)
+									if alt := fileScope.Insert(obj); alt != nil {
+										check.errorf(s.Name.Pos(), "%s redeclared in this block", obj.Name())
+										check.reportAltDecl(alt)
+									}
 								}
 							}
 							// add position to set of dot-import positions for this file
@@ -317,6 +317,7 @@ func (check *Checker) collectObjects() {
 							check.addUnusedDotImport(fileScope, imp, s.Pos())
 						} else {
 							// declare imported package object in file scope
+							// (no need to provide s.Name since we called check.recordDef earlier)
 							check.declare(fileScope, nil, obj, token.NoPos)
 						}