Request for review:8023547:com/sun/jdi/RedefineMulti.sh fails with IllegalArgumentException after JDK-8021948

[Jiangli Zhou]

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