Review request: 8000459: assert(java_lang_String::is_instance(entry)) failure with various mlvm tests

[Jiangli Zhou]

Hi Coleen,

Thanks!

Jiangli

On 10/10/2012 10:50 AM, Coleen Phillimore wrote:
>
> This change looks good.  Thank you for fixing it.
> Coleen
>
> On 10/10/2012 12:44 PM, serguei.spitsyn at oracle.com wrote:
>> Hi Jiangli,
>>
>>
>> On 10/10/12 9:37 AM, Jiangli Zhou wrote:
>>> Hi Serguei,
>>>
>>> Thanks for the review! Please see comments below.
>>>
>>> On 10/09/2012 06:05 PM, serguei.spitsyn at oracle.com wrote:
>>>> Hi Jiangli,
>>>>
>>>> I'm not very familiar with the pseudo strings, and so, just have a 
>>>> question:
>>>>
>>>> *src/share/vm/prims/jvmtiTagMap.cpp*
>>>> 2927             entry = pool->resolved_string_at(i);
>>>> 2928             assert(java_lang_String::is_instance(entry) ||
>>>> 2929                    pool->is_pseudo_string_at(i), "must be 
>>>> string");
>>>> I have a little of doubt the above is correct.
>>>>
>>>> What value the "entry" will have in a case of pseudo string cp entry?
>>>> Just want to understand what constant pool reference will be reported.
>>>> Do you see all related failing tests passed?
>>>>
>>>> Would something like the following be more accurate ?
>>>>    entry = pool->is_pseudo_string_at(i) ?
>>>>                pool->pseudo_string_at(i, cp_to_object_index(i)) :
>>>>                pool->resolved_string_at(i);
>>>> assert(java_lang_String::is_instance(entry));
>>>>
>>>> In this case, a reference from CP to string is reported regardless 
>>>> it is pseudo string or not.
>>>
>>> Those are very good questions. Here is an example of the specific 
>>> case with a generated JSR292 java.lang.invoke.LambdaForm$MH class. 
>>> The follow are two constant pool entries of the class. Entry 16 is a 
>>> "sun/misc/Unsafe" String, which could be patched using a 
>>> sun.misc.Unsafe klass oop.
>>>
>>>         ...
>>>         (0016) tag=Utf8 "sun/misc/Unsafe"
>>>         (0017) tag=Class "sun/misc/Unsafe"
>>>         ...
>>>
>>> I discussed with John Rose about different ways to fix the assertion 
>>> issue. The code you have above uses pseudo_string_at() to access the 
>>> 'entry', which is similar to one of the version we discussed. The 
>>> current change was chosen as it's the simplest solution.
>>
>> I'm Ok with the simplest solution.
>>
>> Thank you for the details!
>> -Serguei
>>
>>>
>>> Thanks,
>>>
>>> Jiangli
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>> On 10/9/12 4:26 PM, Jiangli Zhou wrote:
>>>>> Please review following fix for 8000459:
>>>>>
>>>>> http://cr.openjdk.java.net/~jiangli/8000459/webrev/
>>>>>
>>>>> With JSR292, a constant pool String entry could be patched to a 
>>>>> non-string oop. The assert in 
>>>>> VM_HeapWalkOperation::iterate_over_class() needs to be adjusted to 
>>>>> reflect that.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Jiangli
>>>>
>>>
>>