runtime: better trace for fault due to nil pointer call

R=r
CC=golang-dev
http://codereview.appspot.com/854048

7 files changed, 72 insertions, 20 deletions

diff --git a/src/pkg/runtime/darwin/386/signal.c b/src/pkg/runtime/darwin/386/signal.c
index 65c217b4e..5161796dc 100644
--- a/src/pkg/runtime/darwin/386/signal.c
+++ b/src/pkg/runtime/darwin/386/signal.c
@@ -66,10 +66,18 @@ sighandler(int32 sig, Siginfo *info, void *context)
 		gp->sigcode0 = info->si_code;
 		gp->sigcode1 = (uintptr)info->si_addr;
 
-		sp = (uintptr*)r->esp;
-		*--sp = r->eip;
+		// Only push sigpanic if r->eip != 0.
+		// If r->eip == 0, probably panicked because of a
+		// call to a nil func.  Not pushing that onto sp will
+		// make the trace look like a call to sigpanic instead.
+		// (Otherwise the trace will end at sigpanic and we
+		// won't get to see who faulted.)
+		if(r->eip != 0) {
+			sp = (uintptr*)r->esp;
+			*--sp = r->eip;
+			r->esp = (uintptr)sp;
+		}
 		r->eip = (uintptr)sigpanic;
-		r->esp = (uintptr)sp;
 		return;
 	}
 
diff --git a/src/pkg/runtime/darwin/amd64/signal.c b/src/pkg/runtime/darwin/amd64/signal.c
index 9c4f0dc14..56f02e56d 100644
--- a/src/pkg/runtime/darwin/amd64/signal.c
+++ b/src/pkg/runtime/darwin/amd64/signal.c
@@ -74,11 +74,19 @@ sighandler(int32 sig, Siginfo *info, void *context)
 		gp->sig = sig;
 		gp->sigcode0 = info->si_code;
 		gp->sigcode1 = (uintptr)info->si_addr;
-
-		sp = (uintptr*)r->rsp;
-		*--sp = r->rip;
+		
+		// Only push sigpanic if r->rip != 0.
+		// If r->rip == 0, probably panicked because of a
+		// call to a nil func.  Not pushing that onto sp will
+		// make the trace look like a call to sigpanic instead.
+		// (Otherwise the trace will end at sigpanic and we
+		// won't get to see who faulted.)
+		if(r->rip != 0) {
+			sp = (uintptr*)r->rsp;
+			*--sp = r->rip;
+			r->rsp = (uintptr)sp;
+		}
 		r->rip = (uintptr)sigpanic;
-		r->rsp = (uintptr)sp;
 		return;
 	}
 
diff --git a/src/pkg/runtime/freebsd/386/signal.c b/src/pkg/runtime/freebsd/386/signal.c
index ec8ac3a7d..be2f4ce6f 100644
--- a/src/pkg/runtime/freebsd/386/signal.c
+++ b/src/pkg/runtime/freebsd/386/signal.c
@@ -64,10 +64,18 @@ sighandler(int32 sig, Siginfo* info, void* context)
 		gp->sigcode0 = info->si_code;
 		gp->sigcode1 = (uintptr)info->si_addr;
 
-		sp = (uintptr*)r->mc_esp;
-		*--sp = r->mc_eip;
+		// Only push sigpanic if r->mc_eip != 0.
+		// If r->mc_eip == 0, probably panicked because of a
+		// call to a nil func.  Not pushing that onto sp will
+		// make the trace look like a call to sigpanic instead.
+		// (Otherwise the trace will end at sigpanic and we
+		// won't get to see who faulted.)
+		if(r->mc_eip != 0) {
+			sp = (uintptr*)r->mc_esp;
+			*--sp = r->mc_eip;
+			r->mc_esp = (uintptr)sp;
+		}
 		r->mc_eip = (uintptr)sigpanic;
-		r->mc_esp = (uintptr)sp;
 		return;
 	}
 
diff --git a/src/pkg/runtime/freebsd/amd64/signal.c b/src/pkg/runtime/freebsd/amd64/signal.c
index ba8a5cfdb..b0ac650a3 100644
--- a/src/pkg/runtime/freebsd/amd64/signal.c
+++ b/src/pkg/runtime/freebsd/amd64/signal.c
@@ -72,10 +72,18 @@ sighandler(int32 sig, Siginfo* info, void* context)
 		gp->sigcode0 = info->si_code;
 		gp->sigcode1 = (uintptr)info->si_addr;
 
