[GR-39408] Native-image fails to compile application due to unresolved reference although the said reference is unreachable

[zakkak]

Describe the issue
Despite the fact that the static analysis is able to figure out that a code path and thus a class is unreachable native-image will still complain for that class if it is unresolved at build time when using --link-at-build-time.

Steps to reproduce the issue

git clone --branch unreachable-unresolved-code https://github.com/zakkak/issue-reproducers reproducers
cd reproducers
mvn package
export JAVA_HOME=/opt/jvms/graalvm-ce-java11-22.1.0

# run with Unreachable on the classpath
$JAVA_HOME/bin/java -cp target/classes Main

# run with Unreachable not on the classpath
$JAVA_HOME/bin/java \
  -jar target/reproducer-1.0-SNAPSHOT.jar

# generate native-image with Unreachable not on the classpath
$JAVA_HOME/bin/native-image \
  --link-at-build-time \
  --initialize-at-build-time=. \
  -jar target/reproducer-1.0-SNAPSHOT.jar

The last command will fail with:

Error: com.oracle.graal.pointsto.constraints.UnresolvedElementException: Discovered unresolved type during parsing: Unreachable. This error is reported at image build time because class Main is registered for linking at image build time by command line
Detailed message:
Trace: 
	at parsing Main.main(Main.java:26)
Call path from entry point to Main.main(String[]): 
	at Main.main(Main.java:25)
	at app//com.oracle.svm.core.JavaMainWrapper.runCore(JavaMainWrapper.java:149)
	at app//com.oracle.svm.core.JavaMainWrapper.run(JavaMainWrapper.java:185)
	at com.oracle.svm.core.code.IsolateEnterStub.JavaMainWrapper_run_5087f5482cc9a6abc971913ece43acb471d2631b(generated:0)

Current work-around: Removing --link-at-build-time allows the build to succeed, but the build should not fail in the first place.
UnresolvedElementException should only be thrown when reachable, according to the static analysis, classes cannot be resolved.

Describe GraalVM and your environment:

• GraalVM version: 22.1.0 and d0f332c
• JDK major version: 11
• OS: Fedora 36
• Architecture: AMD64

More details
Inspecting the compiler IR we see that for the given example:

    @NeverInline("I want to see the BGV")
    public static void main(String[] args) {
        if (unreachableIsReachable()) {
            Unreachable.reached();
        }

        System.out.println("Hello world!");
    }

Graal is able to figure out that unreachableIsReachable() is always false:

Which means that there is no real need for Unreachable to be resolved at build time since it's not going to be added to the native image anyway.

PS: I am willing to implement a fix for this and would appreciate some guidance on how to approach it.

[fniephaus]

@olpaw could you provide some guidance for @zakkak please? Thanks!

[zakkak]

ping @olpaw

[olpaw]

This is not a bug.

• There are two classes (Main and Unreachable).
• The Main class makes use of Class.forName("Unreachable")
• If you try to build an image for class Main but you do not also have class Unreachable on classpath, the image build fails when the --link-at-build-time option is used. That is exactly what is expected.

[olpaw]

To further clarify, to allow Class.forName("Unreachable") to fail in the context of image building --link-at-build-time must not be used since the idea of --link-at-build-time is to disallow any code that is part of what results in the image to produce ClassNotFoundException or LinkageError. This is true for code that gets run at image build-time are well as code that is only run at image-runtime.

[zakkak]

Thanks for the comments @olpaw.

The thing is that the error message references the access happening in Main.java:26 which is dead code, it doesn't complain for Main.java:14 where the class loading is attempted at build time.

The Main class makes use of Class.forName("Unreachable")

This doesn't seem to trigger an error if I just remove line 26 in the same example.

To try and rephrase my concern/expectation. If a library reflectively checks if classes A or B are reachable and only uses them when reachable, I would like a way to have native-image compile the code without errors even if A and/or B are unreachable while still ensuring that anything reachable by the generated native code is linked at build time.

