cmd/compile: remove gc programs from stack frame objects

This is a two-pronged approach. First, try to keep large objects
off the stack frame. Second, if they do manage to appear anyway,
use straight bitmasks instead of gc programs.

Generally probably a good idea to keep large objects out of stack frames.
But particularly keeping gc programs off the stack simplifies
runtime code a bit.

This CL sets the limit of most stack objects to 131072 bytes (on 64-bit archs).
There can still be large objects if allocated by a late pass, like order, or
they are required to be on the stack, like function arguments.
But the size for the bitmasks for these objects isn't a huge deal,
as we have already have (probably several) bitmasks for the frame
liveness map itself.

Change-Id: I6d2bed0e9aa9ac7499955562c6154f9264061359
Reviewed-on: https://go-review.googlesource.com/c/go/+/542815
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>

src/cmd/compile/internal/gc/main.go

@@ -140,7 +140,7 @@ func Main(archInit func(*ssagen.ArchInfo)) {
 	}
 
 	if base.Flag.SmallFrames {
-		ir.MaxStackVarSize = 128 * 1024
+		ir.MaxStackVarSize = 64 * 1024
 		ir.MaxImplicitStackVarSize = 16 * 1024
 	}
 

src/cmd/compile/internal/ir/cfg.go

@@ -8,7 +8,7 @@ var (
 	// MaxStackVarSize is the maximum size variable which we will allocate on the stack.
 	// This limit is for explicit variable declarations like "var x T" or "x := ...".
 	// Note: the flag smallframes can update this value.
-	MaxStackVarSize = int64(10 * 1024 * 1024)
+	MaxStackVarSize = int64(128 * 1024)
 
 	// MaxImplicitStackVarSize is the maximum size of implicit variables that we will allocate on the stack.
 	//   p := new(T)          allocating T on the stack

src/cmd/compile/internal/liveness/plive.go

@@ -1473,12 +1473,9 @@ func (lv *liveness) emitStackObjects() *obj.LSym {
 		if sz != int64(int32(sz)) {
 			base.Fatalf("stack object too big: %v of type %v, size %d", v, t, sz)
 		}
-		lsym, useGCProg, ptrdata := reflectdata.GCSym(t)
-		if useGCProg {
-			ptrdata = -ptrdata
-		}
+		lsym, ptrBytes := reflectdata.GCSym(t)
 		off = objw.Uint32(x, off, uint32(sz))
-		off = objw.Uint32(x, off, uint32(ptrdata))
+		off = objw.Uint32(x, off, uint32(ptrBytes))
 		off = objw.SymPtrOff(x, off, lsym)
 	}
 

src/cmd/compile/internal/reflectdata/reflect.go

@@ -437,8 +437,10 @@ func dcommontype(c rttype.Cursor, t *types.Type) {
 		sptr = writeType(tptr)
 	}
 
-	gcsym, useGCProg, ptrdata := dgcsym(t, true)
-	delete(gcsymset, t)
+	gcsym, useGCProg, ptrdata := dgcsym(t, true, true)
+	if !useGCProg {
+		delete(gcsymset, t)
+	}
 
 	// ../../../../reflect/type.go:/^type.rtype
 	// actual type structure
@@ -1010,7 +1012,7 @@ func WriteGCSymbols() {
 	}
 	slices.SortFunc(gcsyms, typesStrCmp)
 	for _, ts := range gcsyms {
-		dgcsym(ts.t, true)
+		dgcsym(ts.t, true, false)
 	}
 }
 
@@ -1223,12 +1225,11 @@ func typesStrCmp(a, b typeAndStr) int {
 	return 0
 }
 
-// GCSym returns a data symbol containing GC information for type t, along
-// with a boolean reporting whether the UseGCProg bit should be set in the
-// type kind, and the ptrdata field to record in the reflect type information.
+// GCSym returns a data symbol containing GC information for type t.
+// GC information is always a bitmask, never a gc program.
 // GCSym may be called in concurrent backend, so it does not emit the symbol
 // content.
