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

[Staffan Larsen]

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