Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GWT Compiler failure with type-use annotations #10001

Closed
natros opened this issue Sep 10, 2024 · 3 comments · Fixed by #10002
Closed

GWT Compiler failure with type-use annotations #10001

natros opened this issue Sep 10, 2024 · 3 comments · Fixed by #10002

Comments

@natros
Copy link
Contributor

natros commented Sep 10, 2024

Hi,

I rely on dagger in all of my projects, but I have encountered an issue with the recent changes made to it. These changes have caused the GWT compiler to fail. The details of this problem can be found in the following link:
google/dagger#4391 (comment)

I have a repo where this issue is present and would like some help resolving them:
https://github.com/natros/jspec

The error messages are as follows:

[INFO] --- gwt:1.1.0:compile (default-compile) @ jspec-client ---
[INFO] Compiling module com.github.natros.jspec.App
[INFO] [ERROR] Unexpected internal compiler error
[INFO] java.lang.ArrayIndexOutOfBoundsException: Index 25856 out of bounds for length 21
[INFO]  at org.objectweb.asm.ClassReader.readUtf(ClassReader.java:3669)
[INFO]  at org.objectweb.asm.ClassReader.readUTF8(ClassReader.java:3656)
[INFO]  at org.objectweb.asm.ClassReader.accept(ClassReader.java:604)
[INFO]  at org.objectweb.asm.ClassReader.accept(ClassReader.java:424)
[INFO]  at com.google.gwt.dev.javac.BytecodeSignatureMaker.visitCompileDependenciesInBytecode(BytecodeSignatureMaker.java:228)
[INFO]  at com.google.gwt.dev.javac.BytecodeSignatureMaker.getCompileDependencySignature(BytecodeSignatureMaker.java:209)
[INFO]  at com.google.gwt.dev.javac.CompiledClass.getSignatureHash(CompiledClass.java:166)
[INFO]  at com.google.gwt.dev.javac.Dependencies$Ref.<init>(Dependencies.java:41)
[INFO]  at com.google.gwt.dev.javac.Dependencies$Ref.<init>(Dependencies.java:36)
[INFO]  at com.google.gwt.dev.javac.Dependencies.resolve(Dependencies.java:107)
[INFO]  at com.google.gwt.dev.javac.CompilationStateBuilder$CompileMoreLater.compile(CompilationStateBuilder.java:349)
[INFO]  at com.google.gwt.dev.javac.CompilationStateBuilder.doBuildFrom(CompilationStateBuilder.java:532)
[INFO]  at com.google.gwt.dev.javac.CompilationStateBuilder.buildFrom(CompilationStateBuilder.java:464)
[INFO]  at com.google.gwt.dev.cfg.ModuleDef.getCompilationState(ModuleDef.java:423)
[INFO]  at com.google.gwt.dev.Precompile.precompile(Precompile.java:210)
[INFO]  at com.google.gwt.dev.Precompile.precompile(Precompile.java:190)
[INFO]  at com.google.gwt.dev.Precompile.precompile(Precompile.java:131)
[INFO]  at com.google.gwt.dev.Compiler.compile(Compiler.java:192)
[INFO]  at com.google.gwt.dev.Compiler.compile(Compiler.java:143)
[INFO]  at com.google.gwt.dev.Compiler.compile(Compiler.java:132)
[INFO]  at com.google.gwt.dev.Compiler$1.run(Compiler.java:110)
[INFO]  at com.google.gwt.dev.CompileTaskRunner.doRun(CompileTaskRunner.java:55)
[INFO]  at com.google.gwt.dev.CompileTaskRunner.runWithAppropriateLogger(CompileTaskRunner.java:50)
[INFO]  at com.google.gwt.dev.Compiler.main(Compiler.java:113)

My understanding is that the GWT compiler does not support type-use annotations with the JSpecify library.
The code in question is similar to the code generated by Dagger.

import org.jspecify.annotations.Nullable;

public interface Factory<T extends @Nullable Object> {}

Tested with gwt 2.11.0 and HEAD-SNAPSHOT

Thanks

@niloc132
Copy link
Contributor

niloc132 commented Sep 11, 2024

I can reproduce the issue with both 2.11 and HEAD. The error message seems to imply that asm's ClassReader believes that the bytecode of org.jspecify.NullMarked is invalid - the string pool only has 21 elements, but it ClassReader seems to think it sees an offset to 25856 in it.

javap believes that this is valid:

$ javap -v --class-path /home/colin/.m2/repository/org/jspecify/jspecify/1.0.0/jspecify-1.0.0.jar org.jspecify.annotations.NullMarked
Classfile jar:file:///home/colin/.m2/repository/org/jspecify/jspecify/1.0.0/jspecify-1.0.0.jar!/org/jspecify/annotations/NullMarked.class
  Last modified Feb 1, 1980; size 498 bytes
  SHA-256 checksum fb7da3d28ee589dcbda99bf8cb2b4f127b2369ff37f4f3e7e34c1befeaa5052f
  Compiled from "NullMarked.java"
