Rickard, The comments on line 221-222 is are a bit confusion in association with the conditional and the follow-up assert on line 225. The comment outlines the layout and invariant for nmethods, but in the conditional (sender_blob->frame_size() <= 0), there is an assert that sender_blob is NOT an nmethod? This is a bit unclear. Also, the sentence is weird:
"if the frame size is 0 something (or less) is bad because every nmethod..." Also, do you need that particular assert there? The same logic comes again in non-asserting code at line 234. Maybe just rework/remove the assert and weave in the real logic for that check? Not a Reviewer. Cheers Markus -----Original Message----- From: Rickard Bäckman Sent: den 5 mars 2013 08:08 To: serviceability-dev serviceability-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR(S): 8008357: [sampling] assert(sender_blob->is_runtime_stub() || sender_blob->is_nmethod()) failed: Impossible call chain Anyone have time to look at this? Thanks /R On Mar 1, 2013, at 10:27 AM, Rickard Bäckman wrote: > Hi all, > > here comes another update to frame.safe_for_sender. > If the PC at a place where the stack doesn't match the _frame_size we > sometimes read an invalid return PC. > In this case we read one that pointed into the Safepoint blob. > > We now pretty much guarded for all kind of blobs in safe_for_sender, > so I've changed the method to not assert in the end but to do it the > same was as the frame_sparc.cpp did it. Everything that is not a nmethod at > the end of the method is not safe. > > I've also changed another check for a frame_size == 0 to frame_size <= 0 to > make it somewhat more safer. > > The bug: http://bugs.sun.com/view_bug.do?bug_id=8008357 > Webrev: http://cr.openjdk.java.net/~rbackman/8008357/ > > This webrev is for HS24. > > Thanks > /R
