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 <mdempsky@google.com>
This commit is contained in:
Robert Griesemer 2018-09-20 15:22:33 -07:00
parent 499fbb1a8a
commit ce58a39fca
6 changed files with 41 additions and 33 deletions

View file

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

View file

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

View file

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

View file

@ -364,9 +364,35 @@ func nodSym(op Op, left *Node, sym *types.Sym) *Node {
return n 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 { func (n *Node) copy() *Node {
n2 := *n copy := *n
return &n2 if n.Orig == n {
copy.Orig = &copy
}
return &copy
} }
// methcmp sorts methods by symbol. // methcmp sorts methods by symbol.
@ -412,8 +438,7 @@ func treecopy(n *Node, pos src.XPos) *Node {
switch n.Op { switch n.Op {
default: default:
m := n.copy() m := n.sepcopy()
m.Orig = m
m.Left = treecopy(n.Left, pos) m.Left = treecopy(n.Left, pos)
m.Right = treecopy(n.Right, pos) m.Right = treecopy(n.Right, pos)
m.List.Set(listtreecopy(n.List.Slice(), pos)) m.List.Set(listtreecopy(n.List.Slice(), pos))

View file

@ -2923,14 +2923,6 @@ func typecheckcomplit(n *Node) *Node {
// Save original node (including n.Right) // Save original node (including n.Right)
norig := n.copy() 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) setlineno(n.Right)
n.Right = typecheck(n.Right, Etype|Ecomplit) n.Right = typecheck(n.Right, Etype|Ecomplit)

View file

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