-func GCSym(t *types.Type) (lsym *obj.LSym, useGCProg bool, ptrdata int64) {
+func GCSym(t *types.Type) (lsym *obj.LSym, ptrdata int64) {
 	// Record that we need to emit the GC symbol.
 	gcsymmu.Lock()
 	if _, ok := gcsymset[t]; !ok {
@@ -1236,16 +1237,17 @@ func GCSym(t *types.Type) (lsym *obj.LSym, useGCProg bool, ptrdata int64) {
 	}
 	gcsymmu.Unlock()
 
-	return dgcsym(t, false)
+	lsym, _, ptrdata = dgcsym(t, false, false)
+	return
 }
 
 // dgcsym returns a data symbol containing GC information for type t, along
 // with a boolean reporting whether the UseGCProg bit should be set in the
 // type kind, and the ptrdata field to record in the reflect type information.
 // When write is true, it writes the symbol data.
-func dgcsym(t *types.Type, write bool) (lsym *obj.LSym, useGCProg bool, ptrdata int64) {
+func dgcsym(t *types.Type, write, gcProgAllowed bool) (lsym *obj.LSym, useGCProg bool, ptrdata int64) {
 	ptrdata = types.PtrDataSize(t)
-	if ptrdata/int64(types.PtrSize) <= abi.MaxPtrmaskBytes*8 {
+	if !gcProgAllowed || ptrdata/int64(types.PtrSize) <= abi.MaxPtrmaskBytes*8 {
 		lsym = dgcptrmask(t, write)
 		return
 	}

src/runtime/mgcmark.go

@@ -951,31 +951,12 @@ func scanstack(gp *g, gcw *gcWork) int64 {
 			println()
 			printunlock()
 		}
-		gcdata := r.gcdata()
-		var s *mspan
-		if r.useGCProg() {
-			// This path is pretty unlikely, an object large enough
-			// to have a GC program allocated on the stack.
-			// We need some space to unpack the program into a straight
-			// bitmask, which we allocate/free here.
-			// TODO: it would be nice if there were a way to run a GC
-			// program without having to store all its bits. We'd have
-			// to change from a Lempel-Ziv style program to something else.
-			// Or we can forbid putting objects on stacks if they require
-			// a gc program (see issue 27447).
-			s = materializeGCProg(r.ptrdata(), gcdata)
-			gcdata = (*byte)(unsafe.Pointer(s.startAddr))
-		}
-
+		ptrBytes, gcData := r.gcdata()
 		b := state.stack.lo + uintptr(obj.off)
 		if conservative {
-			scanConservative(b, r.ptrdata(), gcdata, gcw, &state)
+			scanConservative(b, ptrBytes, gcData, gcw, &state)
 		} else {
-			scanblock(b, r.ptrdata(), gcdata, gcw, &state)
-		}
-
-		if s != nil {
-			dematerializeGCProg(s)
+			scanblock(b, ptrBytes, gcData, gcw, &state)
 		}
 	}
 

src/runtime/stack.go

@@ -722,22 +722,12 @@ func adjustframe(frame *stkframe, adjinfo *adjustinfo) {
 				// we call into morestack.)
 				continue
 			}
-			ptrdata := obj.ptrdata()
-			gcdata := obj.gcdata()
-			var s *mspan
-			if obj.useGCProg() {
-				// See comments in mgcmark.go:scanstack
-				s = materializeGCProg(ptrdata, gcdata)
-				gcdata = (*byte)(unsafe.Pointer(s.startAddr))
-			}
-			for i := uintptr(0); i < ptrdata; i += goarch.PtrSize {
-				if *addb(gcdata, i/(8*goarch.PtrSize))>>(i/goarch.PtrSize&7)&1 != 0 {
+			ptrBytes, gcData := obj.gcdata()
+			for i := uintptr(0); i < ptrBytes; i += goarch.PtrSize {
+				if *addb(gcData, i/(8*goarch.PtrSize))>>(i/goarch.PtrSize&7)&1 != 0 {
 					adjustpointer(adjinfo, unsafe.Pointer(p+i))
 				}
 			}
-			if s != nil {
-				dematerializeGCProg(s)
-			}
 		}
 	}
 }
@@ -1288,24 +1278,14 @@ type stackObjectRecord struct {
 	// if non-negative, offset from argp
 	off       int32
 	size      int32
-	_ptrdata  int32  // ptrdata, or -ptrdata is GC prog is used
+	ptrBytes  int32
 	gcdataoff uint32 // offset to gcdata from moduledata.rodata
 }
 
-func (r *stackObjectRecord) useGCProg() bool {
-	return r._ptrdata < 0
-}
-
-func (r *stackObjectRecord) ptrdata() uintptr {
-	x := r._ptrdata
-	if x < 0 {
-		return uintptr(-x)
-	}
-	return uintptr(x)
-}
-
-// gcdata returns pointer map or GC prog of the type.
-func (r *stackObjectRecord) gcdata() *byte {
+// gcdata returns the number of bytes that contain pointers, and
+// a ptr/nonptr bitmask covering those bytes.
+// Note that this bitmask might be larger than internal/abi.MaxPtrmaskBytes.
+func (r *stackObjectRecord) gcdata() (uintptr, *byte) {
 	ptr := uintptr(unsafe.Pointer(r))
 	var mod *moduledata
 	for datap := &firstmoduledata; datap != nil; datap = datap.next {
@@ -1318,7 +1298,7 @@ func (r *stackObjectRecord) gcdata() *byte {
 	// you may have made a copy of a stackObjectRecord.
 	// You must use the original pointer.
 	res := mod.rodata + uintptr(r.gcdataoff)
-	return (*byte)(unsafe.Pointer(res))
+	return uintptr(r.ptrBytes), (*byte)(unsafe.Pointer(res))
 }
 
 // This is exported as ABI0 via linkname so obj can call it.

src/runtime/stkframe.go

@@ -283,7 +283,7 @@ func stkobjinit() {
 	methodValueCallFrameObjs[0] = stackObjectRecord{
 		off:       -int32(alignUp(abiRegArgsType.Size_, 8)), // It's always the highest address local.
 		size:      int32(abiRegArgsType.Size_),
-		_ptrdata:  int32(abiRegArgsType.PtrBytes),
+		ptrBytes:  int32(abiRegArgsType.PtrBytes),
 		gcdataoff: uint32(uintptr(unsafe.Pointer(abiRegArgsType.GCData)) - mod.rodata),
 	}
 }

test/fixedbugs/issue39292.go

@@ -27,3 +27,12 @@ func z() {
 	z := t{&i}.f // ERROR "t{...}.f escapes to heap"
 	z()
 }
+
+// Should match cmd/compile/internal/ir/cfg.go:MaxStackVarSize.
+const maxStack = 128 * 1024
+
+func w(i int) byte {
+	var x [maxStack]byte
+	var y [maxStack + 1]byte // ERROR "moved to heap: y"
+	return x[i] + y[i]
+}