RFR 8146984: SIGBUS: bool Method::has_method_vptr(const void*)+0xc

[Coleen Phillimore]

Thanks Dan!

On 2/1/16 12:21 PM, Daniel D. Daugherty wrote:
> On 1/30/16 7:39 AM, Coleen Phillimore wrote:
>>
>> I've moved the SafeFetch to has_method_vptr as suggested and retested.
>>
>> http://cr.openjdk.java.net/~coleenp/8146984.02/webrev/
>
> src/share/vm/oops/method.cpp
>     (old) L2114:     return has_method_vptr((const void*)this);
>     (new) L2120:       return has_method_vptr(this);
>         Just curious. I don't see anything that explains why the
>         cast is no longer needed (no type changes). Was this
>         simply cleaning up an unnecessary cast?

The cast is unnecessary.  I didn't add it back when I added the call to 
has_method_vptr back.

thanks,
Coleen

>
> Thumbs up.
>
> Dan
>
>
>>
>> Thanks,
>> Coleen
>>
>> On 1/29/16 11:58 AM, Coleen Phillimore wrote:
>>>
>>>
>>> On 1/29/16 11:02 AM, Daniel D. Daugherty wrote:
>>>> On 1/29/16 5:46 AM, Coleen Phillimore wrote:
>>>>> Summary: Add address check and use SafeFetchN for Method* vptr 
>>>>> access when Method* may be bad pointer.
>>>>>
>>>>> Tested with RBT and failing test case (reproduced 1 in 100 times) 
>>>>> with fatal in the 'return's in the change to verify.
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8146984/
>>>>
>>>> This one caught my eye because it has to do with sampling...
>>>
>>> I should mention sampling in all my RFRs then!
>>>>
>>>> src/share/vm/oops/method.cpp
>>>>     The old code checked "!is_metaspace_object()" and used
>>>>     has_method_vptr((const void*)this).
>>>>
>>>>     The new code skips the "!is_metaspace_object()" check even 
>>>> after sanity
>>>>     checking the pointer, but you don't really explain why that's OK.
>>>
>>> is_metaspace_object is a very expensive check.   It has to traverse 
>>> all the metaspace mmap chunks.   The new code is more robust in that 
>>> it sanity checks the pointer first but uses Safefetch to get the vptr.
>>>
>>>
>>>>
>>>>     The new code also picks up parts of Method::has_method_vptr() 
>>>> which
>>>>     makes me wonder if that's the right place for the fix. Won't other
>>>>     callers to Method::has_method_vptr() be subject to the same 
>>>> crashing
>>>>     mode? Or was the crashing mode only due to the 
>>>> "!is_metaspace_object()"
>>>>     check...
>>>
>>> I should have moved the SafeFetch in to the has_method_vptr. I can't 
>>> remember why I copied it now.  It crashed because the pointer was in 
>>> metaspace (is_metaspace_object returned true) but wasn't aligned, 
>>> but the pointer could come from anywhere.
>>>
>>> Thanks,   I'll test out this fix and resend it.
>>> Coleen
>>>
>>>>
>>>> Dan
>>>>
>>>>
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8146984
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>
>>>
>>
>