RFR: 8268894: ArrayIndexOutOfBoundsException in jdk compiler: at jdk.compiler/com.sun.tools.javac.jvm.ClassWriter.writePosition(ClassWriter.java:671)

[Vicente Romero]

On Sun, 11 Jul 2021 09:36:53 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> Hi all,
> 
> The method `TypeAnnotationPosition.updatePosOffset` sets the `lvarOffset` unnecessarily. After invoking `TypeAnnotationPosition.updatePosOffset`, the lengths of (`lvarOffset`, `lvarLength`, `lvarIndex`) are respectively (1, 0, 0). Then `Code.fillLocalVarPosition` fills the `lvarOffset`, `lvarLength`, `lvarIndex` which make their lengths become (N, N-1, N-1). At last, the method `ClassWrite.writePosition` uses these three arrays whose lengths are not the same so that the `ArrayIndexOutOfBoundsException` occurs.
> 
> The `lvarOffset` should be always changed along with `lvarLength` and `lvarIndex` so that their lengths can be the same.
> 
> This patch removes the unnecessary `lvarOffset = new int[]{to};` of the method  `TypeAnnotationPosition.updatePosOffset` and add a corresponding test.
> 
> ---
> Steps to reproduce the issue:
> 
> 1. Construct a annotation processor which set the `pos` of the method sub-tree nodes to the same position. Please see the file `TypeAnnotationPositionProcessor.java` for more details.
> 2. Compile the code which has the corresponding method with the annotated local variable by using the annotation processor generated at the step 1. Please see the file `TypeAnnotationPositionTest.java` for more details.
> 3. Then the crash occurs.
> 
> Actually, this bug may never occur if the users use the compiler friendly. Because the method `TypeAnnotationPosition.updatePosOffset`, which we mentioned at the beginning, may never be invoked at the situation that the different trees has different and right position.
> 
> The lib `lombok` constructs some new tree nodes by using the internal method of the compiler. But it doesn't set the `pos` correctly and uses the same fake position. The unfriendly code of the `lombok` is in the method [JavacHandlerUtil.setGeneratedBy](https://github.com/projectlombok/lombok/blob/master/src/core/lombok/javac/handlers/JavacHandlerUtil.java#L167). At much time, the fake same position works well, but this time, it crashes the compiler.
> 
> ---
> A brief history:
> 
> According to the git's records, I found the code `ta.position.lvarOffset = new int[] { code.cp };` was firstly added at [8006775: JSR 308: Compiler changes in JDK8](http://hg.openjdk.java.net/jdk8/jdk8/langtools/rev/71f35e4b93a5#l52.35) .
> 
> Then, [8013852: update reference impl for type-annotations](http://hg.openjdk.java.net/jdk8/jdk8/langtools/rev/ddb4a2bfcd82#l8.52) moved the code to `TypeAnnotationPosition.updatePosOffset`.
> 
> The bug didn't occur at that time, because the old [Code.fillLocalVarPosition](http://hg.openjdk.java.net/jdk8/jdk8/langtools/rev/71f35e4b93a5#l51.21) always created a new array instead of using the old array.
> 
> But later, [8231826: Implement javac changes for pattern matching for instanceof](https://hg.openjdk.java.net/jdk/jdk/rev/7799a51dbe30#l24.27) revised the method `Code.fillLocalVarPosition` to reuse the old array so that the bug occurs.
> 
> Concluded, this is an issue introduced at JDK8 when implementing the type annotation. But it was hidden by other code at the past. 
> 
> ---
> 
> Thanks for taking the time to review.
> 
> Best Regards,
> -- Guoxiong

from your description it seems like the bug is more on the lombok side than in javac's, if I understand correctly you are saying that the bug is provoked by lombok because it is not generating completely correct ASTs right?, it could be that we need to make sure that the 3 arrays are in sync before generating the class file, but still the information in `lvarOffset` seems to be used so not generating it doesn't seem like the best option. Please let me know if I'm missing something

-------------

PR: https://git.openjdk.java.net/jdk/pull/4749