Operation offset for type annotations for NEW / CHECKCAST etc.

[Michael Ernst]

Thanks to quick work by Werner Dietl, the discussion at
   https://bugs.openjdk.java.net/browse/JDK-8033803
already includes a fix.

(Also thanks to Alex for creating the bug report and to Eric for 
integrating the fix into the version-9 repository.)

Thanks again, André, for the bug report.  Please send us your other 
remarks, too.

                     -Mike


> Subject: Re: Operation offset for type annotations for NEW / CHECKCAST 
> etc.
> From: Alex Buckley <alex.buckley at oracle.com>
> To: type-annotations-dev at openjdk.java.net
> Date: Thu, 06 Feb 2014 11:27:12 -0800
>
> André,
>
> Thanks for reporting this.
>
> Note that a cast expression is represented in the 
> RuntimeVisibleTypeAnnotations attribute differently than an instanceof 
> or new expression. See Table 4.7.20-B. Still, the offset is specified as 
> you would expect: "The value of the offset item specifies the code array 
> offset of either the bytecode instruction corresponding to the cast 
> expression, ...". (I recall we didn't want to mention 'checkcast' 
> explicitly since it wasn't clear that all compilers would emit it all 
> the time - but that's a sideshow.)
>
> Since there is clearly a bug here, I have filed:
>
> https://bugs.openjdk.java.net/browse/JDK-8033803
>
> Alex
>
> On 2/6/2014 8:31 AM, André Pankraz wrote:
>> Hi,
>>
>> because http://bugs.java.com/ didn't work for me, I would like to
>> describe some of my findings with type annotations here.
>> I used jdk-8-fcs-bin-b128-windows-x64-01_feb_2014 for this.
>>
>>
>> Type annotations for operations like instanceof / new are compiled
>> differently for Oracle JDK and Eclipse.
>> The type structure contains an offset in this case, which points to the
>> _previous_ operation for Oracle JDK and not to the relevant operation.
>>
>> Example:
>>
>>      public static String testTypeParam() {
>>          CharSequence str = "test";
>>          return (@Nonnull String) str;
>>      }
>>
>> Javac:
>>    public static java.lang.String testTypeParam();
>>      0  ldc <String "test"> [5]
>>      2  astore_0 [str]
>>      3  aload_0 [str]
>>      4  checkcast java.lang.String [4]
>>      7  areturn
>>        Line numbers:
>>          [pc: 0, line: 61]
>>          [pc: 3, line: 62]
>>        Local variable table:
>>          [pc: 3, pc: 8] local: str index: 0 type: java.lang.CharSequence
>>      RuntimeVisibleTypeAnnotations:
>>        #31 @org.decojer.cavaj.test.jdk8.
>> Nonnull(
>>          target type = 0x47 CAST
>>          offset = 3
>>          type argument index = 0
>>        )
>>
>> Offset 3 should be offset 4 for the checkcast operation (like Eclipse
>> compiler does).
>>
>>
>> The current specification for such type annotations says:
>>
>> "The value of the offset item specifies the code array offset of either 
>> the
>> instanceof bytecode instruction corresponding to the instanceof
>> expression, the
>> new bytecode instruction corresponding to the new expression, or the
>> bytecode
>> instruction corresponding to the method or constructor reference
>> expression."
>>
>> I would guess the javac does it wrong / not spec conform and the offset
>> is to small?
>>
>> One serious consequence here is, that the ASM 5 visits the operations
>> and type annotations in different orders for Eclipse and JDK. The type
>> annotation visitor method is triggered before the operation visitor,
>> which is very bad to handle from the customizing view point.
>>
>>
>> Another recently found consequence is, that I think that this offset
>> calculation is totally wrong. ;)
>> Take this artificial method as example:
>>
>>      public static String testArrays() {
>>          @Size(max = 41)
>>          @Nonnull
>>          List<@Size(max = 42) CharSequence>[] @Size(max = 43) []
>> @Nonnull @Size(max = 44) [][] test = new @Size(max = 45) ArrayList
>> @Size(max = 46) [10][][] @Size(max = 47) @Size(max = 48) [];
>>          return (@Size(max = 49) String) test[0][1][2][3].get(0);
>>      }
>>
>> The offset for Size(49) is 6 and should be 21!
>>
>> If you look into the bytecode you can guess what happens. javac points
>> to the previous atomar stack expression, which is the new
>> ArrayList[][][][]. This results into multiple operations in bytecode
>> (many constants are needed for anewarray). So offset 6 points to
>> iconst_0, which is the first dimension stack value for the anewarray.
>>
>> Sry for long mail...also have other stuff / remarks, but this is the
>> most important one and the spec cannot be changed anymore anyway...
>>
>>
>> Cheers,
>> André