Skip to content

Commit 37a32a1

Browse files
committed
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 <[email protected]> Run-TryBot: Cherry Zhang <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent b78b427 commit 37a32a1

File tree

8 files changed

+74
-0
lines changed

8 files changed

+74
-0
lines changed

src/cmd/compile/internal/ssa/gen/386.rules

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,7 @@
531531
// fold ADDL into LEAL
532532
(ADDLconst [c] (LEAL [d] {s} x)) && is32Bit(int64(c)+int64(d)) => (LEAL [c+d] {s} x)
533533
(LEAL [c] {s} (ADDLconst [d] x)) && is32Bit(int64(c)+int64(d)) => (LEAL [c+d] {s} x)
534+
(ADDLconst [c] x:(SP)) => (LEAL [c] x) // so it is rematerializeable
534535
(LEAL [c] {s} (ADDL x y)) && x.Op != OpSB && y.Op != OpSB => (LEAL1 [c] {s} x y)
535536
(ADDL x (LEAL [c] {s} y)) && x.Op != OpSB && y.Op != OpSB => (LEAL1 [c] {s} x y)
536537

src/cmd/compile/internal/ssa/gen/PPC64.rules

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,7 @@
845845
(SUB x (MOVDconst [c])) && is32Bit(-c) => (ADDconst [-c] x)
846846

847847
(ADDconst [c] (MOVDaddr [d] {sym} x)) && is32Bit(c+int64(d)) => (MOVDaddr [int32(c+int64(d))] {sym} x)
848+
(ADDconst [c] x:(SP)) && is32Bit(c) => (MOVDaddr [int32(c)] x) // so it is rematerializeable
848849

849850
(MULL(W|D) x (MOVDconst [c])) && is16Bit(c) => (MULL(W|D)const [int32(c)] x)
850851

src/cmd/compile/internal/ssa/gen/Wasm.rules

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,7 @@
399399
// folding offset into address
400400
(I64AddConst [off] (LoweredAddr {sym} [off2] base)) && isU32Bit(off+int64(off2)) =>
401401
(LoweredAddr {sym} [int32(off)+off2] base)
402+
(I64AddConst [off] x:(SP)) && isU32Bit(off) => (LoweredAddr [int32(off)] x) // so it is rematerializeable
402403

403404
// transforming readonly globals into constants
404405
(I64Load [off] (LoweredAddr {sym} [off2] (SB)) _) && symIsRO(sym) && isU32Bit(off+int64(off2)) => (I64Const [int64(read64(sym, off+int64(off2), config.ctxt.Arch.ByteOrder))])

src/cmd/compile/internal/ssa/rewrite386.go

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/cmd/compile/internal/ssa/rewritePPC64.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/cmd/compile/internal/ssa/rewriteWasm.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/cmd/compile/internal/wasm/ssa.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,12 @@ func ssaGenValueOnStack(s *gc.SSAGenState, v *ssa.Value, extend bool) {
230230
}
231231

232232
case ssa.OpWasmLoweredAddr:
233+
if v.Aux == nil { // address of off(SP), no symbol
234+
getValue64(s, v.Args[0])
235+
i64Const(s, v.AuxInt)
236+
s.Prog(wasm.AI64Add)
237+
break
238+
}
233239
p := s.Prog(wasm.AGet)
234240
p.From.Type = obj.TYPE_ADDR
235241
switch v.Aux.(type) {

test/fixedbugs/issue42944.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// errorcheck -0 -live
2+
3+
// Copyright 2020 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
// Issue 42944: address of callee args area should only be short-lived
8+
// and never across a call.
9+
10+
package p
11+
12+
type T [10]int // trigger DUFFCOPY when passing by value, so it uses the address
13+
14+
func F() {
15+
var x T
16+
var i int
17+
for {
18+
x = G(i) // no autotmp live at this and next calls
19+
H(i, x)
20+
}
21+
}
22+
23+
func G(int) T
24+
func H(int, T)

0 commit comments

Comments
 (0)