runtime: adjust traceback / garbage collector boundary

The garbage collection routine addframeroots is duplicating
logic in the traceback routine that calls it, sometimes correctly,
sometimes incorrectly, sometimes incompletely.
Pass necessary information to addframeroots instead of
deriving it anew.

Should make addframeroots significantly more robust.
It's certainly smaller.

Also try to standardize on uintptr for saved pc, sp values.

Will make CL 10036044 trivial.

R=golang-dev, dave, dvyukov
CC=golang-dev
https://golang.org/cl/10169045

src/pkg/runtime/mgc0.c

@@ -1385,54 +1385,41 @@ addroot(Obj obj)
 	work.nroot++;
 }
 
-// Scan a stack frame.  Normally, this scans the locals area,
-// belonging to the current frame, and the arguments area, belonging
-// to the calling frame.  When the arguments area size is unknown, the
-// arguments area scanning is delayed and the doframe parameter
-// signals that the previously scanned activation has an unknown
-// argument size.  When *doframe is true, the possible arguments area
-// for the callee, located between the stack pointer and the bottom of
-// the locals area, is additionally scanned.  Otherwise, this area is
-// ignored, as it must have been scanned when the callee was scanned.
+// Scan a stack frame: local variables and function arguments/results.
 static void
-addframeroots(Func *f, byte*, byte *sp, void *doframe)
+addframeroots(Stkframe *frame, void*)
 {
-	byte *fp, *ap;
-	uintptr outs;
-	int32 i, j, rem;
+	Func *f;
+	byte *ap;
+	int32 i, j, nuintptr;
 	uint32 w, b;
 
-	if(thechar == '5')
-		sp += sizeof(uintptr);
-	fp = sp + f->frame;
-	if(f->locals == 0 || *(bool*)doframe == true)
-		// Scan the entire stack frame.
-		addroot((Obj){sp, f->frame - sizeof(uintptr), 0});
-	else if(f->locals > 0) {
-		// Scan the locals area.
-		outs = f->frame - sizeof(uintptr) - f->locals;
-		addroot((Obj){sp + outs, f->locals, 0});
-	}
-	if(f->args > 0) {
-		// Scan the arguments area.
-		if(f->ptrs.array != nil) {
-			ap = fp;
-			rem = f->args / sizeof(uintptr);
-			for(i = 0; i < f->ptrs.len; i++) {
-				w = ((uint32*)f->ptrs.array)[i];
-				b = 1;
-				for((j = (rem < 32) ? rem : 32); j > 0; j--) {
-					if(w & b)
-						addroot((Obj){ap, sizeof(uintptr), 0});
-					b <<= 1;
-					ap += sizeof(uintptr);
-				}
-				rem -= 32;
+	// Scan local variables if stack frame has been allocated.
+	if(frame->varlen > 0)
+		addroot((Obj){frame->varp, frame->varlen, 0});
+
+	// Scan arguments.
+	// Use pointer information if known.
+	f = frame->fn;
+	if(f->args > 0 && f->ptrs.array != nil) {
+		ap = frame->argp;
+		nuintptr = f->args / sizeof(uintptr);
+		for(i = 0; i < f->ptrs.len; i++) {
+			w = ((uint32*)f->ptrs.array)[i];
+			b = 1;
+			j = nuintptr;
+			if(j > 32)
+				j = 32;
+			for(; j > 0; j--) {
+				if(w & b)
+					addroot((Obj){ap, sizeof(uintptr), 0});
+				b <<= 1;
+				ap += sizeof(uintptr);
 			}
-		} else
-			addroot((Obj){fp, f->args, 0});
-	}
-	*(bool*)doframe = (f->args == ArgsSizeUnknown);
+			nuintptr -= 32;
+		}
+	} else
+		addroot((Obj){frame->argp, frame->arglen, 0});
 }
 
 static void
@@ -1441,12 +1428,10 @@ addstackroots(G *gp)
 	M *mp;
 	int32 n;
 	Stktop *stk;
-	byte *sp, *guard, *pc;
-	Func *f;
-	bool doframe;
+	uintptr sp, guard, pc;
 
 	stk = (Stktop*)gp->stackbase;
-	guard = (byte*)gp->stackguard;
+	guard = gp->stackguard;
 
 	if(gp == g)
 		runtime·throw("can't scan our own stack");
@@ -1458,51 +1443,30 @@ addstackroots(G *gp)
 		// as schedlock and may have needed to start a new stack segment.
 		// Use the stack segment and stack pointer at the time of
 		// the system call instead, since that won't change underfoot.
-		sp = (byte*)gp->gcsp;
+		sp = gp->gcsp;
 		pc = gp->gcpc;
 		stk = (Stktop*)gp->gcstack;
-		guard = (byte*)gp->gcguard;
+		guard = gp->gcguard;
 	} else {
 		// Scanning another goroutine's stack.
 		// The goroutine is usually asleep (the world is stopped).
-		sp = (byte*)gp->sched.sp;
+		sp = gp->sched.sp;
 		pc = gp->sched.pc;
-		if(ScanStackByFrames && pc == (byte*)runtime·goexit && gp->fnstart != nil) {
-			// The goroutine has not started. However, its incoming
-			// arguments are live at the top of the stack and must
-			// be scanned.  No other live values should be on the
-			// stack.
-			f = runtime·findfunc((uintptr)gp->fnstart->fn);
-			if(f->args != 0) {
-				if(thechar == '5')
-					sp += sizeof(uintptr);
-				// If the size of the arguments is known
-				// scan just the incoming arguments.
-				// Otherwise, scan everything between the
-				// top and the bottom of the stack.
-				if(f->args > 0)
-					addroot((Obj){sp, f->args, 0});
-				else
-					addroot((Obj){sp, (byte*)stk - sp, 0}); 
-			} 
-			return;
-		}
 	}
 	if(ScanStackByFrames) {
 		USED(stk);
 		USED(guard);
-		doframe = false;
-		runtime·gentraceback(pc, sp, nil, gp, 0, nil, 0x7fffffff, addframeroots, &doframe);
+		runtime·gentraceback(pc, sp, 0, gp, 0, nil, 0x7fffffff, addframeroots, nil);
 	} else {
 		USED(pc);
 		n = 0;
 		while(stk) {
-			if(sp < guard-StackGuard || (byte*)stk < sp) {
+			if(sp < guard-StackGuard || (uintptr)stk < sp) {
 				runtime·printf("scanstack inconsistent: g%D#%d sp=%p not in [%p,%p]\n", gp->goid, n, sp, guard-StackGuard, stk);
 				runtime·throw("scanstack");
 			}
-			addroot((Obj){sp, (byte*)stk - sp, (uintptr)defaultProg | PRECISE | LOOP});
-			sp = (byte*)stk->gobuf.sp;
+			addroot((Obj){(byte*)sp, (uintptr)stk - sp, (uintptr)defaultProg | PRECISE | LOOP});
+			sp = stk->gobuf.sp;
 			guard = stk->stackguard;
 			stk = (Stktop*)stk->stackbase;
 			n++;

src/pkg/runtime/mprof.goc

@@ -441,10 +441,10 @@ func ThreadCreateProfile(p Slice) (n int, ok bool) {
 }
 
 func Stack(b Slice, all bool) (n int) {
-	byte *pc, *sp;
+	uintptr pc, sp;
 	
 	sp = runtime·getcallersp(&b);
-	pc = runtime·getcallerpc(&b);
+	pc = (uintptr)runtime·getcallerpc(&b);
 
 	if(all) {
 		runtime·semacquire(&runtime·worldsema);
@@ -474,22 +474,22 @@ func Stack(b Slice, all bool) (n int) {
 }
 
 static void
-saveg(byte *pc, byte *sp, G *gp, TRecord *r)
+saveg(uintptr pc, uintptr sp, G *gp, TRecord *r)
 {
 	int32 n;
 	
-	n = runtime·gentraceback(pc, sp, 0, gp, 0, r->stk, nelem(r->stk), nil, nil);
+	n = runtime·gentraceback((uintptr)pc, (uintptr)sp, 0, gp, 0, r->stk, nelem(r->stk), nil, nil);
 	if(n < nelem(r->stk))
 		r->stk[n] = 0;
 }
 
 func GoroutineProfile(b Slice) (n int, ok bool) {
-	byte *pc, *sp;
+	uintptr pc, sp;
 	TRecord *r;
 	G *gp;
 	
 	sp = runtime·getcallersp(&b);
-	pc = runtime·getcallerpc(&b);
+	pc = (uintptr)runtime·getcallerpc(&b);
 	
 	ok = false;
 	n = runtime·gcount();
@@ -506,7 +506,7 @@ func GoroutineProfile(b Slice) (n int, ok bool) {
 			for(gp = runtime·allg; gp != nil; gp = gp->alllink) {
 				if(gp == g || gp->status == Gdead)
 					continue;
-				saveg(gp->sched.pc, (byte*)gp->sched.sp, gp, r++);
+				saveg(gp->sched.pc, gp->sched.sp, gp, r++);
 			}
 		}
 	

src/pkg/runtime/panic.c

@@ -214,7 +214,7 @@ runtime·panic(Eface e)
 	p = runtime·mal(sizeof *p);
 	p->arg = e;
 	p->link = g->panic;
-	p->stackbase = (byte*)g->stackbase;
+	p->stackbase = g->stackbase;
 	g->panic = p;
 
 	for(;;) {
@@ -254,11 +254,11 @@ static void
 recovery(G *gp)
 {
 	void *argp;
-	void *pc;
+	uintptr pc;
 	
 	// Info about defer passed in G struct.
 	argp = (void*)gp->sigcode0;
-	pc = (void*)gp->sigcode1;
+	pc = (uintptr)gp->sigcode1;
 
 	// Unwind to the stack frame with d's arguments in it.
 	runtime·unwindstack(gp, argp);
@@ -292,12 +292,12 @@ runtime·unwindstack(G *gp, byte *sp)
 	if(g == gp)
 		runtime·throw("unwindstack on self");
 
-	while((top = (Stktop*)gp->stackbase) != nil && top->stackbase != nil) {
+	while((top = (Stktop*)gp->stackbase) != 0 && top->stackbase != 0) {
 		stk = (byte*)gp->stackguard - StackGuard;
 		if(stk <= sp && sp < (byte*)gp->stackbase)
 			break;
-		gp->stackbase = (uintptr)top->stackbase;
-		gp->stackguard = (uintptr)top->stackguard;
+		gp->stackbase = top->stackbase;
+		gp->stackguard = top->stackguard;
 		gp->stackguard0 = gp->stackguard;
 		if(top->free != 0)
 			runtime·stackfree(stk, top->free);
@@ -413,7 +413,7 @@ runtime·dopanic(int32 unused)
 		if(g != m->g0) {
 			runtime·printf("\n");
 			runtime·goroutineheader(g);
-			runtime·traceback(runtime·getcallerpc(&unused), runtime·getcallersp(&unused), 0, g);
+			runtime·traceback((uintptr)runtime·getcallerpc(&unused), (uintptr)runtime·getcallersp(&unused), 0, g);
 		}
 		if(!didothers) {
 			didothers = true;

src/pkg/runtime/proc.c

@@ -241,7 +241,7 @@ runtime·tracebackothers(G *me)
 			continue;
 		runtime·printf("\n");
 		runtime·goroutineheader(gp);
-		runtime·traceback(gp->sched.pc, (byte*)gp->sched.sp, 0, gp);
+		runtime·traceback(gp->sched.pc, gp->sched.sp, 0, gp);
 	}
 }
 
@@ -473,7 +473,7 @@ runtime·mstart(void)
 	// Once we call schedule we're never coming back,
 	// so other calls can reuse this stack space.
 	runtime·gosave(&m->g0->sched);
-	m->g0->sched.pc = (void*)-1;  // make sure it is never used
+	m->g0->sched.pc = (uintptr)-1;  // make sure it is never used
 	m->g0->stackguard = m->g0->stackguard0;  // cgo sets only stackguard0, copy it to stackguard
 	m->seh = &seh;
 	runtime·asminit();
@@ -651,7 +651,7 @@ runtime·newextram(void)
 	// the goroutine stack ends.
 	mp = runtime·allocm(nil);
 	gp = runtime·malg(4096);
-	gp->sched.pc = (void*)runtime·goexit;
+	gp->sched.pc = (uintptr)runtime·goexit;
 	gp->sched.sp = gp->stackbase;
 	gp->sched.g = gp;
 	gp->status = Gsyscall;
@@ -997,7 +997,7 @@ execute(G *gp)
 	if(m->profilehz != hz)
 		runtime·resetcpuprofiler(hz);
 
-	if(gp->sched.pc == (byte*)runtime·goexit)  // kickoff
+	if(gp->sched.pc == (uintptr)runtime·goexit)  // kickoff
 		runtime·gogocallfn(&gp->sched, gp->fnstart);
 	runtime·gogo(&gp->sched, 0);
 }
@@ -1281,7 +1281,7 @@ void
 
 	// Leave SP around for gc and traceback.
 	g->sched.sp = (uintptr)runtime·getcallersp(&dummy);
-	g->sched.pc = runtime·getcallerpc(&dummy);
+	g->sched.pc = (uintptr)runtime·getcallerpc(&dummy);
 	g->sched.g = g;
 	g->gcsp = g->sched.sp;
 	g->gcpc = g->sched.pc;
@@ -1330,8 +1330,8 @@ void
 		runtime·setprof(false);
 
 	// Leave SP around for gc and traceback.
-	g->sched.sp = (uintptr)runtime·getcallersp(&dummy);
-	g->sched.pc = runtime·getcallerpc(&dummy);
+	g->sched.sp = runtime·getcallersp(&dummy);
+	g->sched.pc = (uintptr)runtime·getcallerpc(&dummy);
 	g->sched.g = g;
 	g->gcsp = g->sched.sp;
 	g->gcpc = g->sched.pc;
@@ -1548,14 +1548,14 @@ runtime·newproc1(FuncVal *fn, byte *argp, int32 narg, int32 nret, void *callerp
 	}
 
 	newg->sched.sp = (uintptr)sp;
-	newg->sched.pc = (byte*)runtime·goexit;
+	newg->sched.pc = (uintptr)runtime·goexit;
 	newg->sched.g = newg;
 	newg->fnstart = fn;
 	newg->gopc = (uintptr)callerpc;
 	newg->status = Grunnable;
 	newg->goid = runtime·xadd64(&runtime·sched.goidgen, 1);
 	if(raceenabled)
-		newg->racectx = runtime·racegostart(callerpc);
+		newg->racectx = runtime·racegostart((void*)callerpc);
 	runqput(m->p, newg);
 
 	if(runtime·atomicload(&runtime·sched.npidle) != 0 && runtime·atomicload(&runtime·sched.nmspinning) == 0 && fn->fn != runtime·main)  // TODO: fast atomic
@@ -1802,7 +1802,7 @@ runtime·sigprof(uint8 *pc, uint8 *sp, uint8 *lr, G *gp)
 		runtime·unlock(&prof);
 		return;
 	}
-	n = runtime·gentraceback(pc, sp, lr, gp, 0, prof.pcbuf, nelem(prof.pcbuf), nil, nil);
+	n = runtime·gentraceback((uintptr)pc, (uintptr)sp, (uintptr)lr, gp, 0, prof.pcbuf, nelem(prof.pcbuf), nil, nil);
 	if(n > 0)
 		prof.fn(prof.pcbuf, n);
 	runtime·unlock(&prof);

src/pkg/runtime/runtime.h

@@ -211,7 +211,7 @@ struct	Gobuf
 {
 	// The offsets of these fields are known to (hard-coded in) libmach.
 	uintptr	sp;
-	byte*	pc;
+	uintptr	pc;
 	G*	g;
 };
 struct	GCStats
@@ -234,7 +234,7 @@ struct	G
 	Gobuf	sched;
 	uintptr	gcstack;		// if status==Gsyscall, gcstack = stackbase to use during gc
 	uintptr	gcsp;		// if status==Gsyscall, gcsp = sched.sp to use during gc
-	byte*	gcpc;		// if status==Gsyscall, gcpc = sched.pc to use during gc
+	uintptr	gcpc;		// if status==Gsyscall, gcpc = sched.pc to use during gc
 	uintptr	gcguard;		// if status==Gsyscall, gcguard = stackguard to use during gc
 	uintptr	stackguard;	// same as stackguard0, but not set to StackPreempt
 	uintptr	stack0;
@@ -375,8 +375,8 @@ enum
 struct	Stktop
 {
 	// The offsets of these fields are known to (hard-coded in) libmach.
-	uint8*	stackguard;
-	uint8*	stackbase;
+	uintptr	stackguard;
+	uintptr	stackbase;
 	Gobuf	gobuf;
 	uint32	argsize;
 
@@ -646,11 +646,32 @@ struct DeferChunk
 struct Panic
 {
 	Eface	arg;		// argument to panic
-	byte*	stackbase;	// g->stackbase in panic
+	uintptr	stackbase;	// g->stackbase in panic
 	Panic*	link;		// link to earlier panic
 	bool	recovered;	// whether this panic is over
 };
 
+/*
+ * stack traces
+ */
+typedef struct Stkframe Stkframe;
+struct Stkframe
+{
+	Func*	fn;	// function being run
+	uintptr	pc;	// program counter within fn
+	uintptr	lr;	// program counter at caller aka link register
+	uintptr	sp;	// stack pointer at pc
+	uintptr	fp;	// stack pointer at caller aka frame pointer
+	byte*	argp;	// pointer to function arguments
+	uintptr	arglen;	// number of bytes at argp
+	byte*	varp;	// pointer to local variables
+	uintptr	varlen;	// number of bytes at varp
+};
+
+int32	runtime·gentraceback(uintptr, uintptr, uintptr, G*, int32, uintptr*, int32, void(*)(Stkframe*, void*), void*);
+void	runtime·traceback(uintptr pc, uintptr sp, uintptr lr, G* gp);
+void	runtime·tracebackothers(G*);
+
 /*
  * external data
  */
@@ -718,8 +739,6 @@ void	runtime·sigenable(uint32 sig);
 void	runtime·sigdisable(uint32 sig);
 int32	runtime·gotraceback(bool *crash);
 void	runtime·goroutineheader(G*);
-void	runtime·traceback(uint8 *pc, uint8 *sp, uint8 *lr, G* gp);
-void	runtime·tracebackothers(G*);
 int32	runtime·open(int8*, int32, int32);
 int32	runtime·read(int32, void*, int32);
 int32	runtime·write(int32, void*, int32);
@@ -770,7 +789,7 @@ void*	runtime·malloc(uintptr size);
 void	runtime·free(void *v);
 bool	runtime·addfinalizer(void*, FuncVal *fn, uintptr);
 void	runtime·runpanic(Panic*);
-void*	runtime·getcallersp(void*);
+uintptr	runtime·getcallersp(void*);
 int32	runtime·mcount(void);
 int32	runtime·gcount(void);
 void	runtime·mcall(void(*)(G*));
@@ -792,7 +811,6 @@ void	runtime·exitsyscall(void);
 G*	runtime·newproc1(FuncVal*, byte*, int32, int32, void*);
 bool	runtime·sigsend(int32 sig);
 int32	runtime·callers(int32, uintptr*, int32);
-int32	runtime·gentraceback(byte*, byte*, byte*, G*, int32, uintptr*, int32, void (*)(Func*, byte*, byte*, void*), void*);
 int64	runtime·nanotime(void);
 void	runtime·dopanic(int32);
 void	runtime·startpanic(void);
@@ -813,6 +831,7 @@ int32	runtime·netpollopen(uintptr, PollDesc*);
 int32   runtime·netpollclose(uintptr);
 void	runtime·netpollready(G**, PollDesc*, int32);
 void	runtime·crash(void);
+void	_rt0_go(void);
 
 #pragma	varargck	argpos	runtime·printf	1
 #pragma	varargck	type	"c"	int32

src/pkg/runtime/signal_386.c

@@ -111,7 +111,7 @@ Throw:
 	runtime·printf("\n");
 
 	if(runtime·gotraceback(&crash)){
-		runtime·traceback((void*)SIG_EIP(info, ctxt), (void*)SIG_ESP(info, ctxt), 0, gp);
+		runtime·traceback(SIG_EIP(info, ctxt), SIG_ESP(info, ctxt), 0, gp);
 		runtime·tracebackothers(gp);
 		runtime·dumpregs(info, ctxt);
 	}

src/pkg/runtime/signal_amd64.c

@@ -121,7 +121,7 @@ Throw:
 	runtime·printf("\n");
 
 	if(runtime·gotraceback(&crash)){
-		runtime·traceback((void*)SIG_RIP(info, ctxt), (void*)SIG_RSP(info, ctxt), 0, gp);
+		runtime·traceback(SIG_RIP(info, ctxt), SIG_RSP(info, ctxt), 0, gp);
 		runtime·tracebackothers(gp);
 		runtime·dumpregs(info, ctxt);
 	}

src/pkg/runtime/signal_arm.c

@@ -111,7 +111,7 @@ Throw:
 	runtime·printf("\n");
 
 	if(runtime·gotraceback(&crash)){
-		runtime·traceback((void*)SIG_PC(info, ctxt), (void*)SIG_SP(info, ctxt), (void*)SIG_LR(info, ctxt), gp);
+		runtime·traceback(SIG_PC(info, ctxt), SIG_SP(info, ctxt), SIG_LR(info, ctxt), gp);
 		runtime·tracebackothers(gp);
 		runtime·printf("\n");
 		runtime·dumpregs(info, ctxt);

src/pkg/runtime/stack.c

@@ -155,8 +155,8 @@ runtime·oldstack(void)
 	USED(goid);
 
 	label = top->gobuf;
-	gp->stackbase = (uintptr)top->stackbase;
-	gp->stackguard = (uintptr)top->stackguard;
+	gp->stackbase = top->stackbase;
+	gp->stackguard = top->stackguard;
 	gp->stackguard0 = gp->stackguard;
 	if(top->free != 0)
 		runtime·stackfree(old, top->free);
@@ -176,7 +176,8 @@ runtime·newstack(void)
 {
 	int32 framesize, minalloc, argsize;
 	Stktop *top;
-	byte *stk, *sp;
+	byte *stk;
+	uintptr sp;
 	uintptr *src, *dst, *dstend;
 	G *gp;
 	Gobuf label;
@@ -234,14 +235,14 @@ runtime·newstack(void)
 			framesize, argsize, m->morepc, m->moreargp, m->morebuf.pc, m->morebuf.sp, top, gp->stackbase);
 	}
 
-	top->stackbase = (byte*)gp->stackbase;
-	top->stackguard = (byte*)gp->stackguard;
+	top->stackbase = gp->stackbase;
+	top->stackguard = gp->stackguard;
 	top->gobuf = m->morebuf;
 	top->argp = m->moreargp;
 	top->argsize = argsize;
 	top->free = free;
 	m->moreargp = nil;
-	m->morebuf.pc = nil;
+	m->morebuf.pc = (uintptr)nil;
 	m->morebuf.sp = (uintptr)nil;
 
 	// copy flag from panic
@@ -252,7 +253,7 @@ runtime·newstack(void)
 	gp->stackguard = (uintptr)stk + StackGuard;
 	gp->stackguard0 = gp->stackguard;
 
-	sp = (byte*)top;
+	sp = (uintptr)top;
 	if(argsize > 0) {
 		sp -= argsize;
 		dst = (uintptr*)sp;
@@ -269,8 +270,8 @@ runtime·newstack(void)
 
 	// Continue as if lessstack had just called m->morepc
 	// (the PC that decided to grow the stack).
-	label.sp = (uintptr)sp;
-	label.pc = (byte*)runtime·lessstack;
+	label.sp = sp;
+	label.pc = (uintptr)runtime·lessstack;
 	label.g = m->curg;
 	if(reflectcall)
 		runtime·gogocallfn(&label, (FuncVal*)m->morepc);