From 0fd427fda70d635a526efc8cf40251718e5a45bf Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Wed, 25 Apr 2018 16:53:04 -0400 Subject: [PATCH] runtime: use entry stack map at function entry Currently, when the runtime looks up the stack map for a frame, it uses frame.continpc - 1 unless continpc is the function entry PC, in which case it uses frame.continpc. As a result, if continpc is the function entry point (which happens for deferred frames), it will actually look up the stack map *following* the first instruction. I think, though I am not positive, that this is always okay today because the first instruction of a function can never change the stack map. It's usually not a CALL, so it doesn't have PCDATA. Or, if it is a CALL, it has to have the entry stack map. But we're about to start emitting stack maps at every instruction that changes them, which means the first instruction can have PCDATA (notably, in leaf functions that don't have a prologue). To prepare for this, tweak how the runtime looks up stack map indexes so that if continpc is the function entry point, it directly uses the entry stack map. For #24543. Change-Id: I85aa818041cd26aff416f7b1fba186e9c8ca6568 Reviewed-on: https://go-review.googlesource.com/109349 Reviewed-by: Rick Hudson --- src/runtime/heapdump.go | 3 ++- src/runtime/mbitmap.go | 3 ++- src/runtime/mgcmark.go | 7 ++++++- src/runtime/stack.go | 3 ++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go index b255cbbae3..2b8937834c 100644 --- a/src/runtime/heapdump.go +++ b/src/runtime/heapdump.go @@ -248,10 +248,11 @@ func dumpframe(s *stkframe, arg unsafe.Pointer) bool { // Figure out what we can about our stack map pc := s.pc + pcdata := int32(-1) // Use the entry map at function entry if pc != f.entry { pc-- + pcdata = pcdatavalue(f, _PCDATA_StackMapIndex, pc, nil) } - pcdata := pcdatavalue(f, _PCDATA_StackMapIndex, pc, nil) if pcdata == -1 { // We do not have a valid pcdata value but there might be a // stackmap for this function. It is likely that we are looking diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go index 38d994eb5a..69bd0b502b 100644 --- a/src/runtime/mbitmap.go +++ b/src/runtime/mbitmap.go @@ -2004,10 +2004,11 @@ func getgcmask(ep interface{}) (mask []byte) { if targetpc == 0 { return } + pcdata := int32(-1) // Use the entry map at function entry if targetpc != f.entry { targetpc-- + pcdata = pcdatavalue(f, _PCDATA_StackMapIndex, targetpc, nil) } - pcdata := pcdatavalue(f, _PCDATA_StackMapIndex, targetpc, nil) if pcdata == -1 { return } diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go index 270fa6cd32..7be2820909 100644 --- a/src/runtime/mgcmark.go +++ b/src/runtime/mgcmark.go @@ -801,10 +801,15 @@ func scanframeworker(frame *stkframe, cache *pcvalueCache, gcw *gcWork) { if _DebugGC > 1 { print("scanframe ", funcname(f), "\n") } + pcdata := int32(-1) if targetpc != f.entry { + // Back up to the CALL. If we're at the function entry + // point, we want to use the entry map (-1), even if + // the first instruction of the function changes the + // stack map. targetpc-- + pcdata = pcdatavalue(f, _PCDATA_StackMapIndex, targetpc, cache) } - pcdata := pcdatavalue(f, _PCDATA_StackMapIndex, targetpc, cache) if pcdata == -1 { // We do not have a valid pcdata value but there might be a // stackmap for this function. It is likely that we are looking diff --git a/src/runtime/stack.go b/src/runtime/stack.go index 63a286bf59..2d10ac8381 100644 --- a/src/runtime/stack.go +++ b/src/runtime/stack.go @@ -625,10 +625,11 @@ func adjustframe(frame *stkframe, arg unsafe.Pointer) bool { // have full GC info for it (because it is written in asm). return true } + pcdata := int32(-1) // Use the entry map at function entry if targetpc != f.entry { targetpc-- + pcdata = pcdatavalue(f, _PCDATA_StackMapIndex, targetpc, &adjinfo.cache) } - pcdata := pcdatavalue(f, _PCDATA_StackMapIndex, targetpc, &adjinfo.cache) if pcdata == -1 { pcdata = 0 // in prologue }