Eclipse JDT produces bytecode that can't be indexed by Jandex

[RoiSoleil]

Eclipse JDT produces bytecode that can't be indexed by Jandex

[iloveeclipse]

@RoiSoleil : could you please attach a self contained project that has all required libraries to reproduce the problem?

Why do you think if the 3rd party library can't parse bytecode, it is a bug in JDT and not in the 3rd party bytecode parser?

[iloveeclipse]

Actually I can reproduce without the "Indexer" from Jandex.
Just have Bytecode Outline view opened in 4.27 master and get this error:

Cannot decompile: [Working copy] A.java [in test [in src [in A2]]]
  package test
  import java.util.*
  class A
    static void main(String[])
    class EList
      EList(T ...)
      Iterator<T> iterator() null

java.lang.IllegalArgumentException
	at org.objectweb.asm.signature.SignatureReader.parseType(SignatureReader.java:249)
	at org.objectweb.asm.signature.SignatureReader.accept(SignatureReader.java:114)
	at org.objectweb.asm.util.Textifier.appendJavaDeclaration(Textifier.java:1349)
	at org.objectweb.asm.util.Textifier.visitMethod(Textifier.java:418)
	at org.eclipse.jdt.bcoview.asm.CommentedClassVisitor.visitMethod(CommentedClassVisitor.java:164)
	at org.eclipse.jdt.bcoview.asm.CommentedClassVisitor.visitMethod(CommentedClassVisitor.java:1)
	at org.objectweb.asm.util.TraceClassVisitor.visitMethod(TraceClassVisitor.java:230)
	at org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:645)
	at org.objectweb.asm.tree.ClassNode.accept(ClassNode.java:468)
	at org.eclipse.jdt.bcoview.asm.DecompilerHelper.getDecompiledClass(DecompilerHelper.java:44)
	at org.eclipse.jdt.bcoview.views.BytecodeOutlineView.decompileBytecode(BytecodeOutlineView.java:1458)
	at org.eclipse.jdt.bcoview.views.BytecodeOutlineView.refreshView(BytecodeOutlineView.java:1054)
	at org.eclipse.jdt.bcoview.views.BytecodeOutlineView.handlePartVisible(BytecodeOutlineView.java:951)
	at org.eclipse.jdt.bcoview.views.EditorListener.partOpened(EditorListener.java:121)
	at org.eclipse.ui.internal.PartService$10.run(PartService.java:193)

[Ladicek]

Hi, Jandex maintainer here (the bytecode parsing library in question).

Here's a minimal reproducer:

import java.util.function.Supplier;
  
public class MyList<T> {
    public MyList(T... elements) {
    }

    public static Supplier<? extends MyList<Object>> supplier() {
        return MyList<Object>::new;
    }
}

Compile with ECJ like this:

java -jar ecj-4.26.jar -17 MyList.java

And inspect the generated bytecode using javap:

javap -v -p MyList.class

You'll notice that the generated signature for the synthetic lambda method is ()+LMyList<Ljava/lang/Object;>;. That doesn't conform to the grammar in the JVMS, chapter 4.7.9.1.

[iloveeclipse]

• ecj_421_dump.txt
• ecj_dump.txt

ECJ generates this (4.21 and 4.27 master are generating same code):

  private static ? extends MyList<java.lang.Object> lambda$1();
    descriptor: ()LMyList;
    flags: (0x100a) ACC_PRIVATE, ACC_STATIC, ACC_SYNTHETIC
    Signature: #29                          // ()+LMyList<Ljava/lang/Object;>;
    Code:
      stack=3, locals=0, args_size=0
         0: new           #1                  // class MyList
         3: dup
         4: iconst_0
         5: anewarray     #3                  // class java/lang/Object
         8: invokespecial #30                 // Method "<init>":([Ljava/lang/Object;)V
        11: areturn
      LineNumberTable:
        line 1: 0

javac generates this

  private static MyList lambda$supplier$0();
    descriptor: ()LMyList;
    flags: (0x100a) ACC_PRIVATE, ACC_STATIC, ACC_SYNTHETIC
    Code:
      stack=3, locals=0, args_size=0
         0: new           #3                  // class MyList
         3: dup
         4: iconst_0
         5: anewarray     #4                  // class java/lang/Object
         8: invokespecial #5                  // Method "<init>":([Ljava/lang/Object;)V
        11: areturn
      LineNumberTable:
        line 8: 0

[Ladicek]

Aside: one funny thing is that javap doesn't hesitate to print private static ? extends MyList<java.lang.Object> lambda$1();, which clearly contains an invalid return type :-)

[iloveeclipse]

? extends MyList<java.lang.Object> as return type for synthetic lambda$1 method seem to be strange, differs from simple MyList of javac.

[Ladicek]

Because javac doesn't emit the signature. (And hence javap prints a return type based on the descriptor.)

[iloveeclipse]

@othomann, @stephan-herrmann : could you please look at the lambda generation above? It is not a recent regression, so is not something I can easily spot.

[RoiSoleil]

Nice to see that it's quickly handled ;) Thx

[stephan-herrmann]

