Skip to content

Commit 9e6388a

Browse files
committed
internal/gcimporter: work around go/types data race in 1.23.
Work around golang/go#69912 by punching a hole through the TypeName type to mark it black after importing. This is an unfortunate workaround, but the fix for the long-standing data race is probably not worth back-porting. For golang/go#69912 Change-Id: I583305f6e893e28b881dab932c9c4825430bc4ad Reviewed-on: https://go-review.googlesource.com/c/tools/+/621855 Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent c457787 commit 9e6388a

File tree

3 files changed

+134
-0
lines changed

3 files changed

+134
-0
lines changed

internal/gcimporter/gcimporter_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,3 +1043,71 @@ func testAliases(t *testing.T, f func(*testing.T)) {
10431043
})
10441044
}
10451045
}
1046+
1047+
type importMap map[string]*types.Package
1048+
1049+
func (m importMap) Import(path string) (*types.Package, error) { return m[path], nil }
1050+
1051+
func TestIssue69912(t *testing.T) {
1052+
fset := token.NewFileSet()
1053+
1054+
check := func(pkgname, src string, imports importMap) (*types.Package, error) {
1055+
f, err := goparser.ParseFile(fset, "a.go", src, 0)
1056+
if err != nil {
1057+
return nil, err
1058+
}
1059+
config := &types.Config{
1060+
Importer: imports,
1061+
}
1062+
return config.Check(pkgname, fset, []*ast.File{f}, nil)
1063+
}
1064+
1065+
const libSrc = `package lib
1066+
1067+
type T int
1068+
`
1069+
1070+
lib, err := check("lib", libSrc, nil)
1071+
if err != nil {
1072+
t.Fatalf("Checking lib: %v", err)
1073+
}
1074+
1075+
// Export it.
1076+
var out bytes.Buffer
1077+
if err := gcimporter.IExportData(&out, fset, lib); err != nil {
1078+
t.Fatalf("export: %v", err) // any failure to export is a bug
1079+
}
1080+
1081+
// Re-import it.
1082+
imports := make(map[string]*types.Package)
1083+
_, lib2, err := gcimporter.IImportData(fset, imports, out.Bytes(), "lib")
1084+
if err != nil {
1085+
t.Fatalf("import: %v", err) // any failure of export+import is a bug.
1086+
}
1087+
1088+
// Use the resulting package concurrently, via dot-imports.
1089+
1090+
const pSrc = `package p
1091+
1092+
import . "lib"
1093+
1094+
type S struct {
1095+
f T
1096+
}
1097+
`
1098+
importer := importMap{
1099+
"lib": lib2,
1100+
}
1101+
var wg sync.WaitGroup
1102+
for range 10 {
1103+
wg.Add(1)
1104+
go func() {
1105+
defer wg.Done()
1106+
_, err := check("p", pSrc, importer)
1107+
if err != nil {
1108+
t.Errorf("check failed: %v", err)
1109+
}
1110+
}()
1111+
}
1112+
wg.Wait()
1113+
}

internal/gcimporter/iimport.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,14 @@ type importReader struct {
558558
prevColumn int64
559559
}
560560

561+
// markBlack is redefined in iimport_go123.go, to work around golang/go#69912.
562+
//
563+
// If TypeNames are not marked black (in the sense of go/types cycle
564+
// detection), they may be mutated when dot-imported. Fix this by punching a
565+
// hole through the type, when compiling with Go 1.23. (The bug has been fixed
566+
// for 1.24, but the fix was not worth back-porting).
567+
var markBlack = func(name *types.TypeName) {}
568+
561569
func (r *importReader) obj(name string) {
562570
tag := r.byte()
563571
pos := r.pos()
@@ -570,6 +578,7 @@ func (r *importReader) obj(name string) {
570578
}
571579
typ := r.typ()
572580
obj := aliases.NewAlias(r.p.aliases, pos, r.currPkg, name, typ, tparams)
581+
markBlack(obj) // workaround for golang/go#69912
573582
r.declare(obj)
574583

575584
case constTag:
@@ -590,6 +599,9 @@ func (r *importReader) obj(name string) {
590599
// declaration before recursing.
591600
obj := types.NewTypeName(pos, r.currPkg, name, nil)
592601
named := types.NewNamed(obj, nil, nil)
602+
603+
markBlack(obj) // workaround for golang/go#69912
604+
593605
// Declare obj before calling r.tparamList, so the new type name is recognized
594606
// if used in the constraint of one of its own typeparams (see #48280).
595607
r.declare(obj)

internal/gcimporter/iimport_go123.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build go1.23 && !go1.24
6+
7+
package gcimporter
8+
9+
import (
10+
"go/token"
11+
"go/types"
12+
"unsafe"
13+
)
14+
15+
// TODO(rfindley): delete this workaround once gopls no longer compiles with
16+
// go1.23.
17+
18+
func init() {
19+
// Update markBlack so that it correctly sets the color
20+
// of imported TypeNames.
21+
//
22+
// See the doc comment for markBlack for details.
23+
24+
type color uint32
25+
const (
26+
white color = iota
27+
black
28+
grey
29+
)
30+
type object struct {
31+
_ *types.Scope
32+
_ token.Pos
33+
_ *types.Package
34+
_ string
35+
_ types.Type
36+
_ uint32
37+
color_ color
38+
_ token.Pos
39+
}
40+
type typeName struct {
41+
object
42+
}
43+
44+
// If the size of types.TypeName changes, this will fail to compile.
45+
const delta = int64(unsafe.Sizeof(typeName{})) - int64(unsafe.Sizeof(types.TypeName{}))
46+
var _ [-delta * delta]int
47+
48+
markBlack = func(obj *types.TypeName) {
49+
type uP = unsafe.Pointer
50+
var ptr *typeName
51+
*(*uP)(uP(&ptr)) = uP(obj)
52+
ptr.color_ = black
53+
}
54+
}

0 commit comments

Comments
 (0)