cmd/compile: make sure address of offset(SP) is rematerializeable

An address of offset(SP) may point to the callee args area, and
may be used to move things into/out of the args/results. If an
address like that is spilled and picked up by the GC, it may hold
an arg/result live in the callee, which may not actually be live
(e.g. a result not initialized at function entry). Make sure
they are rematerializeable, so they are always short-lived and
never picked up by the GC.

This CL changes 386, PPC64, and Wasm. On AMD64 we already have
the rule (line 2159). On other architectures, we already have
similar rules like
(OffPtr [off] ptr:(SP)) => (MOVDaddr [int32(off)] ptr)
to avoid this problem. (Probably me in the past had run into
this...)

Fixes #42944.

Change-Id: Id2ec73ac08f8df1829a9a7ceb8f749d67fe86d1e
Reviewed-on: https://go-review.googlesource.com/c/go/+/275174
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/cmd/compile/internal/ssa/gen/386.rules b/src/cmd/compile/internal/ssa/gen/386.rules
index 537705c6819dad5716eb0151287c45f2a2dcbbbb..fbc12fd67219a87050a0c61a9238408044f912b1 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/386.rules
+++ b/src/cmd/compile/internal/ssa/gen/386.rules
@@ -531,6 +531,7 @@
 // fold ADDL into LEAL
 (ADDLconst [c] (LEAL [d] {s} x)) && is32Bit(int64(c)+int64(d)) => (LEAL [c+d] {s} x)
 (LEAL [c] {s} (ADDLconst [d] x)) && is32Bit(int64(c)+int64(d)) => (LEAL [c+d] {s} x)
+(ADDLconst [c] x:(SP)) => (LEAL [c] x) // so it is rematerializeable
 (LEAL [c] {s} (ADDL x y)) && x.Op != OpSB && y.Op != OpSB => (LEAL1 [c] {s} x y)
 (ADDL x (LEAL [c] {s} y)) && x.Op != OpSB && y.Op != OpSB => (LEAL1 [c] {s} x y)
 

diff --git a/src/cmd/compile/internal/ssa/gen/PPC64.rules b/src/cmd/compile/internal/ssa/gen/PPC64.rules
index 31b186d167901d7eb522a6fed0dae4a3db155dbb..c06404617297a5877b81d186811c43e37ff775e2 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/PPC64.rules
+++ b/src/cmd/compile/internal/ssa/gen/PPC64.rules
@@ -845,6 +845,7 @@
 (SUB x (MOVDconst [c])) && is32Bit(-c) => (ADDconst [-c] x)
 
 (ADDconst [c] (MOVDaddr [d] {sym} x)) && is32Bit(c+int64(d)) => (MOVDaddr [int32(c+int64(d))] {sym} x)
+(ADDconst [c] x:(SP)) && is32Bit(c) => (MOVDaddr [int32(c)] x) // so it is rematerializeable
 
 (MULL(W|D) x (MOVDconst [c])) && is16Bit(c) => (MULL(W|D)const [int32(c)] x)
 

diff --git a/src/cmd/compile/internal/ssa/gen/Wasm.rules b/src/cmd/compile/internal/ssa/gen/Wasm.rules
index ea12c5d617a96ed596218247abd4d75c320f45db..fc45cd3ed5f1d9159a38bfb9513ae53bb7e18d8c 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/Wasm.rules
+++ b/src/cmd/compile/internal/ssa/gen/Wasm.rules
@@ -399,6 +399,7 @@
 // folding offset into address
 (I64AddConst [off] (LoweredAddr {sym} [off2] base)) && isU32Bit(off+int64(off2)) =>
        (LoweredAddr {sym} [int32(off)+off2] base)
+(I64AddConst [off] x:(SP)) && isU32Bit(off) => (LoweredAddr [int32(off)] x) // so it is rematerializeable
 
 // transforming readonly globals into constants
 (I64Load [off] (LoweredAddr {sym} [off2] (SB)) _) && symIsRO(sym) && isU32Bit(off+int64(off2)) => (I64Const [int64(read64(sym, off+int64(off2), config.ctxt.Arch.ByteOrder))])

diff --git a/src/cmd/compile/internal/ssa/rewrite386.go b/src/cmd/compile/internal/ssa/rewrite386.go
index eca4817b9bd2f2fe211794e27d6ffbc40c498b57..2acdccd5684aaf5364eb4a58ae44d9688769dc8d 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewrite386.go
+++ b/src/cmd/compile/internal/ssa/rewrite386.go
@@ -1027,6 +1027,19 @@ func rewriteValue386_Op386ADDLconst(v *Value) bool {
                v.AddArg(x)
                return true
        }
