Implementation of Annotation.toString() in native binary is inconsistent with JDK and breaks Guice named bindings

[timboudreau]

The implementation of toString() for annotation instances in a native image generated by SubstrateVM simply return the class name of the annotation. This breaks all uses of named or otherwise annotated bindings in Guice, since Key and other code (patching Key was insufficient to work around the problem) in Guice relies on toString() returning different values for instances of the same annotation class with different member values, and returning the same value for annotation instances that have the same values. This renders Guice unusable in native images other than for trivial applications - annotated bindings are a common and encouraged pattern.

While not strictly non-compliant with the annotation spec for toString():

Returns a string representation of this annotation. The details of the representation are implementation-dependent, but the following may be regarded as typical: @com.acme.util.Name(first=Alfred, middle=E., last=Neuman)

the documentation does suggest that returning the same value for all instances of the same annotation type is not expected behavior.

Here is a project which demonstrates the problem and which will log success and exit with an exit code of 0 if the problem is fixed. To run it:

• Have Maven 3.x on your PATH
• Set GRAAL_HOME to a built source checkout of Graal
• CD into the root dir of the unpacked tarball and run build-guice-test. It will
  • Build and run the project normally in Java
  • The injection-reflection-indexer annotation processor generates META-INF/injection/reflective.json in the JAR, which the script passes to native-image
  • The https://github.com/timboudreau/giulius/tree/master/maven-merge-configuration plugin builds a fat-jar and merges all such reflective.json files on the classpath
  • Builds a native image
  • Runs it - you will see some failure messages and it exits non-zero

A fix for this need not produce output identical to that of the JDK, but needs to meet the tests that

• Equal instances of the same non-marker annotation type produce the same output for toString()
• Unequal instances do not,
• All annotation values affect the return value of toString() in such a way that different values result in different output

[timboudreau]

Actually, it appears there is a secondary issue: Guice relies on getDeclaredMethods().length on Annotation subclasses' class objects to determine if an annotation is a marker annotation which has no methods, and can be referenced by type rather than retaining the annotation instance.

In the native image, getDeclaredMethods() always returns an empty array.

So that is the root cause of the key equality problem.

I was able to write a partial patch for SubstrateVM that handles the case of constant values correctly (I don't know this code at all, and it's not obvious how to generate code that concatenates strings - it works for toString() on Named). One thing of interest is that my patch did not quote the string, and Guice actually contains a check whether, in the running JVM, string values in toString() of annotations are quoted, and the implementation of Named that it generates dynamically when creating bindings is quoted or not depending on the result of that test.

So, to deal with this issue entirely requires both getDeclaredMethods() and toString() to behave as they would in the JDK.

I have updated the test project to note differences in both.

[timboudreau]

An update: Exposing the methods @Named (and the other annotation for testing in the test project) works to have getDeclaredMethods() return the right thing, so that works to solve that.

Interestingly, if the metadata simply says to expose all fields and methods of the annotation type for reflection, that does not work - but explicitly exposing methods singlely does.

Further attempts at patching AnnotationSupport just got me to

org.graalvm.compiler.java.BytecodeParser$BytecodeParserError: java.lang.NoClassDefFoundError: Could not initialize class com.oracle.svm.core.nodes.CFunctionPrologueNode
    at parsing com.oracle.svm.core.thread.Safepoint.freezeAtSafepoint(Safepoint.java:202)

which I can't make heads or tails of. I was attempting generate code that would call new StringBuilder("@TheTypeName(").append(callOneAnnotationMethod()).append(", ").append(callNextAnnotationMethod())...toString() - using how hashCode() is implemented as a guide - however, I think I'm getting value nodes for the method calls appended out-of-order somehow.

If you have any pointers on the correct way to concatenate objects into a string in SubstrateVM, I may try further to cook up a working patch.

[peter-hofer]

Thank you for your report @timboudreau! I assigned @cstancu because he is more familiar with our annotations code. Building a complex compiler graph with GraphKit can be cumbersome. The error message you got doesn't tell me anything either. You can use IGV (Ideal Graph Visualizer) to see the compiler graphs, see OPTIONS.md. Some nodes in a graph can "float" and be reordered, but others are anchored or fixed, including invocations. It might be helpful to write an example method in Java and compare its graph to the generated graph in IGV. Did you use allDeclaredMethods on the type in the reflection configuration? That should have the same effect as declaring individual methods -- allPublicMethods only includes those methods that are returned by Class.getMethods. If you give me an example where it doesn't work, I'll look into it. Since you're planning to contribute a patch, please also submit an Oracle Contributor Agreement if you haven't already done so.