-		sp = (uintptr*)r->mc_rsp;
-		*--sp = r->mc_rip;
+		// Only push sigpanic if r->mc_rip != 0.
+		// If r->mc_rip == 0, probably panicked because of a
+		// call to a nil func.  Not pushing that onto sp will
+		// make the trace look like a call to sigpanic instead.
+		// (Otherwise the trace will end at sigpanic and we
+		// won't get to see who faulted.)
+		if(r->mc_rip != 0) {
+			sp = (uintptr*)r->mc_rsp;
+			*--sp = r->mc_rip;
+			r->mc_rsp = (uintptr)sp;
+		}
 		r->mc_rip = (uintptr)sigpanic;
-		r->mc_rsp = (uintptr)sp;
 		return;
 	}
 
diff --git a/src/pkg/runtime/linux/386/signal.c b/src/pkg/runtime/linux/386/signal.c
index fed052f63..8c76ec366 100644
--- a/src/pkg/runtime/linux/386/signal.c
+++ b/src/pkg/runtime/linux/386/signal.c
@@ -61,10 +61,18 @@ sighandler(int32 sig, Siginfo* info, void* context)
 		gp->sigcode0 = info->si_code;
 		gp->sigcode1 = ((uintptr*)info)[3];
 
-		sp = (uintptr*)r->esp;
-		*--sp = r->eip;
+		// Only push sigpanic if r->eip != 0.
+		// If r->eip == 0, probably panicked because of a
+		// call to a nil func.  Not pushing that onto sp will
+		// make the trace look like a call to sigpanic instead.
+		// (Otherwise the trace will end at sigpanic and we
+		// won't get to see who faulted.)
+		if(r->eip != 0) {
+			sp = (uintptr*)r->esp;
+			*--sp = r->eip;
+			r->esp = (uintptr)sp;
+		}
 		r->eip = (uintptr)sigpanic;
-		r->esp = (uintptr)sp;
 		return;
 	}
 
diff --git a/src/pkg/runtime/linux/amd64/signal.c b/src/pkg/runtime/linux/amd64/signal.c
index 57cdea132..fbe6599f6 100644
--- a/src/pkg/runtime/linux/amd64/signal.c
+++ b/src/pkg/runtime/linux/amd64/signal.c
@@ -71,10 +71,18 @@ sighandler(int32 sig, Siginfo* info, void* context)
 		gp->sigcode0 = info->si_code;
 		gp->sigcode1 = ((uintptr*)info)[2];
 
-		sp = (uintptr*)r->rsp;
-		*--sp = r->rip;
+		// Only push sigpanic if r->rip != 0.
+		// If r->rip == 0, probably panicked because of a
+		// call to a nil func.  Not pushing that onto sp will
+		// make the trace look like a call to sigpanic instead.
+		// (Otherwise the trace will end at sigpanic and we
+		// won't get to see who faulted.)
+		if(r->rip != 0) {
+			sp = (uintptr*)r->rsp;
+			*--sp = r->rip;
+			r->rsp = (uintptr)sp;
+		}
 		r->rip = (uintptr)sigpanic;
-		r->rsp = (uintptr)sp;
 		return;
 	}
 
diff --git a/src/pkg/runtime/linux/arm/signal.c b/src/pkg/runtime/linux/arm/signal.c
index 6cc4ac9be..4d315cc80 100644
--- a/src/pkg/runtime/linux/arm/signal.c
+++ b/src/pkg/runtime/linux/arm/signal.c
@@ -70,7 +70,11 @@ sighandler(int32 sig, Siginfo *info, void *context)
 
 		// If this is a leaf function, we do smash LR,
 		// but we're not going back there anyway.
-		r->arm_lr = r->arm_pc;
+		// Don't bother smashing if r->arm_pc is 0,
+		// which is probably a call to a nil func: the
+		// old link register is more useful in the stack trace.
+		if(r->arm_pc != 0)
+			r->arm_lr = r->arm_pc;
 		r->arm_pc = (uintptr)sigpanic;
 		return;
 	}