Skip to content

Commit

Permalink
runtime: use correct pc to obtain liveness info during stack copy
Browse files Browse the repository at this point in the history
The old code was using the PC of the instruction after the CALL.
Variables live during the call but not live when it returns would
not be seen as live during the stack copy, which might lead to
corruption. The correct PC to use is the one just before the
return address. After this CL the lookup matches what mgc0.c does.

The only time this matters is if you have back to back CALL instructions:

        CALL f1 // x live here
        CALL f2 // x no longer live

If a stack copy occurs during the execution of f1, the old code will
use the liveness bitmap intended for the execution of f2 and will not
treat x as live.

The only way this situation can arise and cause a problem in a stack copy
is if x lives on the stack has had its address taken but the compiler knows
enough about the context to know that x is no longer needed once f1
returns. The compiler has never known that much, so using the f2 context
cannot currently cause incorrect execution. For the same reason, it is not
possible to write a test for this today.

CL 83090046 will make the compiler precise enough in some cases
that this distinction will start mattering. The existing stack growth tests
in package runtime will fail if that CL is submitted without this one.

While we're here, print the frame PC in debug mode and update the
bitmap interpretation strings.

LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/83250043
  • Loading branch information
rsc committed Apr 1, 2014
1 parent b700cb4 commit cfb347f
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions src/pkg/runtime/stack.c
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,9 @@ uintptr runtime·maxstacksize = 1<<20; // enough until runtime.main sets it for
static uint8*
mapnames[] = {
(uint8*)"---",
(uint8*)"scalar",
(uint8*)"ptr",
(uint8*)"iface",
(uint8*)"eface",
(uint8*)"multi",
};

// Stack frame layout
Expand Down Expand Up @@ -437,14 +437,18 @@ adjustframe(Stkframe *frame, void *arg)
StackMap *stackmap;
int32 pcdata;
BitVector *bv;
uintptr targetpc;

adjinfo = arg;
f = frame->fn;
if(StackDebug >= 2)
runtime·printf(" adjusting %s frame=[%p,%p]\n", runtime·funcname(f), frame->sp, frame->fp);
runtime·printf(" adjusting %s frame=[%p,%p] pc=%p\n", runtime·funcname(f), frame->sp, frame->fp, frame->pc);
if(f->entry == (uintptr)runtime·main)
return true;
pcdata = runtime·pcdatavalue(f, PCDATA_StackMapIndex, frame->pc);
targetpc = frame->pc;
if(targetpc != f->entry)
targetpc--;
pcdata = runtime·pcdatavalue(f, PCDATA_StackMapIndex, targetpc);
if(pcdata == -1)
pcdata = 0; // in prologue

Expand Down

0 comments on commit cfb347f

Please sign in to comment.