Request for review: 8001341/8004223: SIGSEGV in methodOopDesc::fast_exception_handler_bci_for(KlassHandle, int, Thread*)+0x3e9

[serguei.spitsyn at oracle.com]

Hi Jiangli,

It looks good.
The only comment is that the following initialization does not look 
necessary:

src/share/vm/prims/jvmtiExport.cpp:

   1312         methodHandle current_mh = methodHandle(thread, current_method);



Thanks,
Serguei


On 1/7/13 3:25 PM, Jiangli Zhou wrote:
> Hi Chris,
>
> I ended up with adding the following comments in 
> JvmtiExport::post_exception_throw() in 
> src/share/vm/prims/jvmtiExport.cpp, since that seems be a more 
> appropriate place.
>
>         // A GC may occur during the 
> Method::fast_exception_handler_bci_for()
>         // call below if it needs to load the constraint class. Using a
>         // methodHandle to keep the 'current_method' from being 
> deallocated
>         // if GC happens.
>
> The updated jdk8 webrev is 
> http://cr.openjdk.java.net/~jiangli/8001341/webrev_jdk8.01/.
>
> Thanks,
> Jiangli
>
> On 01/07/2013 02:31 PM, Jiangli Zhou wrote:
>> Hi Chris,
>>
>> Thanks for the review and suggestion! Will add a comment in 
>> src/share/vm/oops/method.hpp.
>>
>> Thanks,
>> Jiangli
>>
>>
>> On 01/07/2013 02:34 PM, Christian Thalinger wrote:
>>> On Jan 7, 2013, at 12:30 PM, Jiangli Zhou<jiangli.zhou at oracle.com>  
>>> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please review the fix for 8001341/8004223: SIGSEGV in 
>>>> methodOopDesc::fast_exception_handler_bci_for(KlassHandle,int,Thread*)+0x3e9.
>>>>
>>>> hs24 webrev: 
>>>> http://cr.openjdk.java.net/~jiangli/8001341/webrev_hs24.00/
>>>> jdk8 webrev: 
>>>> http://cr.openjdk.java.net/~jiangli/8001341/webrev_jdk8.00/
>>>>
>>>> The SIGSEGV was caused by an unhandled methodOop in 
>>>> methodOopDesc::fast_exception_handler_bci_for() and 
>>>> JvmtiExport::post_exception_throw(). It's only reproducible on hs24 
>>>> as oops in the perm-gen could be moved by the GC. The bug was not 
>>>> reported on jdk8. On jdk8, Method* doesn't move. However the fix is 
>>>> still needed on jdk8 since the Method* in 
>>>> JvmtiExport::post_exception_throw() could be redefined after the 
>>>> methodOopDesc::fast_exception_handler_bci_for() call. The handle 
>>>> will keep it from being deallocated.
>>> I was just going to say it's not required for HS25 but the 
>>> redefinition is a valid point.  Although imperceptible.  Could we 
>>> add a comment in:
>>>
>>> src/share/vm/oops/method.hpp
>>>
>>> to prevent someone undoing this change?
>>>
>>> -- Chris
>>>
>>>> Tested with -XX:+CheckUnhandledOops on solaris (x64) using 
>>>> vm.quick.testlist. Tested with jprt.
>>>>
>>>> Thanks,
>>>> Jiangli
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130107/551608c9/attachment.html