Review request for: 7191786 retransformClasses() does not pass in LocalVariableTypeTable of a method

[serguei.spitsyn at oracle.com]

Dmitry,

Thank you for the review!

Please, find new webrev version here:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2012/7191786-JVMTI-LVTT.1/

I also changed the line:
   196 if (elem[idx].signature_cp_index > 0) {
to this:
   196 if (elem[idx].signature_cp_index != 0) {

Both are correct as the signature_cp_index is u2.
But "signature_cp_index != 0" is more clear.


Thanks,
Serguei


On 8/21/12 12:45 PM, serguei.spitsyn at oracle.com wrote:
> On 8/21/12 12:11 PM, Dmitry Samersoff wrote:
>> Serguei,
>>
>> On 2012-08-21 23:05, serguei.spitsyn at oracle.com wrote:
>>> You can  see the same pattern for all attributes (for instance,
>>> has_stackmap_table - L160).
>> OK.
>>
>>>> 202: local_variable_type_table_length
>>>>       is always 0 if local_variable_table_length == 0
>>>>
>>>>       so I think if should be nested.
>>> It does not matter.
>> It saves one if in some cases and makes logic better readable,
>> so I would like to have it changed.
>>
>> Sorry!
>
> In fact, I initially considered this option and can change it as you 
> prefer.
> But let's wait for other reviewers input.
>
> Thanks,
> Serguei
>
>>
>>>> 216: It might be better to place both attr_size changes together.
>>> I'm trying to follow the existing style.
>> OK
>>
>> -Dmitry
>>
>>
>>>
>>> Thanks,
>>> Serguei
>>>> -Dmitry
>>>>
>>>>
>>>> On 2012-08-21 22:13, serguei.spitsyn at oracle.com wrote:
>>>>> Hello,
>>>>>
>>>>>
>>>>> Please, review the fix for:
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7191786
>>>>>
>>>>> Open webrev:
>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2012/7191786-JVMTI-LVTT/
>>>>>
>>>>>
>>>>> Summary:
>>>>>
>>>>>   The following CR was recently fixed:
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064927
>>>>>
>>>>>   But the same issue exists for the LocalVariableTypeTable attribute.
>>>>>
>>>>>   The fix was tested with the modified test:
>>>>> test/java/lang/instrument/VerifyLocalVariableTableOnRetransformTest.sh 
>>>>>
>>>>>
>>>>> The modification is that a local variable with generic signature 
>>>>> is added
>>>>> to the class DummyClassWithLVT.java:
>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2012/7191786-JLI-Test-For-JVMTI-LVTT/ 
>>>>>
>>>>>
>>>>>
>>>>> The test patch will be integrated into the jdk8/tl after the 
>>>>> HotSpot fix
>>>>> is promoted.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>
>