cmd/compile: pad zero-sized stack variables

If someone takes a pointer to a zero-sized stack variable, it can
be incorrectly interpreted as a pointer to the next object in the
stack frame. To avoid this, add some padding after zero-sized variables.

We only need to pad if the next variable in memory (which is the
previous variable in the order in which we allocate variables to the
stack frame) has pointers. If the next variable has no pointers, it
won't hurt to have a pointer to it.

Because we allocate all pointer-containing variables before all
non-pointer-containing variables, we should only have to pad once per
frame.

Fixes #24993

Change-Id: Ife561cdfdf964fdbf69af03ae6ba97d004e6193c
Reviewed-on: https://go-review.googlesource.com/c/155698
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go
index bdc66f3e275a01f1d9bb5b2100a21d2b1a27f0fd..63e586095052fca39ee6c4aa0d687dda93076344 100644 (file)
--- a/src/cmd/compile/internal/gc/pgen.go
+++ b/src/cmd/compile/internal/gc/pgen.go
@@ -153,6 +153,7 @@ func (s *ssafn) AllocFrame(f *ssa.Func) {
        sort.Sort(byStackVar(fn.Dcl))
 
        // Reassign stack offsets of the locals that are used.
+       lastHasPtr := false
        for i, n := range fn.Dcl {
                if n.Op != ONAME || n.Class() != PAUTO {
                        continue
@@ -167,10 +168,20 @@ func (s *ssafn) AllocFrame(f *ssa.Func) {
                if w >= thearch.MAXWIDTH || w < 0 {
                        Fatalf("bad width")
                }
+               if w == 0 && lastHasPtr {
+                       // Pad between a pointer-containing object and a zero-sized object.
+                       // This prevents a pointer to the zero-sized object from being interpreted
+                       // as a pointer to the pointer-containing object (and causing it
+                       // to be scanned when it shouldn't be). See issue 24993.
+                       w = 1
+               }
                s.stksize += w
                s.stksize = Rnd(s.stksize, int64(n.Type.Align))
                if types.Haspointers(n.Type) {
                        s.stkptrsize = s.stksize
+                       lastHasPtr = true
+               } else {
+                       lastHasPtr = false
                }
                if thearch.LinkArch.InFamily(sys.MIPS, sys.MIPS64, sys.ARM, sys.ARM64, sys.PPC64, sys.S390X) {
                        s.stksize = Rnd(s.stksize, int64(Widthptr))

diff --git a/test/codegen/zerosize.go b/test/codegen/zerosize.go
new file mode 100644 (file)
index 0000000..cd0c83b
--- /dev/null
+++ b/test/codegen/zerosize.go
@@ -0,0 +1,25 @@
+// asmcheck
+
+// Copyright 2018 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.
+
+// Make sure a pointer variable and a zero-sized variable
+// aren't allocated to the same stack slot.
+// See issue 24993.
+
+package codegen
+
+func zeroSize() {
+       c := make(chan struct{})
+       // amd64:`MOVQ\t\$0, ""\.s\+32\(SP\)`
+       var s *int
+       g(&s) // force s to be a stack object
+
+       // amd64:`LEAQ\t""\..*\+31\(SP\)`
+       c <- struct{}{}
+}
+
+//go:noinline
+func g(p **int) {
+}