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>

diff --git a/src/runtime/panic.go b/src/runtime/panic.go
index c51948bd18e5805d745596237cf3a76adfd0933b..e1477e248611b9cd2e8fed01d3e22da178882fb7 100644 (file)
--- a/src/runtime/panic.go
+++ b/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
+}

diff --git a/src/runtime/signal_386.go b/src/runtime/signal_386.go
index 416c7c2715dc630892231d46dc44dae503702df0..143deb9de78dbdf2dcdaad20c76eae286a4beed3 100644 (file)
--- a/src/runtime/signal_386.go
+++ b/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
-       // but we do recognize the top pointer on the stack as code,
-       // 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 shouldPushSigpanic(gp, pc, *(*uintptr)(unsafe.Pointer(sp))) {
+               // Make it look like the faulting PC called sigpanic.
                if sys.RegSize > sys.PtrSize {
                        sp -= sys.PtrSize
                        *(*uintptr)(unsafe.Pointer(sp)) = 0

diff --git a/src/runtime/signal_amd64x.go b/src/runtime/signal_amd64x.go
index fad5fc0f8aed1993a5964b172f82b87a48ccfe18..823fd295ae7ce2b13b78167fc0923f745ff56fd2 100644 (file)
--- a/src/runtime/signal_amd64x.go
+++ b/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
-       // but we do recognize the top pointer on the stack as code,
-       // 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 shouldPushSigpanic(gp, pc, *(*uintptr)(unsafe.Pointer(sp))) {
+               // Make it look the like faulting PC called sigpanic.
                if sys.RegSize > sys.PtrSize {
                        sp -= sys.PtrSize
                        *(*uintptr)(unsafe.Pointer(sp)) = 0

diff --git a/src/runtime/signal_arm.go b/src/runtime/signal_arm.go
index d00b225ef65e3110eccdd664caaba11e9c34ba01..bb597c5608e845f5a0c8c4904efb75ac254a0096 100644 (file)
--- a/src/runtime/signal_arm.go
+++ b/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
-       // but we do recognize the link register as code,
-       // 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 {
+       if shouldPushSigpanic(gp, pc, uintptr(c.lr())) {
+               // Make it look the like faulting PC called sigpanic.
                c.set_lr(uint32(pc))
        }
 

diff --git a/src/runtime/signal_arm64.go b/src/runtime/signal_arm64.go
index 1db052538c139841a429ad5e8ee81c942a9b8296..2d4c9e8129b4c6cbcb1a1fa82c651d9efc376ab2 100644 (file)
--- a/src/runtime/signal_arm64.go
+++ b/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
-       // but we do recognize the link register as code,
-       // 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 {
+       if shouldPushSigpanic(gp, pc, uintptr(c.lr())) {
+               // Make it look the like faulting PC called sigpanic.
                c.set_lr(uint64(pc))
        }
 

diff --git a/src/runtime/signal_linux_s390x.go b/src/runtime/signal_linux_s390x.go
index a31f4364117d7e5167006bca401feb8a6bf7d480..6892f63b9f52d60141d66b866fe51255726394e6 100644 (file)
--- a/src/runtime/signal_linux_s390x.go
+++ b/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
-       // but we do recognize the link register as code,
-       // 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 {
+       if shouldPushSigpanic(gp, pc, uintptr(c.link())) {
+               // Make it look the like faulting PC called sigpanic.
                c.set_link(uint64(pc))
        }
 

diff --git a/src/runtime/signal_mips64x.go b/src/runtime/signal_mips64x.go
index 35b356c2fb104ebcd1ee5d134922c327abbdc849..1b9684295e53128737007ce1a5a782026e154dbf 100644 (file)
--- a/src/runtime/signal_mips64x.go
+++ b/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
-       // but we do recognize the link register as code,
-       // 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 {
+       if shouldPushSigpanic(gp, pc, uintptr(c.link())) {
+               // Make it look the like faulting PC called sigpanic.
                c.set_link(uint64(pc))
        }
 

diff --git a/src/runtime/signal_mipsx.go b/src/runtime/signal_mipsx.go
index 1c545ec8cb9ab3917c356349c1b1d90293828fea..e223c284024ba51d415129d9b25ba5cd62942e51 100644 (file)
--- a/src/runtime/signal_mipsx.go
+++ b/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
-       // but we do recognize the link register as code,
-       // 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 {
+       if shouldPushSigpanic(gp, pc, uintptr(c.link())) {
+               // Make it look the like faulting PC called sigpanic.
                c.set_link(uint32(pc))
        }
 

diff --git a/src/runtime/signal_ppc64x.go b/src/runtime/signal_ppc64x.go
index 03cb996f3fde4a8a0e76f8a756087c31c4050ad0..5a1a5cae60d644a17d54eeb4fe0b8c73bd4022bf 100644 (file)
--- a/src/runtime/signal_ppc64x.go
+++ b/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
-       // but we do recognize the link register as code,
-       // 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 {
+       if shouldPushSigpanic(gp, pc, uintptr(c.link())) {
+               // Make it look the like faulting PC called sigpanic.
                c.set_link(uint64(pc))
        }