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

[serguei.spitsyn at oracle.com]

Dmitry,

Please, see inlined.

On 8/21/12 11:47 AM, Dmitry Samersoff wrote:
> Serguei,
>
> jvmtiClassFileReconstituter.cpp:
>
> 178: attr_count was 1 but become 0 for the case
>
> has_localvariable_table() == true && local_variable_table_length == 0
>
> is it intentional?
Yes.
You can  see the same pattern for all attributes (for instance, 
has_stackmap_table - L160).

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

>
> 216: It might be better to place both attr_size changes together.
>
>     i.e.
>
> if (local_variable_table_length != 0) {
>    ++attr_count;
>    LocalVariableTableElement *elem = method->localvariable_table_start();
>    for (int idx = 0; idx < local_variable_table_length; idx++) {
>      ...
>    }
>
>     // Big comment block
>
>     attr_size += 2 + 4 + 2 + local_variable_table_length ...
>     if (local_variable_type_table_length != 0) {
>        ++attr_count;
>        attr_size += 2 + 4 + 2 + local_variable_type_table_length  ...
>     }
>
> }

I'm trying to follow the existing style.
But thank you for sharing your thoughts on this!


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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20120821/d9addf34/attachment.html