RFR(M): 8057043: Type annotations not retained during class redefine / retransform

[Coleen Phillimore]

Andreas,
This is excellent work!  It looks really good.  Some comments.

http://cr.openjdk.java.net/~aeriksso/8057043/webrev.00/src/share/vm/prims/jvmtiRedefineClasses.cpp.udiff.html

+  u1 target_type = type_annotations_typeArray->at(byte_i_ref);
+  byte_i_ref += 1;
+  RC_TRACE_WITH_THREAD(0x02000000, THREAD, ("target_type=0x%.2x", target_type));
+  RC_TRACE_WITH_THREAD(0x02000000, THREAD, ("location=%s", location_mesg));
+
+  // Skip over target_info
+  switch (target_type) {
+    case 0x00:
+    // kind: type parameter declaration of generic class or interface
+    // location: ClassFile
+    case 0x01:
+    // kind: type parameter declaration of generic method or constructor
+    // location: method_info

Are there any #defines that make these numbers into some symbolic values?

These comments are good in case there aren't.

+    case 0x17:
+    // kind: type in throws clause of method or constructor
+    // location: method_info


I can't find anything wrong.  I don't really understand the test that 
well so hopefully someone that knows type annotations can look at it 
also.   Maybe include lang tools and Joel and Eric?

This code is needed because we don't replace the InstanceKlass when 
redefining classes and rewrite the constant pool.  If we were able to 
replace the InstanceKlass instead, we wouldn't need to do all this 
work.  We're evaluating this at the moment, but have no real progress, 
so thank you for fixing this!

Thanks,
Coleen

On 10/9/14, 9:21 AM, Andreas Eriksson wrote:
> Hi all,
>
> Please review this patch to RedefineClasses to allow type annotations 
> to be preserved.
>
> Summary:
> During redefine / retransform class the constant pool indexes can change.
> Since annotations have indexes into the constant pool these indexes 
> need to be rewritten.
> This is already done for regular annotations, but not for type 
> annotations.
> This patch adds code to add this rewriting for the type annotations as 
> well.
> The patch also contains minor changes to ClassFileReconstituter, to 
> make sure that type annotations are preserved during a redefine / 
> retransform class operation.
> It also has a test that uses asm to change constant pool indexes 
> through a retransform, and then verifies that type annotations are 
> preserved.
>
> Detail:
> A type annotation struct consists of some target information and a 
> type path, followed by a regular annotation struct.
> Constant pool indexes are only present in the regular annotation struct.
> The added code skips over the type annotation specific parts, then 
> calls previously existing code to rewrite constant pool indexes in the 
> regular annotation struct.
> Please see the Java SE 8 Ed. VM Spec. section 4.7.20 for more info 
> about the type annotation struct.
>
> JPRT with the new test passes without failures on all platforms.
>
> Webrev:
> http://cr.openjdk.java.net/~aeriksso/8057043/webrev.00/
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8057043
>
> Regards
> Andreas