public interface org.jspecify.annotations.NullMarked extends java.lang.annotation.Annotation
  minor version: 0
  major version: 52
  flags: (0x2601) ACC_PUBLIC, ACC_INTERFACE, ACC_ABSTRACT, ACC_ANNOTATION
  this_class: #1                          // org/jspecify/annotations/NullMarked
  super_class: #3                         // java/lang/Object
  interfaces: 1, fields: 0, methods: 0, attributes: 2
Constant pool:
   #1 = Class              #2             // org/jspecify/annotations/NullMarked
   #2 = Utf8               org/jspecify/annotations/NullMarked
   #3 = Class              #4             // java/lang/Object
   #4 = Utf8               java/lang/Object
   #5 = Class              #6             // java/lang/annotation/Annotation
   #6 = Utf8               java/lang/annotation/Annotation
   #7 = Utf8               SourceFile
   #8 = Utf8               NullMarked.java
   #9 = Utf8               RuntimeVisibleAnnotations
  #10 = Utf8               Ljava/lang/annotation/Documented;
  #11 = Utf8               Ljava/lang/annotation/Target;
  #12 = Utf8               value
  #13 = Utf8               Ljava/lang/annotation/ElementType;
  #14 = Utf8               MODULE
  #15 = Utf8               PACKAGE
  #16 = Utf8               TYPE
  #17 = Utf8               METHOD
  #18 = Utf8               CONSTRUCTOR
  #19 = Utf8               Ljava/lang/annotation/Retention;
  #20 = Utf8               Ljava/lang/annotation/RetentionPolicy;
  #21 = Utf8               RUNTIME
{
}
SourceFile: "NullMarked.java"
RuntimeVisibleAnnotations:
  0: #10()
    java.lang.annotation.Documented
  1: #11(#12=[e#13.#14,e#13.#15,e#13.#16,e#13.#17,e#13.#18])
    java.lang.annotation.Target(
      value=[Ljava/lang/annotation/ElementType;.MODULE,Ljava/lang/annotation/ElementType;.PACKAGE,Ljava/lang/annotation/ElementType;.TYPE,Ljava/lang/annotation/ElementType;.METHOD,Ljava/lang/annotation/ElementType;.CONSTRUCTOR]
    )
  2: #19(#12=e#20.#21)
    java.lang.annotation.Retention(
      value=Ljava/lang/annotation/RetentionPolicy;.RUNTIME
    )

At the time when the error is happening, it is processing the RuntimeVisibleAnnotations section, and working on the annotation at index 2, @Retention(RUNTIME), but fails when reading the class's name. It should get an index of 19 (that is, 0x0013, which does appear at position 477 and 478 in the class buffer), but instead sees 25856 which is 0x6500 at positions 455 and 456.

