From c5414457c62fc11f299946a46f6c868c4f0bf2ab Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Fri, 21 Dec 2018 16:36:45 -0800 Subject: [PATCH] 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 Reviewed-by: Ian Lance Taylor TryBot-Result: Gobot Gobot --- src/cmd/compile/internal/gc/pgen.go | 11 +++++++++++ test/codegen/zerosize.go | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 test/codegen/zerosize.go diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index bdc66f3e27..63e5860950 100644 --- 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 index 0000000000..cd0c83b6ef --- /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) { +}