From 39ac1fbd1393deed8888245dcf653220b14376f6 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Fri, 14 Oct 2022 19:52:00 -0400 Subject: [PATCH] [release-branch.go1.19] cmd/compile: copy blank parameter node when substituting 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 Run-TryBot: Cherry Mui TryBot-Result: Gopher Robot Reviewed-by: Keith Randall (cherry picked from commit 4725c71b735143a138b24f2b0e055c717d8d69ca) Reviewed-on: https://go-review.googlesource.com/c/go/+/445176 --- src/cmd/compile/internal/test/race.go | 65 ++++++++++++++++++++++ src/cmd/compile/internal/typecheck/subr.go | 10 +++- 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 src/cmd/compile/internal/test/race.go diff --git a/src/cmd/compile/internal/test/race.go b/src/cmd/compile/internal/test/race.go new file mode 100644 index 0000000000000..4cc4d536095c9 --- /dev/null +++ b/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 +} diff --git a/src/cmd/compile/internal/typecheck/subr.go b/src/cmd/compile/internal/typecheck/subr.go index ffd00ec3a796a..b12b6a4a23f03 100644 --- a/src/cmd/compile/internal/typecheck/subr.go +++ b/src/cmd/compile/internal/typecheck/subr.go @@ -1350,7 +1350,8 @@ 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 @@ -1358,6 +1359,13 @@ func (ts *Tsubster) tstruct(t *types.Type, force bool) *types.Type { // 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