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

[Jiangli Zhou]

Hi Serguei,

Thanks for the review! You are right about line 1312. I was a little 
paranoid about uninitialized 'current_mh', so I put the initialization 
code there be consistent with the initialization of the 
''current_method' and 'current_bci' variables.

Thanks!
Jiangli

On 01/07/2013 04:25 PM, serguei.spitsyn at oracle.com wrote:
> 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/f70d0528/attachment.html