cmd/compile/internal/gc: fix Node.copy and introduce (raw|sep)copy

Node.copy used to make a shallow copy of a node. Often, this is not
correct: If a node n's Orig field pointed to itself, the copy's Orig
field has to be adjusted to point to the copy. Otherwise, if n is
modified later, the copy's Orig appears modified as well (because it
points to n).

This was fixed for one specific case with
https://go-review.googlesource.com/c/go/+/136395 (issue #26855).

This change instead addresses copy in general:

In two cases we don't want the Orig adjustment as it causes escape
analysis output to fail (not match the existing error messages).
rawcopy is used in those cases.

In several cases Orig is set to the copy immediately after making
a copy; a new function sepcopy is used there.

Updates #26855.
Fixes #27765.

Change-Id: Idaadeb5c4b9a027daabd46a2361348f7a93f2b00
Reviewed-on: https://go-review.googlesource.com/136540
Reviewed-by: Matthew Dempsky <[email protected]>

Author: griesemer

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

@@ -234,7 +234,7 @@ func convlit1(n *Node, t *types.Type, explicit bool, reuse canReuseNode) *Node {
 	if n.Op == OLITERAL && !reuse {
 		// Can't always set n.Type directly on OLITERAL nodes.
 		// See discussion on CL 20813.
-		n = n.copy()
+		n = n.rawcopy()
 		reuse = true
 	}
 
@@ -1200,8 +1200,7 @@ func setconst(n *Node, v Val) {
 	// Ensure n.Orig still points to a semantically-equivalent
 	// expression after we rewrite n into a constant.
 	if n.Orig == n {
-		n.Orig = n.copy()
-		n.Orig.Orig = n.Orig
+		n.Orig = n.sepcopy()
 	}
 
 	*n = Node{
@@ -1331,7 +1330,7 @@ func defaultlitreuse(n *Node, t *types.Type, reuse canReuseNode) *Node {
 	}
 
 	if n.Op == OLITERAL && !reuse {
-		n = n.copy()
+		n = n.rawcopy()
 		reuse = true
 	}
 

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

@@ -109,8 +109,7 @@ func (o *Order) cheapExpr(n *Node) *Node {
 		if l == n.Left {
 			return n
 		}
-		a := n.copy()
-		a.Orig = a
+		a := n.sepcopy()
 		a.Left = l
 		return typecheck(a, Erv)
 	}
@@ -135,8 +134,7 @@ func (o *Order) safeExpr(n *Node) *Node {
 		if l == n.Left {
 			return n
 		}
-		a := n.copy()
-		a.Orig = a
+		a := n.sepcopy()
 		a.Left = l
 		return typecheck(a, Erv)
 
@@ -145,8 +143,7 @@ func (o *Order) safeExpr(n *Node) *Node {
 		if l == n.Left {
 			return n
 		}
-		a := n.copy()
-		a.Orig = a
+		a := n.sepcopy()
 		a.Left = l
 		return typecheck(a, Erv)
 
@@ -161,8 +158,7 @@ func (o *Order) safeExpr(n *Node) *Node {
 		if l == n.Left && r == n.Right {
 			return n
 		}
-		a := n.copy()
-		a.Orig = a
+		a := n.sepcopy()
 		a.Left = l
 		a.Right = r
 		return typecheck(a, Erv)

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

@@ -349,15 +349,13 @@ func staticcopy(l *Node, r *Node, out *[]*Node) bool {
 				gdata(n, e.Expr, int(n.Type.Width))
 				continue
 			}
-			ll := n.copy()
-			ll.Orig = ll // completely separate copy
+			ll := n.sepcopy()
 			if staticassign(ll, e.Expr, out) {
 				continue
 			}
 			// Requires computation, but we're
 			// copying someone else's computation.
-			rr := orig.copy()
-			rr.Orig = rr // completely separate copy
+			rr := orig.sepcopy()
 			rr.Type = ll.Type
 			rr.Xoffset += e.Xoffset
 			setlineno(rr)
@@ -453,8 +451,7 @@ func staticassign(l *Node, r *Node, out *[]*Node) bool {
 				continue
 			}
 			setlineno(e.Expr)
-			a := n.copy()
-			a.Orig = a // completely separate copy
+			a := n.sepcopy()
 			if !staticassign(a, e.Expr, out) {
 				*out = append(*out, nod(OAS, a, e.Expr))
 			}
@@ -518,8 +515,7 @@ func staticassign(l *Node, r *Node, out *[]*Node) bool {
 			// Copy val directly into n.
 			n.Type = val.Type
 			setlineno(val)
-			a := n.copy()
-			a.Orig = a
+			a := n.sepcopy()
 			if !staticassign(a, val, out) {
 				*out = append(*out, nod(OAS, a, val))
 			}

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

@@ -364,9 +364,35 @@ func nodSym(op Op, left *Node, sym *types.Sym) *Node {
 	return n
 }
 
+// rawcopy returns a shallow copy of n.
+// Note: copy or sepcopy (rather than rawcopy) is usually the
+//       correct choice (see comment with Node.copy, below).
+func (n *Node) rawcopy() *Node {
+	copy := *n
+	return &copy
+}
+
+// sepcopy returns a separate shallow copy of n, with the copy's
+// Orig pointing to itself.
+func (n *Node) sepcopy() *Node {
+	copy := *n
+	copy.Orig = &copy
+	return &copy
+}
+
+// copy returns shallow copy of n and adjusts the copy's Orig if
+// necessary: In general, if n.Orig points to itself, the copy's
+// Orig should point to itself as well. Otherwise, if n is modified,
+// the copy's Orig node appears modified, too, and then doesn't
+// represent the original node anymore.
+// (This caused the wrong complit Op to be used when printing error
+// messages; see issues #26855, #27765).
 func (n *Node) copy() *Node {
-	n2 := *n
-	return &n2
+	copy := *n
+	if n.Orig == n {
+		copy.Orig = &copy
+	}
+	return &copy
 }
 
 // methcmp sorts methods by symbol.
@@ -412,8 +438,7 @@ func treecopy(n *Node, pos src.XPos) *Node {
 
 	switch n.Op {
 	default:
-		m := n.copy()
-		m.Orig = m
+		m := n.sepcopy()
 		m.Left = treecopy(n.Left, pos)
 		m.Right = treecopy(n.Right, pos)
 		m.List.Set(listtreecopy(n.List.Slice(), pos))

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

@@ -2923,14 +2923,6 @@ func typecheckcomplit(n *Node) *Node {
 
 	// Save original node (including n.Right)
 	norig := n.copy()
-	// If n.Orig points to itself, norig.Orig must point to itself, too.
-	// Otherwise, because n.Op is changed below, n.Orig's Op is changed
-	// as well because it (and the copy norig) still point to the original
-	// node n. This caused the wrong complit Op to be used when printing
-	// error messages (issue #26855).
-	if n.Orig == n {
-		norig.Orig = norig
-	}
 
 	setlineno(n.Right)
 	n.Right = typecheck(n.Right, Etype|Ecomplit)

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

@@ -4052,7 +4052,7 @@ func wrapCall(n *Node, init *Nodes) *Node {
 // The result of substArgTypes MUST be assigned back to old, e.g.
 // 	n.Left = substArgTypes(n.Left, t1, t2)
 func substArgTypes(old *Node, types_ ...*types.Type) *Node {
-	n := old.copy() // make shallow copy
+	n := old.copy()
 
 	for _, t := range types_ {
 		dowidth(t)