cmd/compile/internal/ir: set Addrtaken on Canonical ONAME too

In CL 522879, I moved the logic for setting Addrtaken from typecheck's
markAddrOf and ComputeAddrtaken directly into ir.NewAddrExpr. However,
I took the logic from markAddrOf, and failed to notice that
ComputeAddrtaken also set Addrtaken on the canonical ONAME.

The result is that if the only address-of expressions were within a
function literal, the canonical variable never got marked Addrtaken.
In turn, this could cause the consistency check in ir.Reassigned to
fail. (Yay for consistency checks turning mistakes into ICEs, rather
than miscompilation.)

Fixes #62313.

Change-Id: Ieab2854cd7fcc1b6c5d1e61de66453add9890a4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/523375
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go
index 9ade6f6a6e5a01c70c0853c88ca63ad781e0e419..37a30edca23f32e54fda5163bf01b562227b585c 100644 (file)
--- a/src/cmd/compile/internal/ir/expr.go
+++ b/src/cmd/compile/internal/ir/expr.go
@@ -92,6 +92,19 @@ func NewAddrExpr(pos src.XPos, x Node) *AddrExpr {
                n.op = OADDR
                if r, ok := OuterValue(x).(*Name); ok && r.Op() == ONAME {
                        r.SetAddrtaken(true)
+
+                       // If r is a closure variable, we need to mark its canonical
+                       // variable as addrtaken too, so that closure conversion
+                       // captures it by reference.
+                       //
+                       // Exception: if we've already marked the variable as
+                       // capture-by-value, then that means this variable isn't
+                       // logically modified, and we must be taking its address to pass
+                       // to a runtime function that won't mutate it. In that case, we
+                       // only need to make sure our own copy is addressable.
+                       if r.IsClosureVar() && !r.Byval() {
+                               r.Canonical().SetAddrtaken(true)
+                       }
                }
        }
 

diff --git a/test/fixedbugs/issue62313.go b/test/fixedbugs/issue62313.go
new file mode 100644 (file)
index 0000000..139f1eb
--- /dev/null
+++ b/test/fixedbugs/issue62313.go
@@ -0,0 +1,13 @@
+// compile
+
+// Copyright 2023 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.
+
+package p
+
+func f() {
+       var err error = nil
+       defer func() { _ = &err }()
+       err.Error()
+}