From ab523fc510aadb82dc39dec89741fcbb90093ff0 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 15 Jan 2021 00:39:24 -0800 Subject: [PATCH] [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 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Cuong Manh Le --- src/cmd/compile/internal/ssagen/ssa.go | 11 +++++++++-- test/fixedbugs/issue43701.go | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 test/fixedbugs/issue43701.go diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go index ab2e21bea0..fe9a1f617b 100644 --- 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 index 0000000000..6e16180046 --- /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 + } +}