RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[David Holmes]

On 2/06/2021 10:17 pm, Peter Levart wrote:
> On Wed, 2 Jun 2021 11:40:13 GMT, Peter Levart <plevart at openjdk.org> wrote:
> 
>>> ...hm, it seems that mere presence of any RUNTIME annotation that was RUNTIME already at the use compile time somehow affects -XX:+PreserveAllAnnotations option so that now RUNTIME annotations that were CLASS annotations at use compile time are not returned. Checking VM logic...
>>
>> For example, if I try this:
>>
>>      @Retention(CLASS)
>>      public @interface AnnA_v1 {}
>>
>>      // An alternative version of AnnA_v1 with RUNTIME retention instead.
>>      // Used to simulate separate compilation (see AltClassLoader below).
>>      @Retention(RUNTIME)
>>      public @interface AnnA_v2 {}
>>
>>      @Retention(RUNTIME)
>>      public @interface AnnB {}
>>
>>      @AnnA_v1
>>      @AnnB
>>      public static class TestTask implements Runnable {
>>          @Override
>>          public void run() {
>>              AnnA_v1 ann1 = TestTask.class.getDeclaredAnnotation(AnnA_v1.class);
>>
>> ... then `ann1` is not returned, but if I comment out the `@AnnB` annotation use on TestTask, `ann1` is returned.
> 
> I think we found a bug here. After all, this endeavor was not in vain. The bug is in AnnotationParser:
> https://github.com/openjdk/jdk/blob/6765f902505fbdd02f25b599f942437cd805cad1/src/java.base/share/classes/sun/reflect/annotation/AnnotationParser.java#L111
> 
> You see, the parser reads a 16bit integer from the start of `rawAnnotations` and interprets it as the number of annotations to parse. But OTOH, `rawAnnotations` may in case when `-XX:+PreserveAllAnnotations` was used, contain two concatenated sets of encoded annotations, each starting with its own count of annotations. See how this concatenation is performed here:
> https://github.com/openjdk/jdk/blob/6765f902505fbdd02f25b599f942437cd805cad1/src/hotspot/share/classfile/classFileParser.cpp#L3965

Two arrays are combined into one array and that is returned. What do you 
mean by "each starting with its own count" ?? Where is the code that 
shoves the apparent length into the start of the rawAnnotations byte[] ?

Thanks,
David
-----

> The concatenation is simply joining the arrays of u1 elements into a concatenated array. So when both RuntimeVisibleAnnotations and RuntimeInvisibleAnnotations are present (in case -XX:+PreserveAllAnnotations was used), the concatenated array starts with RuntimeVisibleAnnotations followed by RuntimeInvisibleAnnotations.
> 
> AnnotationParser should not stop parsing when the 1st loop is finished and the rawAnnotations is not exhausted yet. It should continue with the 2nd round of parsing.
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/4280
>