RFR (XXS): 8017571: JSR292: JVM crashing on assert "cast to instanceKlass" while producing MethodHandle for array methods with MethodHandle.findVirtual

[Vladimir Kozlov]

Good.

Thanks,
Vladimir

On 7/2/13 4:23 PM, Christian Thalinger wrote:
>
> On Jun 26, 2013, at 11:25 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
>> On 6/26/13 11:19 AM, Christian Thalinger wrote:
>>>
>>> On Jun 26, 2013, at 11:02 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>
>>>> On 6/26/13 10:42 AM, Christian Thalinger wrote:
>>>>>
>>>>> On Jun 26, 2013, at 10:02 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>>
>>>>>> You can code it simpler (you don't need to check all types variations):
>>>>>
>>>>> I could but I wanted to keep the MemberName class around for debugging purposes.  I could get rid of the type array:
>>>>
>>>> What debugging? C++ will optimize it anyway in product build. And with debug build you can call java_lang_invoke_MemberName::clazz(mname()) in debugger to get original klass.
>>>
>>> Not always.  Sometimes the debugger just barfs on you (for whatever reason) and sometimes you can't call methods (e.g. when you are out of stack space).  I had all this recently.
>>
>> You can simple have next for that:
>>
>> !     Klass* reference_klass = java_lang_Class::as_Klass(java_lang_invoke_MemberName::clazz(mname()));
>> +     DEBUG_ONLY( Klass* mname_klass = reference_klass; )
>
> I've used your earlier suggestion and updated the webrev:
>
> http://cr.openjdk.java.net/~twisti/8017571/webrev/
>
> -- Chris
>
>>
>> Vladimir
>>
>>>
>>> -- Chris
>>>
>>>>
>>>> Vladimir
>>>>
>>>>>
>>>>> +           reference_klass = NULL;
>>>>>
>>>>> though.  Sometimes I'm torn between simpler, better to read and less code.
>>>>>
>>>>> -- Chris
>>>>>
>>>>>>
>>>>>> !     Klass* reference_klass = java_lang_Class::as_Klass(java_lang_invoke_MemberName::clazz(mname()));
>>>>>> +     if (reference_klass != NULL && reference_klass->oop_is_objArray()) {
>>>>>> +       reference_klass = ObjArrayKlass::cast(reference_klass)->bottom_klass();
>>>>>> +     }
>>>>>> +
>>>>>> +     // Reflection::verify_class_access can only handle instance classes.
>>>>>> +     if (reference_klass != NULL && reference_klass->oop_is_instance()) {
>>>>>>         // Emulate LinkResolver::check_klass_accessability.
>>>>>>
>>>>>> Vladimir
>>>>>>
>>>>>> On 6/26/13 9:15 AM, Christian Thalinger wrote:
>>>>>>>
>>>>>>> On Jun 25, 2013, at 6:19 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>>>>
>>>>>>>> How you got ObjArrayKlass here? Also bottom_klass() could be TypeArrayKlass.
>>>>>>>
>>>>>>> You are right; it doesn't work.  Reflection::verify_class_access can only handle instance classes.  Here is a new webrev that checks for instance classes before doing the check:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~twisti/8017571/webrev/
>>>>>>>
>>>>>>> I noticed that is_public is a Klass method so we can remove the InstanceKlass cast.
>>>>>>>
>>>>>>> -- Chris
>>>>>>>
>>>>>>>>
>>>>>>>> Vladimir
>>>>>>>>
>>>>>>>> On 6/25/13 6:03 PM, Christian Thalinger wrote:
>>>>>>>>> http://cr.openjdk.java.net/~twisti/8017571/webrev/
>>>>>>>>>
>>>>>>>>> 8017571: JSR292: JVM crashing on assert "cast to instanceKlass" while producing MethodHandle for array methods with MethodHandle.findVirtual
>>>>>>>>> Reviewed-by:
>>>>>>>>>
>>>>>>>>> Verification code in MHN_resolve_Mem calls Reflection::verify_class_access with MemberName.clazz as new_class.  If the class happens to be an array class we hit an assert.
>>>>>>>>>
>>>>>>>>> The fix is to check for array types and use the bottom type for these.
>>>>>>>>>
>>>>>>>>> src/share/vm/prims/methodHandles.cpp
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>