Skip to content

Commit

Permalink
[release-branch.go1.19] cmd/compile: copy blank parameter node when s…
Browse files Browse the repository at this point in the history
…ubstituting function type

When a function type is copied (e.g. for substituting type
parameters), we make copies of its parameter ir.Name nodes, so
they are not shared with the old function type. But currently a
blank (_) identifier is not copied but shared. The parameter
node's frame offset is assigned (in ABI analysis) and then used in
the concurrent backend. Shared node can cause a data race. Make a
new blank parameter node to avoid sharing. (Unified IR does already
not have this problem. This fixes non-unified-IR mode.)

Updates #55357.
Fixes #56360.

Change-Id: Ie27f08e5589ac7d5d3f0d0d5de1a21e4fd2765c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/443158
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
(cherry picked from commit 4725c71)
Reviewed-on: https://go-review.googlesource.com/c/go/+/445176
  • Loading branch information
cherrymui authored and mknyszek committed Nov 8, 2022
1 parent 23fd10b commit 39ac1fb
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 1 deletion.
65 changes: 65 additions & 0 deletions src/cmd/compile/internal/test/race.go
@@ -0,0 +1,65 @@
// Copyright 2022 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//go:build !compiler_bootstrap
// +build !compiler_bootstrap

package test

// The racecompile builder only builds packages, but does not build
// or run tests. This is a non-test file to hold cases that (used
// to) trigger compiler data races, so they will be exercised on
// the racecompile builder.
//
// This package is not imported so functions here are not included
// in the actual compiler.

// Issue 55357: data race when building multiple instantiations of
// generic closures with _ parameters.
func Issue55357() {
type U struct {
A int
B string
C string
}
var q T55357[U]
q.Count()
q.List()

type M struct {
A int64
B uint32
C uint32
}
var q2 T55357[M]
q2.Count()
q2.List()
}

type T55357[T any] struct{}

//go:noinline
func (q *T55357[T]) do(w, v bool, fn func(bk []byte, v T) error) error {
return nil
}

func (q *T55357[T]) Count() (n int, rerr error) {
err := q.do(false, false, func(kb []byte, _ T) error {
n++
return nil
})
return n, err
}

func (q *T55357[T]) List() (list []T, rerr error) {
var l []T
err := q.do(false, true, func(_ []byte, v T) error {
l = append(l, v)
return nil
})
if err != nil {
return nil, err
}
return l, nil
}
10 changes: 9 additions & 1 deletion src/cmd/compile/internal/typecheck/subr.go
Expand Up @@ -1350,14 +1350,22 @@ func (ts *Tsubster) tstruct(t *types.Type, force bool) *types.Type {
newfields[i].SetNointerface(true)
}
if f.Nname != nil && ts.Vars != nil {
v := ts.Vars[f.Nname.(*ir.Name)]
n := f.Nname.(*ir.Name)
v := ts.Vars[n]
if v != nil {
// This is the case where we are
// translating the type of the function we
// are substituting, so its dcls are in
// the subst.ts.vars table, and we want to
// change to reference the new dcl.
newfields[i].Nname = v
} else if ir.IsBlank(n) {
// Blank variable is not dcl list. Make a
// new one to not share.
m := ir.NewNameAt(n.Pos(), ir.BlankNode.Sym())
m.SetType(n.Type())
m.SetTypecheck(1)
newfields[i].Nname = m
} else {
// This is the case where we are
// translating the type of a function
Expand Down

0 comments on commit 39ac1fb

Please sign in to comment.