? extends MyList<java.lang.Object> as return type for synthetic lambda$1 method seem to be strange, differs from simple MyList of javac.

I think from a type inference p.o.v. that return type is correct (modulo wildcard capture), since the method implements Supplier<? extends MyList<Object>>. Simple MyList, OTOH, is correct from a JVM p.o.v.

Without having debugged the sample, my guess is: ecj sees good reason to generate a signature attribute (not needed by the JVM, but useful for tools and reflection), but then JVMS doesn't specify syntax for this type. In other similar bugs we learned that the compiler should simply refuse generating the signature, reasoning that even if the compiler should normally generate this attribute, we should rather err on the side of missing information in the byte code, rather than emitting illegal signatures. We probably need to implement more heuristics, which signatures should be avoided.

This could be further complicated if it's the same implementation where internally JDT expects some signature always, and now for code generation it must refuse to compute a signature.

[RoiSoleil]

Any news about that ?

[srikanth-sankaran]

See https://mail.openjdk.org/pipermail/compiler-dev/2015-January/009268.html - javac never generates Signature attributes for synthetic lambda body methods.

See also: https://bugs.eclipse.org/bugs/show_bug.cgi?id=449063

I'll investigate

[srikanth-sankaran]

? extends MyList<java.lang.Object> as return type for synthetic lambda$1 method seem to be strange, differs from simple MyList of javac.

I think from a type inference p.o.v. that return type is correct (modulo wildcard capture), since the method implements Supplier<? extends MyList<Object>>. Simple MyList, OTOH, is correct from a JVM p.o.v.

I'll start with that assertion.

Without having debugged the sample, my guess is: ecj sees good reason to generate a signature attribute (not needed by the JVM, but useful for tools and reflection), but then JVMS doesn't specify syntax for this type. In other similar bugs we learned that the compiler should simply refuse generating the signature, reasoning that even if the compiler should normally generate this attribute, we should rather err on the side of missing information in the byte code, rather than emitting illegal signatures. We probably need to implement more heuristics, which signatures should be avoided.

I went hunting for uses of ExtraCompilerModifiers.AccGenericSignature where we see a clear (&= ~ExtraCompilerModifiers.AccGenericSignature), but see only two uninteresting cases.

I am fairly certain what you are alluding is to "non-denotable types" and the compiler punting from whatever operation (diamond inference) it is doing when encountering one of those.

I agree, we should suppress the generation of this attribute when we encounter a non-denotable type.

(See org.eclipse.jdt.internal.compiler.ast.QualifiedAllocationExpression.validate(...).ValidityInspector - this itself is fairly "polluted" and can't be used as is as it has side effects)

This could be further complicated if it's the same implementation where internally JDT expects some signature always, and now for code generation it must refuse to compute a signature.

[srikanth-sankaran]

I have raised a PR that is undergoing test - apart from the code freeze period coming in the way, there is also some problems with my old and new eclipse identities colliding in various manner. So, this will take a while to get integrated into mainline, but if someone wants to test against their tooling/library environments, then the patch can be grabbed (after Jenkins blesses it)

[stephan-herrmann]

More prior art FYI: https://bugs.eclipse.org/bugs/show_bug.cgi?id=494198 where we approximate by erasure rather than dropping the signature.

[srikanth-sankaran]

Thanks, for good or bad, I will try aligning with this prior strategy.

[srikanth-sankaran]

Food for thought: This fix ought to leave in us a better place but I wonder if "approximate by erasure" results in coarseness. I wonder if we should reach for the bounds suitably instead in the case of wildcards.

[stephan-herrmann]

Food for thought: This fix ought to leave in us a better place but I wonder if "approximate by erasure" results in coarseness. I wonder if we should reach for the bounds suitably instead in the case of wildcards.

When asking which strategy would be 'better', do we have any metric for this, i.e., what would be our goals (in which priority):

• don't crash
• don't let tools reading our class files crash
• don't violate JVMS
• mimic (exactly?) what javac does
• use our own judgement which strategy (roughly within JVMS) might provide most value to tools and reflection

[srikanth-sankaran]

See #1048 - while the GSFE goes away. encoding the generic signature as ()Ljava/util/stream/Stream<TT;>; appears problematic - I am yet to fully analyze.

[srikanth-sankaran]

Food for thought: This fix ought to leave in us a better place but I wonder if "approximate by erasure" results in coarseness. I wonder if we should reach for the bounds suitably instead in the case of wildcards.

When asking which strategy would be 'better', do we have any metric for this, i.e., what would be our goals (in which priority):

• don't crash
• don't let tools reading our class files crash
• don't violate JVMS
• mimic (exactly?) what javac does
• use our own judgement which strategy (roughly within JVMS) might provide most value to tools and reflection

We are using the "use our own approach ..." here to accomplish some of the earlier bullets. Javac doesn't emit generic signature for synthetic methods at all - I think we should in the cases where the signature is non denotable do exactly that.

Lengthy discussion here: #414

[srikanth-sankaran]

FTR, pursuant to discussion at #414, the PR #1070 replaces the approximate-by-erasure strategy implemented for this ticket with "skip generic signature emission for synthetic methods if non denotable types are involved" strategy.