[dev.regabi] cmd/compile: don't promote Byval CaptureVars if Addrtaken

We decide during escape analysis whether to pass closure variables by
value or reference. One of the factors that's considered is whether a
variable has had its address taken.

However, this analysis is based only on the user-written source code,
whereas order+walk may introduce rewrites that take the address of a
variable (e.g., passing a uint16 key by reference to the size-generic
map runtime builtins).

Typically this would be harmless, albeit suboptimal. But in #43701 it
manifested as needing a stack object for a function where we didn't
realize we needed one up front when we generate symbols.

Probably we should just generate symbols on demand, now that those
routines are all concurrent-safe, but this is a first fix.

Thanks to Alberto Donizetti for reporting the issue, and Cuong Manh Le
for initial investigation.

Fixes #43701.

Change-Id: I16d87e9150723dcb16de7b43f2a8f3cd807a9437
Reviewed-on: https://go-review.googlesource.com/c/go/+/284075
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>

diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
index ab2e21bea064c696271664ce56b3e56064338eeb..fe9a1f617bb00408533e6d41bf0e441fe43af6e5 100644 (file)
--- a/src/cmd/compile/internal/ssagen/ssa.go
+++ b/src/cmd/compile/internal/ssagen/ssa.go
@@ -490,8 +490,15 @@ func buildssa(fn *ir.Func, worker int) *ssa.Func {
                        ptr := s.newValue1I(ssa.OpOffPtr, types.NewPtr(typ), offset, clo)
                        offset += typ.Size()
 
-                       if n.Byval() && TypeOK(n.Type()) {
-                               // If it is a small variable captured by value, downgrade it to PAUTO.
+                       // If n is a small variable captured by value, promote
+                       // it to PAUTO so it can be converted to SSA.
+                       //
+                       // Note: While we never capture a variable by value if
+                       // the user took its address, we may have generated
+                       // runtime calls that did (#43701). Since we don't
+                       // convert Addrtaken variables to SSA anyway, no point
+                       // in promoting them either.
+                       if n.Byval() && !n.Addrtaken() && TypeOK(n.Type()) {
                                n.Class = ir.PAUTO
                                fn.Dcl = append(fn.Dcl, n)
                                s.assign(n, s.load(n.Type(), ptr), false, 0)

diff --git a/test/fixedbugs/issue43701.go b/test/fixedbugs/issue43701.go
new file mode 100644 (file)
index 0000000..6e16180
--- /dev/null
+++ b/test/fixedbugs/issue43701.go
@@ -0,0 +1,18 @@
+// compile
+
+// Copyright 2021 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 st struct {
+               s string
+               i int16
+       }
+       _ = func() {
+               var m map[int16]int
+               m[st.i] = 0
+       }
+}