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>

diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go
index 41741e5882de5f4bbaf688730e46c4b12b098b1d..417e4e79aa00bf3f9572e3a32411a5a43e203ac8 100644 (file)
--- 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)
                                                }