Request for review:8023477: Invalid CP index when reading ConstantPool

[Jiangli Zhou]

Hi,

Please review the fix for 8023477:

   http://cr.openjdk.java.net/~jiangli/8023477/webrev.00/

Both tests reported by the bug failed due to the same problem. They both 
are passing now.

The original memory reduction change for 8021948 turned out to be a 
little trickier than I expected.

Thanks,
Jiangli


On 08/23/2013 11:00 AM, Jiangli Zhou wrote:
> I found the issue in the SA code. For class without generic signature, 
> the InstanceKlass::_generic_signature_index is 0. Need to check for 
> this case in the SA code. I'm working on the fix and doing testing.
>
> Thanks,
> Jiangli
>
> On 08/23/2013 08:21 AM, Jiangli Zhou wrote:
>> Hi Staffan,
>>
>> Thanks for the info. Will look into that one.
>>
>> Jiangli
>>
>> On 08/23/2013 05:27 AM, Staffan Larsen wrote:
>>> Unfortunately there are two more tests that started failing after 
>>> the initial change. See: JDK-8023477.
>>>
>>> /Staffan
>>>
>>> On 23 aug 2013, at 01:16, Jiangli Zhou <jiangli.zhou at oracle.com> wrote:
>>>
>>>> Hi Coleen,
>>>>
>>>> Yes, that's the case. Thanks for the review. I'll push this to 
>>>> hotspot-rt.
>>>>
>>>> Thanks,
>>>> Jiangli
>>>>
>>>> On 08/22/2013 03:34 PM, Coleen Phillimore wrote:
>>>>> Hi,
>>>>> Is it the case that the old index isn't in the index map because 
>>>>> it didn't change?  If so, this looks correct.
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>> On 08/22/2013 06:15 PM, Jiangli Zhou wrote:
>>>>>> On 08/22/2013 03:10 PM, serguei.spitsyn at oracle.com wrote:
>>>>>>> Hi Jiangli,
>>>>>>>
>>>>>>> The fix is good and safe.
>>>>>>> I'm happy you fixed another case as well!
>>>>>>> Let's consider current bug as a clean-up issue so that we do not 
>>>>>>> need to file a separate bug. :)
>>>>>> Ok.
>>>>>>
>>>>>> Thanks,
>>>>>> Jiangli
>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>
>>>>>>> On 8/22/13 2:50 PM, Jiangli Zhou wrote:
>>>>>>>> Hi Serguei,
>>>>>>>>
>>>>>>>> I've also made change to the case that you discovered. Please 
>>>>>>>> let me know if you think a separate bug should be filed to 
>>>>>>>> track it instead.
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~jiangli/8023547/webrev.01/
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Jiangli
>>>>>>>>
>>>>>>>> On 08/22/2013 02:24 PM, Jiangli Zhou wrote:
>>>>>>>>> Hi Serguei,
>>>>>>>>>
>>>>>>>>> Thank you very much for the review and confirmation with the 
>>>>>>>>> test.
>>>>>>>>>
>>>>>>>>> Jiangli
>>>>>>>>>
>>>>>>>>> On 08/22/2013 02:18 PM, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>> Hi Jiangli,
>>>>>>>>>>
>>>>>>>>>> Thank you for the quick fix which looks fine to me.
>>>>>>>>>> I confirm the test is passed with it.
>>>>>>>>>>
>>>>>>>>>> Staffan, thank you for the regression isolation.
>>>>>>>>>> I've noticed the following fragment in this file which seems 
>>>>>>>>>> has a similar issue:
>>>>>>>>>>
>>>>>>>>>>   // We also need to rewrite the parameter name indexes, if 
>>>>>>>>>> there is
>>>>>>>>>>   // method parameter data present
>>>>>>>>>>   if(method->has_method_parameters()) {
>>>>>>>>>>     const int len = method->method_parameters_length();
>>>>>>>>>>     MethodParametersElement* elem = 
>>>>>>>>>> method->method_parameters_start();
>>>>>>>>>>
>>>>>>>>>>     for (int i = 0; i < len; i++) {
>>>>>>>>>>       const u2 cp_index = elem[i].name_cp_index;
>>>>>>>>>>       elem[i].name_cp_index = find_new_index(cp_index);
>>>>>>>>>>     }
>>>>>>>>>>   }
>>>>>>>>>> } // end rewrite_cp_refs_in_method()
>>>>>>>>>>
>>>>>>>>>> The result of the find_new_index() above is not checked for 0.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Serguei
>>>>>>>>>>
>>>>>>>>>> On 8/22/13 12:38 PM, Jiangli Zhou wrote:
>>>>>>>>>>> Hi Staffan, Serguei and others,
>>>>>>>>>>>
>>>>>>>>>>> Here is the webrev for the 8023547 fix:
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~jiangli/8023547/webrev.00/
>>>>>>>>>>>
>>>>>>>>>>> Thanks!
>>>>>>>>>>>
>>>>>>>>>>> Jiangli
>>
>