From f95beae61d21898710d7d405ac39bc7b3b205c79 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 16 Sep 2014 10:36:38 -0400 Subject: runtime: use traceback to traverse defer structures This makes the GC and the stack copying agree about how to interpret the defer structures. Previously, only the stack copying treated them precisely. This removes an untyped memory allocation and fixes at least three copystack bugs. To make sure the GC can find the deferred argument frame until it has been copied, keep a Defer on the defer list during its execution. In addition to making it possible to remove the untyped memory allocation, keeping the Defer on the list fixes two races between copystack and execution of defers (in both gopanic and Goexit). The problem is that once the defer has been taken off the list, a stack copy that happens before the deferred arguments have been copied back to the stack will not update the arguments correctly. The new tests TestDeferPtrsPanic and TestDeferPtrsGoexit (variations on the existing TestDeferPtrs) pass now but failed before this CL. In addition to those fixes, keeping the Defer on the list helps correct a dangling pointer error during copystack. The traceback routines walk the Defer chain to provide information about where a panic may resume execution. When the executing Defer was not on the Defer chain but instead linked from the Panic chain, the traceback had to walk the Panic chain too. But Panic structs are on the stack and being updated by copystack. Traceback's use of the Panic chain while copystack is updating those structs means that it can follow an updated pointer and find itself reading from the new stack. The new stack is usually all zeros, so it sees an incorrect early end to the chain. The new TestPanicUseStack makes this happen at tip and dies when adjustdefers finds an unexpected argp. The new StackCopyPoison mode causes an earlier bad dereference instead. By keeping the Defer on the list, traceback can avoid walking the Panic chain at all, making it okay for copystack to update the Panics. We'd have the same problem for any Defers on the stack. There was only one: gopanic's dabort. Since we are not taking the executing Defer off the chain, we can use it to do what dabort was doing, and then there are no Defers on the stack ever, so it is okay for traceback to use the Defer chain even while copystack is executing: copystack cannot modify the Defer chain. LGTM=khr R=khr CC=dvyukov, golang-codereviews, iant, rlh https://golang.org/cl/141490043 --- src/runtime/panic.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/runtime/panic.c') diff --git a/src/runtime/panic.c b/src/runtime/panic.c index e015e41cca..1cd0aa8654 100644 --- a/src/runtime/panic.c +++ b/src/runtime/panic.c @@ -18,7 +18,8 @@ uint32 runtime·panicking; static Mutex paniclk; void -runtime·deferproc_m(void) { +runtime·deferproc_m(void) +{ int32 siz; FuncVal *fn; uintptr argp; @@ -35,7 +36,7 @@ runtime·deferproc_m(void) { d->fn = fn; d->pc = callerpc; d->argp = argp; - runtime·memmove(d->args, (void*)argp, siz); + runtime·memmove(d+1, (void*)argp, siz); } // Unwind the stack after a deferred function calls recover -- cgit v1.3-6-g1900