[dev.typeparams] runtime: use func() for deferred functions

aclements · aclements · commit ea94e5d3c57f · 2021-07-30T18:49:41.000Z
Prior to regabi, a deferred function could have any signature, so the runtime always manipulated them as funcvals. Now, a deferred function is always func(). Hence, this CL makes the runtime's manipulation of deferred functions more type-safe by using func() directly instead of *funcval. Change-Id: Ib55f38ed49107f74149725c65044e4690761971d Reviewed-on: https://go-review.googlesource.com/c/go/+/337650 Trust: Austin Clements <austin@google.com> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s
@@ -662,7 +662,7 @@ TEXT ·publicationBarrier(SB),NOSPLIT,$0-0
 	// compile barrier.
 	RET
 
-// func jmpdefer(fv *funcval, argp uintptr)
+// func jmpdefer(fv func(), argp uintptr)
 // argp is a caller SP.
 // called from deferreturn.
 // 1. pop the caller
diff --git a/src/runtime/asm_riscv64.s b/src/runtime/asm_riscv64.s
@@ -248,7 +248,7 @@ TEXT gogo<>(SB), NOSPLIT|NOFRAME, $0
 	MOV	gobuf_pc(T0), T0
 	JALR	ZERO, T0
 
-// func jmpdefer(fv *funcval, argp uintptr)
+// func jmpdefer(fv func(), argp uintptr)
 // called from deferreturn
 // 1. grab stored return address from the caller's frame
 // 2. sub 8 bytes to get back to JAL deferreturn
diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go
@@ -381,12 +381,13 @@ func dumpgoroutine(gp *g) {
 		dumpint(uint64(uintptr(unsafe.Pointer(gp))))
 		dumpint(uint64(d.sp))
 		dumpint(uint64(d.pc))
-		dumpint(uint64(uintptr(unsafe.Pointer(d.fn))))
+		fn := *(**funcval)(unsafe.Pointer(&d.fn))
+		dumpint(uint64(uintptr(unsafe.Pointer(fn))))
 		if d.fn == nil {
 			// d.fn can be nil for open-coded defers
 			dumpint(uint64(0))
 		} else {
-			dumpint(uint64(uintptr(unsafe.Pointer(d.fn.fn))))
+			dumpint(uint64(uintptr(unsafe.Pointer(fn.fn))))
 		}
 		dumpint(uint64(uintptr(unsafe.Pointer(d.link))))
 	}
diff --git a/src/runtime/panic.go b/src/runtime/panic.go
@@ -227,7 +227,7 @@ func panicmemAddr(addr uintptr) {
 
 // Create a new deferred function fn, which has no arguments and results.
 // The compiler turns a defer statement into a call to this.
-func deferproc(fn *funcval) { // TODO: Make deferproc just take a func().
+func deferproc(fn func()) {
 	gp := getg()
 	if gp.m.curg != gp {
 		// go code on the system stack can't defer
@@ -303,16 +303,6 @@ func deferprocStack(d *_defer) {
 	// been set and must not be clobbered.
 }
 
-// deferFunc returns d's deferred function. This is temporary while we
-// support both modes of GOEXPERIMENT=regabidefer. Once we commit to
-// that experiment, we should change the type of d.fn.
-//go:nosplit
-func deferFunc(d *_defer) func() {
-	var fn func()
-	*(**funcval)(unsafe.Pointer(&fn)) = d.fn
-	return fn
-}
-
 // Each P holds a pool for defers.
 
 // Allocate a Defer, usually using per-P pool.
@@ -470,9 +460,8 @@ func deferreturn() {
 	// If the defer function pointer is nil, force the seg fault to happen
 	// here rather than in jmpdefer. gentraceback() throws an error if it is
 	// called with a callback on an LR architecture and jmpdefer is on the
-	// stack, because the stack trace can be incorrect in that case - see
-	// issue #8153).
-	_ = fn.fn
+	// stack, because jmpdefer manipulates SP (see issue #8153).
+	_ = **(**funcval)(unsafe.Pointer(&fn))
 	jmpdefer(fn, argp)
 }
 
@@ -536,7 +525,7 @@ func Goexit() {
 		} else {
 			// Save the pc/sp in deferCallSave(), so we can "recover" back to this
 			// loop if necessary.
-			deferCallSave(&p, deferFunc(d))
+			deferCallSave(&p, d.fn)
 		}
 		if p.aborted {
 			// We had a recursive panic in the defer d we started, and
@@ -728,12 +717,14 @@ func runOpenDeferFrame(gp *g, d *_defer) bool {
 		if deferBits&(1<<i) == 0 {
 			continue
 		}
-		closure := *(**funcval)(unsafe.Pointer(d.varp - uintptr(closureOffset)))
+		closure := *(*func())(unsafe.Pointer(d.varp - uintptr(closureOffset)))
 		d.fn = closure
 		deferBits = deferBits &^ (1 << i)
 		*(*uint8)(unsafe.Pointer(d.varp - uintptr(deferBitsOffset))) = deferBits
 		p := d._panic
-		deferCallSave(p, deferFunc(d))
+		// Call the defer. Note that this can change d.varp if
+		// the stack moves.
+		deferCallSave(p, d.fn)
 		if p != nil && p.aborted {
 			break
 		}
@@ -854,8 +845,7 @@ func gopanic(e interface{}) {
 			}
 		} else {
 			p.argp = unsafe.Pointer(getargp())
-			fn := deferFunc(d)
-			fn()
+			d.fn()
 		}
 		p.argp = nil
 
diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
@@ -953,10 +953,10 @@ type _defer struct {
 	// defers. We have only one defer record for the entire frame (which may
 	// currently have 0, 1, or more defers active).
 	openDefer bool
-	sp        uintptr  // sp at time of defer
-	pc        uintptr  // pc at time of defer
-	fn        *funcval // can be nil for open-coded defers
-	_panic    *_panic  // panic that is running defer
+	sp        uintptr // sp at time of defer
+	pc        uintptr // pc at time of defer
+	fn        func()  // can be nil for open-coded defers
+	_panic    *_panic // panic that is running defer
 	link      *_defer
 
 	// If openDefer is true, the fields below record values about the stack
diff --git a/src/runtime/stubs.go b/src/runtime/stubs.go
@@ -177,7 +177,7 @@ func cgocallback(fn, frame, ctxt uintptr)
 func gogo(buf *gobuf)
 
 //go:noescape
-func jmpdefer(fv *funcval, argp uintptr)
+func jmpdefer(fv func(), argp uintptr)
 func asminit()
 func setg(gg *g)
 func breakpoint()
