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

[Daniel D. Daugherty]

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?

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
>>>>
>>>
>>
>