runtime: refactor test for pushing sigpanic frame

This logic is duplicated in all of the preparePanic functions. Pull it
out into one architecture-independent function.

Change-Id: I7ef4e78e3eda0b7be1a480fb5245fc7424fb2b4e
Reviewed-on: https://go-review.googlesource.com/91255
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

src/runtime/panic.go

@@ -786,3 +786,36 @@ func canpanic(gp *g) bool {
 	}
 	return true
 }
+
+// shouldPushSigpanic returns true if pc should be used as sigpanic's
+// return PC (pushing a frame for the call). Otherwise, it should be
+// left alone so that LR is used as sigpanic's return PC, effectively
+// replacing the top-most frame with sigpanic. This is used by
+// preparePanic.
+func shouldPushSigpanic(gp *g, pc, lr uintptr) bool {
+	if pc == 0 {
+		// Probably a call to a nil func. The old LR is more
+		// useful in the stack trace. Not pushing the frame
+		// 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.)
+		return false
+	}
+	// If we don't recognize the PC as code, but we do recognize
+	// the link register as code, then this assumes the panic was
+	// caused by a call to non-code. In this case, we want to
+	// ignore this call to make unwinding show the context.
+	if findfunc(pc).valid() {
+		// This wasn't a bad call, so use PC as sigpanic's
+		// return PC.
+		return true
+	}
+	if findfunc(lr).valid() {
+		// This was a bad call, but the LR is good, so use the
+		// LR as sigpanic's return PC.
+		return false
+	}
+	// Neither the PC or LR is good. Hopefully pushing a frame
+	// will work.
+	return true
+}

src/runtime/signal_386.go

@@ -56,21 +56,8 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
 	pc := uintptr(c.eip())
 	sp := uintptr(c.esp())
 
