John, thanks for the review. I move the entry_frame_is_first(…) to JavaCallWrapper and named it is_first_frame(). Updated webrev: http://cr.openjdk.java.net/~rbackman/8016131.u1/
Thanks /R On Jul 13, 2013, at 12:15 AM, John Rose wrote: > It's good. > > One comment: The new overloading of aux. function entry_frame_is_first(_) > belongs somewhere else. Since it doesn't really use frame::this, it is > confusing as an overload beside a function that does use frame::this. > Suggest placing it in JavaCallWrapper not frame. > > — John > > On Jul 10, 2013, at 4:59 AM, Rickard Bäckman <rickard.back...@oracle.com> > wrote: > >> Still looking for a Reviewer. >> >> Thanks >> /R >> >> On Jul 8, 2013, at 5:49 PM, Rickard Bäckman wrote: >> >>> Thanks Markus! >>> >>> /R >>> >>> On Jul 8, 2013, at 5:45 PM, Markus Grönlund wrote: >>> >>>> Rickard, >>>> >>>> I think it looks good (not a Reviewer). >>>> >>>> Cheers >>>> Markus >>>> >>>> -----Original Message----- >>>> From: Rickard Bäckman >>>> Sent: den 4 juli 2013 11:30 >>>> To: serviceability-dev@openjdk.java.net >>>> serviceability-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net >>>> Subject: RFR(S): 8016131: nsk/sysdict/vm/stress/chain tests crash the VM >>>> in 'entry_frame_is_first()' >>>> >>>> Hi, >>>> >>>> can I please have a couple of reviews for this change? >>>> >>>> The problem in this crash was that we were given an incorrect fp (in this >>>> case 0x0) and had a pc that matched the C -> Java entry frame. The code >>>> then dereferenced fp +- offset. >>>> >>>> This change verifies that the fp +- offset is actually on the stack of the >>>> thread before doing the derefencing. >>>> >>>> Webrev: http://cr.openjdk.java.net/~rbackman/8016131/ >>>> >>>> Thanks >>>> /R >>> >> >
