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

[Jiangli Zhou]

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
>