-	// If we don't recognize the PC as code
+	if shouldPushSigpanic(gp, pc, *(*uintptr)(unsafe.Pointer(sp))) {
-	// but we do recognize the top pointer on the stack as code,
+		// Make it look like the faulting PC called sigpanic.
-	// then assume this was a call to non-code and treat like
-	// pc == 0, to make unwinding show the context.
-	if pc != 0 && !findfunc(pc).valid() && findfunc(*(*uintptr)(unsafe.Pointer(sp))).valid() {
-		pc = 0
-	}
-
-	// Only push runtime.sigpanic if pc != 0.
-	// If pc == 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 runtime.sigpanic instead.
-	// (Otherwise the trace will end at runtime.sigpanic and we
-	// won't get to see who faulted.)
-	if pc != 0 {
 		if sys.RegSize > sys.PtrSize {
 			sp -= sys.PtrSize
 			*(*uintptr)(unsafe.Pointer(sp)) = 0

src/runtime/signal_amd64x.go

@@ -67,21 +67,8 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
 	pc := uintptr(c.rip())
 	sp := uintptr(c.rsp())
 
-	// If we don't recognize the PC as code
+	if shouldPushSigpanic(gp, pc, *(*uintptr)(unsafe.Pointer(sp))) {
-	// but we do recognize the top pointer on the stack as code,
+		// Make it look the like faulting PC called sigpanic.
-	// then assume this was a call to non-code and treat like
-	// pc == 0, to make unwinding show the context.
-	if pc != 0 && !findfunc(pc).valid() && findfunc(*(*uintptr)(unsafe.Pointer(sp))).valid() {
-		pc = 0
-	}
-
-	// Only push runtime.sigpanic if pc != 0.
-	// If pc == 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 runtime.sigpanic instead.
-	// (Otherwise the trace will end at runtime.sigpanic and we
-	// won't get to see who faulted.)
-	if pc != 0 {
 		if sys.RegSize > sys.PtrSize {
 			sp -= sys.PtrSize
 			*(*uintptr)(unsafe.Pointer(sp)) = 0

src/runtime/signal_arm.go

@@ -53,18 +53,8 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
 
 	pc := gp.sigpc
 
-	// If we don't recognize the PC as code
+	if shouldPushSigpanic(gp, pc, uintptr(c.lr())) {
-	// but we do recognize the link register as code,
+		// Make it look the like faulting PC called sigpanic.
-	// then assume this was a call to non-code and treat like
-	// pc == 0, to make unwinding show the context.
-	if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.lr())).valid() {
-		pc = 0
-	}
-
-	// Don't bother saving PC if it's zero, which is
-	// probably a call to a nil func: the old link register
-	// is more useful in the stack trace.
-	if pc != 0 {
 		c.set_lr(uint32(pc))
 	}
 

src/runtime/signal_arm64.go

@@ -69,18 +69,8 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
 
 	pc := gp.sigpc
 
-	// If we don't recognize the PC as code
+	if shouldPushSigpanic(gp, pc, uintptr(c.lr())) {
-	// but we do recognize the link register as code,
+		// Make it look the like faulting PC called sigpanic.
-	// then assume this was a call to non-code and treat like
-	// pc == 0, to make unwinding show the context.
-	if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.lr())).valid() {
-		pc = 0
-	}
-
-	// Don't bother saving PC if it's zero, which is
-	// probably a call to a nil func: the old link register
-	// is more useful in the stack trace.
-	if pc != 0 {
 		c.set_lr(uint64(pc))
 	}
 

src/runtime/signal_linux_s390x.go

@@ -99,18 +99,8 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
 
 	pc := uintptr(gp.sigpc)
 
-	// If we don't recognize the PC as code
+	if shouldPushSigpanic(gp, pc, uintptr(c.link())) {
-	// but we do recognize the link register as code,
+		// Make it look the like faulting PC called sigpanic.
-	// then assume this was a call to non-code and treat like
-	// pc == 0, to make unwinding show the context.
-	if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.link())).valid() {
-		pc = 0
-	}
-
-	// Don't bother saving PC if it's zero, which is
-	// probably a call to a nil func: the old link register
-	// is more useful in the stack trace.
-	if pc != 0 {
 		c.set_link(uint64(pc))
 	}
 

src/runtime/signal_mips64x.go

@@ -73,18 +73,8 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
 
 	pc := gp.sigpc
 
-	// If we don't recognize the PC as code
+	if shouldPushSigpanic(gp, pc, uintptr(c.link())) {
-	// but we do recognize the link register as code,
+		// Make it look the like faulting PC called sigpanic.
-	// then assume this was a call to non-code and treat like
-	// pc == 0, to make unwinding show the context.
-	if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.link())).valid() {
-		pc = 0
-	}
-
-	// Don't bother saving PC if it's zero, which is
-	// probably a call to a nil func: the old link register
-	// is more useful in the stack trace.
-	if pc != 0 {
 		c.set_link(uint64(pc))
 	}
 

src/runtime/signal_mipsx.go

@@ -70,18 +70,8 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
 
 	pc := gp.sigpc
 
-	// If we don't recognize the PC as code
+	if shouldPushSigpanic(gp, pc, uintptr(c.link())) {
-	// but we do recognize the link register as code,
+		// Make it look the like faulting PC called sigpanic.
-	// then assume this was a call to non-code and treat like
-	// pc == 0, to make unwinding show the context.
-	if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.link())).valid() {
-		pc = 0
-	}
-
-	// Don't bother saving PC if it's zero, which is
-	// probably a call to a nil func: the old link register
-	// is more useful in the stack trace.
-	if pc != 0 {
 		c.set_link(uint32(pc))
 	}
 

src/runtime/signal_ppc64x.go

@@ -74,18 +74,8 @@ func (c *sigctxt) preparePanic(sig uint32, gp *g) {
 
 	pc := gp.sigpc
 
-	// If we don't recognize the PC as code
+	if shouldPushSigpanic(gp, pc, uintptr(c.link())) {
-	// but we do recognize the link register as code,
+		// Make it look the like faulting PC called sigpanic.
-	// then assume this was a call to non-code and treat like
-	// pc == 0, to make unwinding show the context.
-	if pc != 0 && !findfunc(pc).valid() && findfunc(uintptr(c.link())).valid() {
-		pc = 0
-	}
-
-	// Don't bother saving PC if it's zero, which is
-	// probably a call to a nil func: the old link register
-	// is more useful in the stack trace.
-	if pc != 0 {
 		c.set_link(uint64(pc))
 	}