I'm pretty sure objectweb has misread the preceding annotation @Target's value, at least from following the pattern of bytes - the offsets for MODULE, PACKAGE, TYPE, METHOD, CONSTRUCTOR should mean we see 14, 15, 16, 17, 18 in some kind of a sequence, and after those we should find 0x0013 (again, that's 0 followed by 19). Here's the bytes in the stream:
screenshot906
There's definitely a break in the pattern though - between 17 and 18 there should be 101, 0, 13, 0. The trailing 0 gets read as part of 18, and 0, 13 gets read as the offset in the constant pool for CONSTRUCTOR, but I'm not yet sure what purpose the 101 (0x65) serves (EDIT: this is ascii for the char e which signals "the next thing is an enum" - how can this be missing?).

Java and asm disagree here, I'm inclined to side with Java without more evidence, but this does look like a bug in asm rather than GWT. I'll try to dig in a little more and file an issue with asm with a reproducer trying to read jspecify.jar

More reading: https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.7.16

niloc132 added a commit to niloc132/gwt that referenced this issue Sep 11, 2024
While GWT has no use for JPMS modules, sources that rely on this target
fail to compile with GWT (causing an error in JDT that results in a
hard-to-interpret asm stack trace).

Fixes gwtproject#10001
@niloc132
Copy link
Contributor

My notes from working on this today, in case something like this comes back again and finding these in a search helps me or anyone else:


Updating GWT to asm 9.7 does not resolve this.

Based on the spec link above, I went over the bytes again, starting from the top of the RuntimeVisibleAnnotations section. I didn't quite have the pattern right above, the e immediately precedes the value rather than following it (this is 4.7.16.1's element_value struct - the tag u1 comes before the union's enum_constant_value struct. I also didn't have the offset quite right, 25856 appears at a few places, but only one that matters.

Bytes of the file with their offset and my summary, starting at 432:

Start of RuntimeVisibleAnnotations_attribute struct
attribute_name_index - ref to constant pool #9, RuntimeVisibleAnnotations
432 = 0
433 = 9

attribute_length
434 = 0
435 = 0
436 = 0
437 = 43

num_annotations - there are three annotations for this class
438 = 0 
439 = 3

Start of first annotation struct
type_index - ref to constant pool #10, Ljava/lang/annotation/Documented;
440 = 0
441 = 10

num_element_value_pairs - zero, so we continue to the next annotation
442 = 0
443 = 0

Start of second annotation struct
type_index - ref to constant pool #11, Ljava/lang/annotation/Target;
444 = 0
445 = 11

num_element_value_pairs - we have one attr value in this annotation - we _should_ only loop once on element_value_pairs
446 = 0
447 = 1

element_value_pairs's element_name_index - ref to constant pool #12, value (i.e. we're definning @Target's value)
448 = 0
449 = 12


element_value_pairs's first value - shouldn't this be 91, '[' to signal that we have an array of values here? Then the count of values (5), then the values themselves?

tag is 'e', this is an enum
450 = 101

enum_constant_value type_name_index - ref to constant pool #13, Ljava/lang/annotation/ElementType;
451 = 0
452 = 13

enum_constant_value const_name_index - ref to constant pool #14, MODULE
453 = 0
454 = 14

Probably should have been another ref to #12 "value" here?

element_value_pairs's second value - but the count was 1, we shouldn't keep reading!
tag is 'e', this *should be* an enum
455 = 101

enum_constant_value type_name_index - ref to constant pool #13, Ljava/lang/annotation/ElementType;
456 = 0
457 = 13

enum_constant_value const_name_index - ref to constant pool #15, PACKAGE
458 = 0
459 = 15

element_value_pairs's third value - but the count was 1, we shouldn't keep reading!
tag is 'e', this *should be* an enum
460 = 101

enum_constant_value type_name_index - ref to constant pool #13, Ljava/lang/annotation/ElementType;
461 = 0
462 = 13

enum_constant_value const_name_index - ref to constant pool #16, TYPE
463 = 0
464 = 16

Next element_value
tag is 'e', this is an enum
465 = 101

enum_constant_value type_name_index - ref to constant pool #13, Ljava/lang/annotation/ElementType;
466 = 0
467 = 13

enum_constant_value const_name_index - ref to constant pool #17, METHOD
468 = 0
469 = 17

Even if we read the wrong 
element_value_pairs's fourth value? - but the count was 1, we shouldn't keep reading! A plain reading of the class says we should read CONSTRUCTOR now
tag is '\0', this is garbage
470 = 0
471 = 18
472 = 0
473 = 1
474 = 0
475 = 12
476 = 101
477 = 0
478 = 19
479 = 0
480 = 20

So the bytes don't make sense, but I couldn't understand why. Next I looked to see where the bytes were coming from, in case ASM was doing its best, but something in GWT was causing the issue.

DiskCacheToken had an offset into my /tmp/gwtbyte-cache file, which pointed to 4 bytes before the specific class started. Those 4 bytes had the length to read from the file - after adding 4 to the offset and using the given length, I was able to use hexdump to read a working jspecify annotation (NonNull), and confirm that the data on disk was nearly the same as in the jar, but not quite. The changes seemed to be at the very beginning of the file

In contrast, MarkedNull had many more inconsistencies, whole sections of the data were missing, rather than just having written slightly different values in a few places. Tracing how the CompiledClass instance was created (the constructor of the call in the initial stack trace), the original JDT-created class had errors associated with it - specifically that ElementType.MODULE could not be found. Updating GWT's own copy of ElementType.java appears to have resolved this:

[INFO] --- gwt:1.1.0:compile (default-compile) @ jspec-client ---
[INFO] Compiling module com.github.natros.jspec.App
[INFO]    Compiling 2 permutations
[INFO]       Compiling permutation 0...
[INFO]       Compiling permutation 1...
[INFO]    Compile of permutations succeeded
[INFO]    Compilation succeeded -- 13.378s
[INFO] Linking into /home/colin/workspace/jspec/jspec-client/target/jspec-client-1.0-SNAPSHOT/app
[INFO]    Link succeeded
[INFO]    Linking succeeded -- 0.106s

The linked PR resolves this for me locally, but it might be a good idea to warn about JDT-reported errors, in case they cause other internal errors like this.

@natros
Copy link
Contributor Author

natros commented Sep 12, 2024

I did a test and it seems to be working fine with dagger HEAD-SNAPSHOT.

You have done an exceptional job in solving this problem.
Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants