Hi, Rickard

There are several methods already in Thread class which do similar address cheacks: on_local_stackO, is_in_stack(). It would be nice to have new check (at least part of it) at the same place.
What about closed changes for entry_frame_call_wrapper_addr()?

Thanks,
Vladimir

On 7/14/13 11:51 PM, Rickard Bäckman wrote:
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




Reply via email to