[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    RE: RFR(S): 8028412 - AsyncGetCallTrace() is broken on x86 in JDK7u40
From:       Markus Gronlund <markus.gronlund () oracle ! com>
Date:       2013-11-28 10:42:53
Message-ID: be61388f-7a7e-4d66-840b-9e183a4edf23 () default
[Download RAW message or body]

Hi Vladimir,

Thanks for taking a look.

I have updated the bug with a link to 8005849, thanks.

"Is it possible to prepare a jtreg test which will use AsynchGetCallTrace and catch \
such situations in a future?"

I agree that having a way to check for regression here would be ideal.

I also realize that creating reliable regression tests for this part of the code is a \
non-trivial undertaking. For example, the code showing this particular regression \
needs to be a call tree with intertwined Java/JNI/native code. For one, a regression \
test for AsyncGetCallTrace needs, besides logic for AsyncGetCallTrace thread \
suspension itself, also involve a target using native code, and this means compiling \
native code etc etc....

I will create a separate bug/enhancement to track investigation efforts as to what \
kind of regression tests can be done here (maybe the native parts can be done in the \
VM itself and then using RegisterNatives() to hook up a test case...hmm...)

Thanks again for reviewing

/Markus


-----Original Message-----
From: Vladimir Kozlov 
Sent: den 27 november 2013 19:04
To: Markus Gronlund; hotspot-compiler-dev@openjdk.java.net; \
                hotspot-runtime-dev@openjdk.java.net; serviceability-dev
Subject: Re: RFR(S): 8028412 - AsyncGetCallTrace() is broken on x86 in JDK7u40

Hi, Markus

I think the bug report should link to 8005849 which introduced this additional check \
and regression.

Is it possible to prepare a jtreg test which will use AsynchGetCallTrace and catch \
such situations in a future?

The fix itself looks good.

Thanks,
Vladimir

On 11/27/13 1:47 AM, Markus Gronlund wrote:
> Greetings,
> 
> Kindly asking for reviews for the following change:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8028412
> 
> Webrev: http://cr.openjdk.java.net/~mgronlun/8028412/webrev01/
> 
> Description:
> 
> AsynchGetCallTrace() uses platform specific code for stack frame traversals.
> 
> On x86, there currently exist a defensive construct that practically 
> disallows stack traversal over entry_frame code stubs, such as
> StubRoutine(1) frames:
> 
> src/cpu/x86/vm/frame_x86.cpp b/src/cpu/x86/vm/frame_x86.cpp:
> 
> bool frame::safe_for_sender(JavaThread* thread)  {
> 
> .
> 
> if (!Interpreter::contains(_pc) && _cb->frame_size() <= 0) {
> //assert(0, "Invalid frame_size");
> 
> return false;
> }
> 
> .
> 
> Since entry frames (such as StubRoutine(1)) have a frame size of 0, 
> the code returns prematurely.
> 
> Fix is to move this frame size check post handling of is_entry_frame(),
> where the code_stubs are processed.
> 
> Testing completed:
> 
> Sun Studio Profiler reproducer testcase
> 
> SpecJBB2005
> 
> Kitchensink
> 
> Thanks
> Markus
> 


[prev in list] [next in list] [prev in thread] [next in thread]