[olpaw]

If a library reflectively checks if classes A or B are reachable and only uses them when reachable, ...

Well, If I library does that, then, generally, you cannot reliably use --link-at-build-time with it. The act of reflectively checking and being able to receive a ClassNotFoundException requires to not use --link-at-build-time.

I would like a way to have native-image compile the code without errors even if A and/or B are unreachable while still ensuring that anything reachable by the generated native code is linked at build time.

How about moving the setting of Main.CNFE into a Feature. Define a org.graalvm.nativeimage.hosted.Feature#beforeAnalysis implementation that uses org.graalvm.nativeimage.hosted.Feature.FeatureAccess#findClassByName. That should work.

[olpaw]

The thing is that the error message references the access happening in Main.java:26 which is dead code

But to determine that unreachableIsReachable() returns false the analysis has to access Main.CNFE. And for that it needs to evaluate the static initializer before. The execution of the static initializer relies on being able to throw a ClassNotFoundException. Which it is not allowed if --link-at-build-time is used.

This doesn't seem to trigger an error if I just remove line 26 in the same example.

If the body of the if-then statement is empty and if the unreachableIsReachable() method is side effect free, the analysis is allowed to not evaluate unreachableIsReachable() at all. cc @cstancu @christianwimmer

[zakkak]

How about moving the setting of Main.CNFE into a Feature. Define a org.graalvm.nativeimage.hosted.Feature#beforeAnalysis implementation that uses org.graalvm.nativeimage.hosted.Feature.FeatureAccess#findClassByName. That should work.

Sure, for things we control that would be fine. I am thinking about 3rd party libraries though, where it's much harder to convince maintainers to include GraalVM dependent code in contrast to asking for some refactoring, like a static initializer and some if-else guards.

If the body of the if-then statement is empty and if the unreachableIsReachable() method is side effect free, the analysis is allowed to not evaluate unreachableIsReachable() at all.

FWIW replacing the if body with a print still doesn't reproduce the issue and the resulting binary behaves as expected.

        if (unreachableIsReachable()) {
            // Unreachable.reached();
            System.out.println("Unreachable.reached()");
        }

[olpaw]

FWIW replacing the if body with a print still doesn't reproduce the issue and the resulting binary behaves as expected.

I assume this is because println is not defined in class Unreachable. Having a call to a method in class Unreachable in the then-body seems to be what triggers evaluation of the static initializer of Unreachable. Looks like I was assuming static-analysis cleverness that we do not actually have.

But none of that invalidates what I said about the semantics of --link-at-build-time.

[zakkak]

OK, thanks for the clarifications @olpaw. If more such cases start appearing in the future we will consider creating a feature request for a less strict --link-at-buil-time mode.

[zakkak]

I assume this is because println is not defined in class Unreachable. Having a call to a method in class Unreachable in the then-body seems to be what triggers evaluation of the static initializer of Unreachable. Looks like I was assuming static-analysis cleverness that we do not actually have.

Note: The static initializer is actually in Main so it's always triggered. It looks like the semantics you describe are not enforced at the moment. If I get this right, in order to be enforced GraalVM would need to catch all CNFEs happening during build time initialization and report them as errors when --link-at-build-time is being used.

[christianwimmer]

First, I want to start with my usual disclaimer that --initialize-at-build-time=. is never a good idea. Currently, with regular class initialization at run time, the simulation of class initialzer would not succeed for the Main class. We intrinsify "successful" Class.forName calls to Class constants, but not yet unsuccessful ones to pre-allocated exceptions. Mostly because that exception then would not have a stack trace at run time.

Why is this relevant? Even if we would simulate the class initializer successfully, the simulation results are not yet available during bytecode parsing, but only for the "inlining before analysis".

But now to the actual issue, what is going on in the example:

