cmd/compile: don't mutate shared nodes in orderinit

A few gc.Node ops may be shared across functions.
The compiler is (mostly) already careful to avoid mutating them.
However, from a concurrency perspective, replacing (say)
an empty list with an empty list still counts as a mutation.
One place this occurs is orderinit. Avoid it.

This requires fixing one spot where shared nodes were mutated.
It doesn't result in any functional or performance changes.

Passes toolstash-check.

Updates #15756

Change-Id: I63c93b31baeeac62d7574804acb6b7f2bc9d14a9
Reviewed-on: https://go-review.googlesource.com/39196
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

src/cmd/compile/internal/gc/order.go

@@ -314,6 +314,14 @@ func orderstmtinplace(n *Node) *Node {
 
 // Orderinit moves n's init list to order->out.
 func orderinit(n *Node, order *Order) {
+	if n.mayBeShared() {
+		// For concurrency safety, don't mutate potentially shared nodes.
+		// First, ensure that no work is required here.
+		if n.Ninit.Len() > 0 {
+			Fatalf("orderinit shared node with ninit")
+		}
+		return
+	}
 	orderstmtlist(n.Ninit, order)
 	n.Ninit.Set(nil)
 }
@@ -1107,7 +1115,7 @@ func orderexpr(n *Node, order *Order, lhs *Node) *Node {
 		var s []*Node
 
 		cleantempnopop(mark, order, &s)
-		n.Right.Ninit.Prepend(s...)
+		n.Right = addinit(n.Right, s)
 		n.Right = orderexprinplace(n.Right, order)
 
 	case OCALLFUNC,

src/cmd/compile/internal/gc/subr.go

@@ -2043,11 +2043,8 @@ func addinit(n *Node, init []*Node) *Node {
 	if len(init) == 0 {
 		return n
 	}
-
-	switch n.Op {
-	// There may be multiple refs to this node;
-	// introduce OCONVNOP to hold init list.
-	case ONAME, OLITERAL:
+	if n.mayBeShared() {
+		// Introduce OCONVNOP to hold init list.
 		n = nod(OCONVNOP, n, nil)
 		n.Type = n.Left.Type
 		n.Typecheck = 1

src/cmd/compile/internal/gc/syntax.go

@@ -14,7 +14,7 @@ import (
 // A Node is a single node in the syntax tree.
 // Actually the syntax tree is a syntax DAG, because there is only one
 // node with Op=ONAME for a given instance of a variable x.
-// The same is true for Op=OTYPE and Op=OLITERAL.
+// The same is true for Op=OTYPE and Op=OLITERAL. See Node.mayBeShared.
 type Node struct {
 	// Tree structure.
 	// Generic recursive walks should follow these fields.
@@ -179,6 +179,16 @@ func (n *Node) SetIota(x int64) {
 	n.Xoffset = x
 }
 
+// mayBeShared reports whether n may occur in multiple places in the AST.
+// Extra care must be taken when mutating such a node.
+func (n *Node) mayBeShared() bool {
+	switch n.Op {
+	case ONAME, OLITERAL, OTYPE:
+		return true
+	}
+	return false
+}
+
 // Name holds Node fields used only by named nodes (ONAME, OTYPE, OPACK, OLABEL, some OLITERAL).
 type Name struct {
 	Pack      *Node  // real package for import . names

src/cmd/compile/internal/gc/walk.go

@@ -500,6 +500,7 @@ opswitch:
 
 	case OTYPE, ONAME, OLITERAL:
 		// TODO(mdempsky): Just return n; see discussion on CL 38655.
+		// Perhaps refactor to use Node.mayBeShared for these instead.
 
 	case ONOT, OMINUS, OPLUS, OCOM, OREAL, OIMAG, ODOTMETH, ODOTINTER,
 		OIND, OSPTR, OITAB, OIDATA, OADDR: