From: Keith Randall <khr@google.com>
Date: Tue, 29 May 2018 23:44:36 +0000 (-0700)
Subject: cmd/compile: include callee args section when checking frame too large
X-Git-Tag: go1.11beta1~230
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=06b326054dc0cad03b7b848d8516a578dd8b559f;p=gostls13.git

cmd/compile: include callee args section when checking frame too large

The stack frame includes the callee args section. At the point where
we were checking the frame size, that part of the frame had not been
computed yet. Move the check later so we can include the callee args size.

Fixes #20780
Update #25507

Change-Id: Iab97cb89b3a24f8ca19b9123ef2a111d6850c3fe
Reviewed-on: https://go-review.googlesource.com/115195
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
---

diff --git a/src/cmd/compile/internal/gc/main.go b/src/cmd/compile/internal/gc/main.go
index 74590ccc39..e9d9e5566e 100644
--- a/src/cmd/compile/internal/gc/main.go
+++ b/src/cmd/compile/internal/gc/main.go
@@ -659,14 +659,6 @@ func Main(archInit func(*Arch)) {
 			Ctxt.DwFixups = nil
 			genDwarfInline = 0
 		}
-
-		// Check whether any of the functions we have compiled have gigantic stack frames.
-		obj.SortSlice(largeStackFrames, func(i, j int) bool {
-			return largeStackFrames[i].Before(largeStackFrames[j])
-		})
-		for _, largePos := range largeStackFrames {
-			yyerrorl(largePos, "stack frame too large (>1GB)")
-		}
 	}
 
 	// Phase 9: Check external declarations.
@@ -688,6 +680,14 @@ func Main(archInit func(*Arch)) {
 		dumpasmhdr()
 	}
 
+	// Check whether any of the functions we have compiled have gigantic stack frames.
+	obj.SortSlice(largeStackFrames, func(i, j int) bool {
+		return largeStackFrames[i].Before(largeStackFrames[j])
+	})
+	for _, largePos := range largeStackFrames {
+		yyerrorl(largePos, "stack frame too large (>1GB)")
+	}
+
 	if len(compilequeue) != 0 {
 		Fatalf("%d uncompiled functions", len(compilequeue))
 	}
diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go
index 8f3947b0a6..cf1164772b 100644
--- a/src/cmd/compile/internal/gc/pgen.go
+++ b/src/cmd/compile/internal/gc/pgen.go
@@ -265,11 +265,24 @@ func compileSSA(fn *Node, worker int) {
 		return
 	}
 	pp := newProgs(fn, worker)
+	defer pp.Free()
 	genssa(f, pp)
+	// Check frame size again.
+	// The check above included only the space needed for local variables.
+	// After genssa, the space needed includes local variables and the callee arg region.
+	// We must do this check prior to calling pp.Flush.
+	// If there are any oversized stack frames,
+	// the assembler may emit inscrutable complaints about invalid instructions.
+	if pp.Text.To.Offset >= maxStackSize {
+		largeStackFramesMu.Lock()
+		largeStackFrames = append(largeStackFrames, fn.Pos)
+		largeStackFramesMu.Unlock()
+		return
+	}
+
 	pp.Flush() // assemble, fill in boilerplate, etc.
 	// fieldtrack must be called after pp.Flush. See issue 20014.
 	fieldtrack(pp.Text.From.Sym, fn.Func.FieldTrack)
-	pp.Free()
 }
 
 func init() {
diff --git a/src/go/types/stdlib_test.go b/src/go/types/stdlib_test.go
index dd1510d37e..229d203099 100644
--- a/src/go/types/stdlib_test.go
+++ b/src/go/types/stdlib_test.go
@@ -175,6 +175,7 @@ func TestStdFixed(t *testing.T) {
 		"issue22200.go",  // go/types does not have constraints on stack size
 		"issue22200b.go", // go/types does not have constraints on stack size
 		"issue25507.go",  // go/types does not have constraints on stack size
+		"issue20780.go",  // go/types does not have constraints on stack size
 	)
 }
 
diff --git a/test/fixedbugs/issue20780.go b/test/fixedbugs/issue20780.go
new file mode 100644
index 0000000000..a31e031b78
--- /dev/null
+++ b/test/fixedbugs/issue20780.go
@@ -0,0 +1,20 @@
+// errorcheck
+
+// 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.
+
+// We have a limit of 1GB for stack frames.
+// Make sure we include the callee args section.
+// (The dispatch wrapper which implements (*S).f
+// copies the return value from f to a stack temp, then
+// from that stack temp to the return value of (*S).f.
+// It uses ~800MB for each section.)
+
+package main
+
+type S struct {
+	i interface {
+		f() [800e6]byte
+	}
+}