+       // match: (ADDLconst [c] x:(SP))
+       // result: (LEAL [c] x)
+       for {
+               c := auxIntToInt32(v.AuxInt)
+               x := v_0
+               if x.Op != OpSP {
+                       break
+               }
+               v.reset(Op386LEAL)
+               v.AuxInt = int32ToAuxInt(c)
+               v.AddArg(x)
+               return true
+       }
        // match: (ADDLconst [c] (LEAL1 [d] {s} x y))
        // cond: is32Bit(int64(c)+int64(d))
        // result: (LEAL1 [c+d] {s} x y)

diff --git a/src/cmd/compile/internal/ssa/rewritePPC64.go b/src/cmd/compile/internal/ssa/rewritePPC64.go
index 7d4cf73fd8e59f31992982a76ba2fb833bb66dc7..455f9b138859b0e0e31231b644e6584ed9b2841c 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewritePPC64.go
+++ b/src/cmd/compile/internal/ssa/rewritePPC64.go
@@ -4195,6 +4195,20 @@ func rewriteValuePPC64_OpPPC64ADDconst(v *Value) bool {
                v.AddArg(x)
                return true
        }
+       // match: (ADDconst [c] x:(SP))
+       // cond: is32Bit(c)
+       // result: (MOVDaddr [int32(c)] x)
+       for {
+               c := auxIntToInt64(v.AuxInt)
+               x := v_0
+               if x.Op != OpSP || !(is32Bit(c)) {
+                       break
+               }
+               v.reset(OpPPC64MOVDaddr)
+               v.AuxInt = int32ToAuxInt(int32(c))
+               v.AddArg(x)
+               return true
+       }
        // match: (ADDconst [c] (SUBFCconst [d] x))
        // cond: is32Bit(c+d)
        // result: (SUBFCconst [c+d] x)

diff --git a/src/cmd/compile/internal/ssa/rewriteWasm.go b/src/cmd/compile/internal/ssa/rewriteWasm.go
index 52b6f6bfc79030a85db39edf9e3d13954b5fee10..c8ecefc7367cb8883c8f434a8d3330839efa6ce9 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewriteWasm.go
+++ b/src/cmd/compile/internal/ssa/rewriteWasm.go
@@ -3693,6 +3693,20 @@ func rewriteValueWasm_OpWasmI64AddConst(v *Value) bool {
                v.AddArg(base)
                return true
        }
+       // match: (I64AddConst [off] x:(SP))
+       // cond: isU32Bit(off)
+       // result: (LoweredAddr [int32(off)] x)
+       for {
+               off := auxIntToInt64(v.AuxInt)
+               x := v_0
+               if x.Op != OpSP || !(isU32Bit(off)) {
+                       break
+               }
+               v.reset(OpWasmLoweredAddr)
+               v.AuxInt = int32ToAuxInt(int32(off))
+               v.AddArg(x)
+               return true
+       }
        return false
 }
 func rewriteValueWasm_OpWasmI64And(v *Value) bool {

diff --git a/src/cmd/compile/internal/wasm/ssa.go b/src/cmd/compile/internal/wasm/ssa.go
index a36fbca4e02bdc677e128c2c9497416e7e10005d..9c9f6edc5f4912162af268f8f11318fbbdacbb01 100644 (file)
--- a/src/cmd/compile/internal/wasm/ssa.go
+++ b/src/cmd/compile/internal/wasm/ssa.go
@@ -230,6 +230,12 @@ func ssaGenValueOnStack(s *gc.SSAGenState, v *ssa.Value, extend bool) {
                }
 
        case ssa.OpWasmLoweredAddr:
+               if v.Aux == nil { // address of off(SP), no symbol
+                       getValue64(s, v.Args[0])
+                       i64Const(s, v.AuxInt)
+                       s.Prog(wasm.AI64Add)
+                       break
+               }
                p := s.Prog(wasm.AGet)
                p.From.Type = obj.TYPE_ADDR
                switch v.Aux.(type) {

diff --git a/test/fixedbugs/issue42944.go b/test/fixedbugs/issue42944.go
new file mode 100644 (file)
index 0000000..bb947bc
--- /dev/null
+++ b/test/fixedbugs/issue42944.go
@@ -0,0 +1,24 @@
+// errorcheck -0 -live
+
+// Copyright 2020 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.
+
+// Issue 42944: address of callee args area should only be short-lived
+// and never across a call.
+
+package p
+
+type T [10]int // trigger DUFFCOPY when passing by value, so it uses the address
+
+func F() {
+       var x T
+       var i int
+       for {
+               x = G(i) // no autotmp live at this and next calls
+               H(i, x)
+       }
+}
+
+func G(int) T
+func H(int, T)