Revert "cmd/compile,runtime: allocate defer records on the stack"

This reverts commit fff4f59.

Reason for revert: Seems to still have issues around GC.

Fixes #32452

Change-Id: Ibe7af629f9ad6a3d5312acd7b066123f484da7f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/180761
Run-TryBot: Keith Randall <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Josh Bleecher Snyder <[email protected]>

Author: randall77

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

@@ -802,7 +802,6 @@ opSwitch:
 
 	case ODEFER:
 		if e.loopdepth == 1 { // top level
-			n.Esc = EscNever // force stack allocation of defer record (see ssa.go)
 			break
 		}
 		// arguments leak out of scope

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

@@ -882,7 +882,6 @@ func (e *Escape) augmentParamHole(k EscHole, where *Node) EscHole {
 	// non-transient location to avoid arguments from being
 	// transiently allocated.
 	if where.Op == ODEFER && e.loopDepth == 1 {
-		where.Esc = EscNever // force stack allocation of defer record (see ssa.go)
 		// TODO(mdempsky): Eliminate redundant EscLocation allocs.
 		return e.teeHole(k, e.newLoc(nil, false).asHole())
 	}

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

@@ -287,7 +287,6 @@ var (
 	assertI2I,
 	assertI2I2,
 	deferproc,
-	deferprocStack,
 	Deferreturn,
 	Duffcopy,
 	Duffzero,

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

@@ -317,48 +317,6 @@ func hiter(t *types.Type) *types.Type {
 	return hiter
 }
 
-// deferstruct makes a runtime._defer structure, with additional space for
-// stksize bytes of args.
-func deferstruct(stksize int64) *types.Type {
-	makefield := func(name string, typ *types.Type) *types.Field {
-		f := types.NewField()
-		f.Type = typ
-		// Unlike the global makefield function, this one needs to set Pkg
-		// because these types might be compared (in SSA CSE sorting).
-		// TODO: unify this makefield and the global one above.
-		f.Sym = &types.Sym{Name: name, Pkg: localpkg}
-		return f
-	}
-	argtype := types.NewArray(types.Types[TUINT8], stksize)
-	argtype.SetNoalg(true)
-	argtype.Width = stksize
-	argtype.Align = 1
-	// These fields must match the ones in runtime/runtime2.go:_defer and
-	// cmd/compile/internal/gc/ssa.go:(*state).call.
-	fields := []*types.Field{
-		makefield("siz", types.Types[TUINT32]),
-		makefield("started", types.Types[TBOOL]),
-		makefield("heap", types.Types[TBOOL]),
-		makefield("sp", types.Types[TUINTPTR]),
-		makefield("pc", types.Types[TUINTPTR]),
-		// Note: the types here don't really matter. Defer structures
-		// are always scanned explicitly during stack copying and GC,
-		// so we make them uintptr type even though they are real pointers.
-		makefield("fn", types.Types[TUINTPTR]),
-		makefield("_panic", types.Types[TUINTPTR]),
-		makefield("link", types.Types[TUINTPTR]),
-		makefield("args", argtype),
-	}
-
-	// build struct holding the above fields
-	s := types.New(TSTRUCT)
-	s.SetNoalg(true)
-	s.SetFields(fields)
-	s.Width = widstruct(s, s, 0, 1)
-	s.Align = uint8(Widthptr)
-	return s
-}
-
 // f is method type, with receiver.
 // return function type, receiver as first argument (or not).
 func methodfunc(f *types.Type, receiver *types.Type) *types.Type {

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

@@ -68,7 +68,6 @@ func initssaconfig() {
 	assertI2I = sysfunc("assertI2I")
 	assertI2I2 = sysfunc("assertI2I2")
 	deferproc = sysfunc("deferproc")
-	deferprocStack = sysfunc("deferprocStack")
 	Deferreturn = sysfunc("deferreturn")
 	Duffcopy = sysvar("duffcopy")             // asm func with special ABI
 	Duffzero = sysvar("duffzero")             // asm func with special ABI
@@ -865,11 +864,7 @@ func (s *state) stmt(n *Node) {
 			}
 		}
 	case ODEFER:
-		d := callDefer
-		if n.Esc == EscNever {
-			d = callDeferStack
-		}
-		s.call(n.Left, d)
+		s.call(n.Left, callDefer)
 	case OGO:
 		s.call(n.Left, callGo)
 
@@ -2864,7 +2859,6 @@ type callKind int8
 const (
 	callNormal callKind = iota
 	callDefer
-	callDeferStack
 	callGo
 )
 
@@ -3805,132 +3799,74 @@ func (s *state) call(n *Node, k callKind) *ssa.Value {
 		rcvr = s.newValue1(ssa.OpIData, types.Types[TUINTPTR], i)
 	}
 	dowidth(fn.Type)
-	stksize := fn.Type.ArgWidth() // includes receiver, args, and results
+	stksize := fn.Type.ArgWidth() // includes receiver
 
 	// Run all assignments of temps.
 	// The temps are introduced to avoid overwriting argument
 	// slots when arguments themselves require function calls.
 	s.stmtList(n.List)
 
+	// Store arguments to stack, including defer/go arguments and receiver for method calls.
+	// These are written in SP-offset order.
+	argStart := Ctxt.FixedFrameSize()
+	// Defer/go args.
+	if k != callNormal {
+		// Write argsize and closure (args to newproc/deferproc).
+		argsize := s.constInt32(types.Types[TUINT32], int32(stksize))
+		addr := s.constOffPtrSP(s.f.Config.Types.UInt32Ptr, argStart)
+		s.store(types.Types[TUINT32], addr, argsize)
+		addr = s.constOffPtrSP(s.f.Config.Types.UintptrPtr, argStart+int64(Widthptr))
+		s.store(types.Types[TUINTPTR], addr, closure)
+		stksize += 2 * int64(Widthptr)
+		argStart += 2 * int64(Widthptr)
+	}
+
+	// Set receiver (for interface calls).
+	if rcvr != nil {
+		addr := s.constOffPtrSP(s.f.Config.Types.UintptrPtr, argStart)
+		s.store(types.Types[TUINTPTR], addr, rcvr)
+	}
+
+	// Write args.
+	t := n.Left.Type
+	args := n.Rlist.Slice()
+	if n.Op == OCALLMETH {
+		f := t.Recv()
+		s.storeArg(args[0], f.Type, argStart+f.Offset)
+		args = args[1:]
+	}
+	for i, n := range args {
+		f := t.Params().Field(i)
+		s.storeArg(n, f.Type, argStart+f.Offset)
+	}
+
+	// call target
 	var call *ssa.Value
-	if k == callDeferStack {
-		// Make a defer struct d on the stack.
-		t := deferstruct(stksize)
-		d := tempAt(n.Pos, s.curfn, t)
-
-		s.vars[&memVar] = s.newValue1A(ssa.OpVarDef, types.TypeMem, d, s.mem())
-		addr := s.addr(d, false)
-
-		// Must match reflect.go:deferstruct and src/runtime/runtime2.go:_defer.
-		// 0: siz
-		s.store(types.Types[TUINT32],
-			s.newValue1I(ssa.OpOffPtr, types.Types[TUINT32].PtrTo(), t.FieldOff(0), addr),
-			s.constInt32(types.Types[TUINT32], int32(stksize)))
-		// 1: started, set in deferprocStack
-		// 2: heap, set in deferprocStack
-		// 3: sp, set in deferprocStack
-		// 4: pc, set in deferprocStack
-		// 5: fn
-		s.store(closure.Type,
-			s.newValue1I(ssa.OpOffPtr, closure.Type.PtrTo(), t.FieldOff(5), addr),
-			closure)
-		// 6: panic, set in deferprocStack
-		// 7: link, set in deferprocStack
-
-		// Then, store all the arguments of the defer call.
-		ft := fn.Type
-		off := t.FieldOff(8)
-		args := n.Rlist.Slice()
-
-		// Set receiver (for interface calls). Always a pointer.
-		if rcvr != nil {
-			p := s.newValue1I(ssa.OpOffPtr, ft.Recv().Type.PtrTo(), off, addr)
-			s.store(types.Types[TUINTPTR], p, rcvr)
-		}
-		// Set receiver (for method calls).
-		if n.Op == OCALLMETH {
-			f := ft.Recv()
-			s.storeArgWithBase(args[0], f.Type, addr, off+f.Offset)
-			args = args[1:]
-		}
-		// Set other args.
-		for _, f := range ft.Params().Fields().Slice() {
-			s.storeArgWithBase(args[0], f.Type, addr, off+f.Offset)
-			args = args[1:]
-		}
-
-		// Call runtime.deferprocStack with pointer to _defer record.
-		arg0 := s.constOffPtrSP(types.Types[TUINTPTR], Ctxt.FixedFrameSize())
-		s.store(types.Types[TUINTPTR], arg0, addr)
-		call = s.newValue1A(ssa.OpStaticCall, types.TypeMem, deferprocStack, s.mem())
-		if stksize < int64(Widthptr) {
-			// We need room for both the call to deferprocStack and the call to
-			// the deferred function.
-			stksize = int64(Widthptr)
-		}
-		call.AuxInt = stksize
-	} else {
-		// Store arguments to stack, including defer/go arguments and receiver for method calls.
-		// These are written in SP-offset order.
-		argStart := Ctxt.FixedFrameSize()
-		// Defer/go args.
-		if k != callNormal {
-			// Write argsize and closure (args to newproc/deferproc).
-			argsize := s.constInt32(types.Types[TUINT32], int32(stksize))
-			addr := s.constOffPtrSP(s.f.Config.Types.UInt32Ptr, argStart)
-			s.store(types.Types[TUINT32], addr, argsize)
-			addr = s.constOffPtrSP(s.f.Config.Types.UintptrPtr, argStart+int64(Widthptr))
-			s.store(types.Types[TUINTPTR], addr, closure)
-			stksize += 2 * int64(Widthptr)
-			argStart += 2 * int64(Widthptr)
-		}
-
-		// Set receiver (for interface calls).
-		if rcvr != nil {
-			addr := s.constOffPtrSP(s.f.Config.Types.UintptrPtr, argStart)
-			s.store(types.Types[TUINTPTR], addr, rcvr)
-		}
-
-		// Write args.
-		t := n.Left.Type
-		args := n.Rlist.Slice()
-		if n.Op == OCALLMETH {
-			f := t.Recv()
-			s.storeArg(args[0], f.Type, argStart+f.Offset)
-			args = args[1:]
-		}
-		for i, n := range args {
-			f := t.Params().Field(i)
-			s.storeArg(n, f.Type, argStart+f.Offset)
-		}
-
-		// call target
-		switch {
-		case k == callDefer:
-			call = s.newValue1A(ssa.OpStaticCall, types.TypeMem, deferproc, s.mem())
-		case k == callGo:
-			call = s.newValue1A(ssa.OpStaticCall, types.TypeMem, newproc, s.mem())
-		case closure != nil:
-			// rawLoad because loading the code pointer from a
-			// closure is always safe, but IsSanitizerSafeAddr
-			// can't always figure that out currently, and it's
-			// critical that we not clobber any arguments already
-			// stored onto the stack.
-			codeptr = s.rawLoad(types.Types[TUINTPTR], closure)
-			call = s.newValue3(ssa.OpClosureCall, types.TypeMem, codeptr, closure, s.mem())
-		case codeptr != nil:
-			call = s.newValue2(ssa.OpInterCall, types.TypeMem, codeptr, s.mem())
-		case sym != nil:
-			call = s.newValue1A(ssa.OpStaticCall, types.TypeMem, sym.Linksym(), s.mem())
-		default:
-			Fatalf("bad call type %v %v", n.Op, n)
-		}
-		call.AuxInt = stksize // Call operations carry the argsize of the callee along with them
+	switch {
+	case k == callDefer:
+		call = s.newValue1A(ssa.OpStaticCall, types.TypeMem, deferproc, s.mem())
+	case k == callGo:
+		call = s.newValue1A(ssa.OpStaticCall, types.TypeMem, newproc, s.mem())
+	case closure != nil:
+		// rawLoad because loading the code pointer from a
+		// closure is always safe, but IsSanitizerSafeAddr
+		// can't always figure that out currently, and it's
+		// critical that we not clobber any arguments already
+		// stored onto the stack.
+		codeptr = s.rawLoad(types.Types[TUINTPTR], closure)
+		call = s.newValue3(ssa.OpClosureCall, types.TypeMem, codeptr, closure, s.mem())
+	case codeptr != nil:
+		call = s.newValue2(ssa.OpInterCall, types.TypeMem, codeptr, s.mem())
+	case sym != nil:
+		call = s.newValue1A(ssa.OpStaticCall, types.TypeMem, sym.Linksym(), s.mem())
+	default:
+		Fatalf("bad call type %v %v", n.Op, n)
 	}
+	call.AuxInt = stksize // Call operations carry the argsize of the callee along with them
 	s.vars[&memVar] = call
 
 	// Finish block for defers
-	if k == callDefer || k == callDeferStack {
+	if k == callDefer {
 		b := s.endBlock()
 		b.Kind = ssa.BlockDefer
 		b.SetControl(call)
@@ -4425,27 +4361,17 @@ func (s *state) storeTypePtrs(t *types.Type, left, right *ssa.Value) {
 }
 
 func (s *state) storeArg(n *Node, t *types.Type, off int64) {
-	s.storeArgWithBase(n, t, s.sp, off)
-}
-
-func (s *state) storeArgWithBase(n *Node, t *types.Type, base *ssa.Value, off int64) {
 	pt := types.NewPtr(t)
-	var addr *ssa.Value
-	if base == s.sp {
-		// Use special routine that avoids allocation on duplicate offsets.
-		addr = s.constOffPtrSP(pt, off)
-	} else {
-		addr = s.newValue1I(ssa.OpOffPtr, pt, off, base)
-	}
+	sp := s.constOffPtrSP(pt, off)
 
 	if !canSSAType(t) {
 		a := s.addr(n, false)
-		s.move(t, addr, a)
+		s.move(t, sp, a)
 		return
 	}
 
 	a := s.expr(n)
-	s.storeType(t, addr, a, 0, false)
+	s.storeType(t, sp, a, 0, false)
 }
 
 // slice computes the slice v[i:j:k] and returns ptr, len, and cap of result.

src/runtime/mgcmark.go

@@ -712,31 +712,15 @@ func scanstack(gp *g, gcw *gcWork) {
 
 	// Find additional pointers that point into the stack from the heap.
 	// Currently this includes defers and panics. See also function copystack.
-
-	// Find and trace all defer arguments.
 	tracebackdefers(gp, scanframe, nil)
-
-	// Find and trace other pointers in defer records.
 	for d := gp._defer; d != nil; d = d.link {
+		// tracebackdefers above does not scan the func value, which could
+		// be a stack allocated closure. See issue 30453.
 		if d.fn != nil {
-			// tracebackdefers above does not scan the func value, which could
-			// be a stack allocated closure. See issue 30453.
 			scanblock(uintptr(unsafe.Pointer(&d.fn)), sys.PtrSize, &oneptrmask[0], gcw, &state)
 		}
-		if d.link != nil {
-			// The link field of a stack-allocated defer record might point
-			// to a heap-allocated defer record. Keep that heap record live.
-			scanblock(uintptr(unsafe.Pointer(&d.link)), sys.PtrSize, &oneptrmask[0], gcw, &state)
-		}
-		// Retain defers records themselves.
-		// Defer records might not be reachable from the G through regular heap
-		// tracing because the defer linked list might weave between the stack and the heap.
-		if d.heap {
-			scanblock(uintptr(unsafe.Pointer(&d)), sys.PtrSize, &oneptrmask[0], gcw, &state)
-		}
 	}
 	if gp._panic != nil {
-		// Panics are always stack allocated.
 		state.putPtr(uintptr(unsafe.Pointer(gp._panic)))
 	}