The --link-at-build-time option is currently implement using the bytecode parser. That means that already in the bytecode parser, we decide if we should put an unconditional throw of a LinkageError into the method, or if we should fail the image build with the linking error. At bytecode parsing, we currently do a limited amount of constant folding, of static final fields where the declaring class is initialized at image build time. But there is no method inlining, so in your example the unreachableIsReachable() call remains (side note: if you inline the CNFE == null into main than your branch already goes away during bytecode parsing). Inlining before analysis is able to inline the method (at least after my latest tweak of the heuristics, which was merged last week) and the static analysis then also does not see the Unreachable branch.

What we could do is move the link-at-build-time check from the bytecode parser into the static analysis, i.e., only report linking errors at image build time if they are still present either before or after the static analysis (I'm not sure yet which would be better). There are some difficulties though with this approach: how do we reliably distinguish at that late step if a LinkageError should be reported at build time or not? Or maybe it does not matter, and if a user throws an explicit throw new LinkageError in the source code than that should also be reported at image build time?

In summary, the current behavior is just what we organically grew into when we switched from "always throw all linkage errors at build time" to "allow the user to configure when linkage errors are thrown". It can be changed if there are enough good reasons for it. Which is my final question: are there example where this behavior is a problem in real-world code?

[christianwimmer]

The execution of the static initializer relies on being able to throw a ClassNotFoundException. Which it is not allowed if --link-at-build-time is used.

Paul is incorrect on that. Neither with nor without class intialization at build time, the class initializer would be flagged by --link-at-build-time. Because Class.forName does not throw any kind of LinkageError.

[olpaw]

Because Class.forName does not throw any kind of LinkageError.

Ah, right. Class.forName throws ClassNotFoundException but that is not a subclass of LinkageError. Sorry for that.

[zakkak]

Hi @christianwimmer thanks for the detailed answer.

First, I want to start with my usual disclaimer that --initialize-at-build-time=. is never a good idea.

Without getting in an argument, the Quarkus team finds the benefits of using BTI globally to outweigh the drawbacks and there are currently no plans to move away from build-time-initializing anything that can be initialized at build time.

only report linking errors at image build time if they are still present either before or after the static analysis (I'm not sure yet which would be better).

I intuitively believe that native-image should only report linking errors at image build time if they are still present after the static analysis (or even at a later stage if there is any chance of dropping the code generating the LinkageError later). I.e., only if there is a way to through a LinkageError at runtime when not using --link-at-build-time.

how do we reliably distinguish at that late step if a LinkageError should be reported at build time or not? Or maybe it does not matter, and if a user throws an explicit throw new LinkageError in the source code than that should also be reported at image build time?

That's a good point. Indeed, users will probably get confused if their code fails to build when they throw a LinkageError, especially one they handle.

are there example where this behavior is a problem in real-world code?

We originally hit this issue in a version bump of Vert.x and Netty in Quarkus. Netty uses pretty much the same technique as the reproducer used in this issue to figure out if Brotli4J is available at run time.

See https://github.com/netty/netty/blame/684dfd88e319bb7870d88977bd6a63d5fea765c0/codec/src/main/java/io/netty/handler/codec/compression/Brotli.java#L30-L60

More specifically, in the following code snippet:

    public static boolean isAvailable() {
        return CNFE == null && Brotli4jLoader.isAvailable();
    }

CNFE is set in the static initializer and native-image is able to figure out that CNFE == null is always false at run time and thus there is no need to run Brotli4jLoader.isAvailable() nor to load Brotli4jLoader which leads in a LinkageError.

A similar pattern/issue that we are aware of has been observed in Apache HttpComponents Client, see https://github.com/apache/httpcomponents-client/blob/5f6ad302ba94e039bb2084ac27fbac42044ef211/httpclient5/src/main/java/org/apache/hc/client5/http/entity/BrotliDecompressingEntity.java#L48-L55 and https://github.com/apache/httpcomponents-client/blob/5f6ad302ba94e039bb2084ac27fbac42044ef211/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java#L91-L110