[GR-41674] Class instanceOf and isAssignableFrom checks do need to make the checked type reachable.

[graalvmbot]

#5065 reported an unnecessary imprecision of the static analysis: Just using a type in an instanceof of Class.isAssignableFrom type check makes the type reachable - which implicitly makes the class initializer of the that type reachable too. So the JDK code in /~https://github.com/openjdk/jdk/blob/844a95b907aaf6ef67d7e4b1ed0998945a6152d2/src/java.desktop/share/classes/java/beans/Introspector.java#L1128 makes the class initializer of java.awt.Component reachable, which then pulls in further AWT classes.

The fact that instanceof makes the checked type reachable has historic reasons: it was necessary when we parsed bytecode again after static analysis for AOT compilation. The fix is easy for that case: MethodTypeFlowBuilder just does not make the type reachable for InstanceOfNode.

Handling Class.isAssignableFrom is a bit trickier, since it involves type constants that are embedded in the graph. So we need to recognize the graph structure and ignore the ConstantNode if the only usages are ClassIsAssignableFromNode.

Making fewer types reachable is generally desirable, but can have also surprising side effects. See the Helidon compatibility problem mentioned separately in a comment.

[christianwimmer]

@tomas-langer This PR passes all of our tests except for Helidon MP, where it leads to a strange exception at image run time:

2022.10.14 12:04:51 WARN com.arjuna.ats.common Thread[main,5,main]: ARJUNA048004: attempt to load com.arjuna.ats.internal.arjuna.utils.SocketProcessId threw ClassNotFound. Wrong classloader?
java.lang.ClassNotFoundException: com.arjuna.ats.internal.arjuna.utils.SocketProcessId
	at java.base@11.0.17/java.lang.Class.forName(DynamicHub.java:1135)
	at java.base@11.0.17/java.lang.Class.forName(DynamicHub.java:1108)
	at com.arjuna.common.internal.util.ClassloadingUtility.loadClass(ClassloadingUtility.java:57)
	at com.arjuna.common.internal.util.ClassloadingUtility.loadClass(ClassloadingUtility.java:85)
	at com.arjuna.common.internal.util.ClassloadingUtility.loadAndInstantiateClass(ClassloadingUtility.java:117)
	at com.arjuna.ats.arjuna.common.CoreEnvironmentBean.getProcessImplementation(CoreEnvironmentBean.java:232)
	at com.arjuna.ats.arjuna.utils.Utility.initDefaultProcess(Utility.java:329)
	at com.arjuna.ats.arjuna.utils.Utility.getProcess(Utility.java:340)
	at com.arjuna.ats.arjuna.utils.Utility.getpid(Utility.java:285)
	at com.arjuna.ats.arjuna.common.Uid.<init>(Uid.java:79)
	at com.arjuna.ats.arjuna.utils.Utility.initProcessUid(Utility.java:306)
	at com.arjuna.ats.arjuna.utils.Utility.getProcessUid(Utility.java:297)
	at com.arjuna.ats.internal.arjuna.recovery.TransactionStatusManagerItem.<init>(TransactionStatusManagerItem.java:318)
	at com.arjuna.ats.internal.arjuna.recovery.TransactionStatusManagerItem.createAndSave(TransactionStatusManagerItem.java:75)
	at com.arjuna.ats.arjuna.recovery.TransactionStatusManager.start(TransactionStatusManager.java:135)
	at com.arjuna.ats.arjuna.recovery.TransactionStatusManager.<init>(TransactionStatusManager.java:58)
	at com.arjuna.ats.arjuna.coordinator.TxControl.createTransactionStatusManager(TxControl.java:188)
	at com.arjuna.ats.arjuna.coordinator.TxControl.<clinit>(TxControl.java:264)
        ...

Why the failing code /~https://github.com/jbosstm/narayana/blob/master/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/common/CoreEnvironmentBean.java#L232 is running at image run time with this PR, but not without this PR, is very interesting:

Without this PR, the lazy initialization of processImplementation happens at image build time, with this stack trace:

	  at com.arjuna.ats.arjuna.common.CoreEnvironmentBean.getProcessImplementation(CoreEnvironmentBean.java:228)
	  at com.arjuna.ats.arjuna.utils.Utility.initDefaultProcess(Utility.java:329)
	  at com.arjuna.ats.arjuna.utils.Utility.getProcess(Utility.java:340)
	  at com.arjuna.ats.arjuna.utils.Utility.getpid(Utility.java:285)
	  at com.arjuna.ats.arjuna.common.Uid.<init>(Uid.java:79)
	  at com.arjuna.ats.arjuna.StateManager.<init>(StateManager.java:812)
	  at com.arjuna.ats.arjuna.StateManager.<init>(StateManager.java:801)
	  at com.arjuna.ats.arjuna.coordinator.BasicAction.<init>(BasicAction.java:72)
	  at com.arjuna.ats.arjuna.coordinator.TwoPhaseCoordinator.<init>(TwoPhaseCoordinator.java:55)
	  at com.arjuna.ats.arjuna.AtomicAction.<init>(AtomicAction.java:74)
	  at com.arjuna.ats.internal.jta.recovery.arjunacore.RecoverConnectableAtomicAction.<clinit>(RecoverConnectableAtomicAction.java:38)
	  at com.arjuna.ats.internal.jta.recovery.arjunacore.CommitMarkableResourceRecordRecoveryModule.<clinit>(CommitMarkableResourceRecordRecoveryModule.java:79)
	  at jdk.internal.misc.Unsafe.ensureClassInitialized0(Unsafe.java:-1)
	  at jdk.internal.misc.Unsafe.ensureClassInitialized(Unsafe.java:1044)
	  at com.oracle.svm.hosted.classinitialization.ClassInitializationSupport.ensureClassInitialized(ClassInitializationSupport.java:172)
	  at com.oracle.svm.hosted.classinitialization.ProvenSafeClassInitializationSupport.computeInitKindAndMaybeInitializeClass(ProvenSafeClassInitializationSupport.java:344)
	  at com.oracle.svm.hosted.classinitialization.ProvenSafeClassInitializationSupport.computeInitKindAndMaybeInitializeClass(ProvenSafeClassInitializationSupport.java:75)
	  at com.oracle.svm.hosted.classinitialization.ClassInitializationSupport.maybeInitializeHosted(ClassInitializationSupport.java:163)
	  at com.oracle.svm.hosted.SVMHost.initializeType(SVMHost.java:268)
	  at com.oracle.graal.pointsto.meta.AnalysisUniverse.initializeType(AnalysisUniverse.java:694)
	  at com.oracle.graal.pointsto.meta.AnalysisType.lambda$new$0(AnalysisType.java:293)
	  at com.oracle.graal.pointsto.meta.AnalysisType$$Lambda$365.511651343.run(Unknown Source:-1)
	  at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	  at java.util.concurrent.FutureTask.run(FutureTask.java:264)
	  at com.oracle.graal.pointsto.util.AnalysisFuture.ensureDone(AnalysisFuture.java:63)
	  at com.oracle.graal.pointsto.meta.AnalysisType.ensureInitialized(AnalysisType.java:670)
	  at com.oracle.graal.pointsto.meta.AnalysisType.onReachable(AnalysisType.java:567)
	  at com.oracle.graal.pointsto.meta.AnalysisType$$Lambda$369.1811942924.run(Unknown Source:-1)
	  at com.oracle.graal.pointsto.util.AtomicUtils.atomicMarkAndRun(AtomicUtils.java:52)
	  at com.oracle.graal.pointsto.meta.AnalysisType.lambda$registerAsReachable$7(AnalysisType.java:539)
	  at com.oracle.graal.pointsto.meta.AnalysisType$$Lambda$368.828610686.accept(Unknown Source:-1)
	  at com.oracle.graal.pointsto.meta.AnalysisType.forAllSuperTypes(AnalysisType.java:650)
	  at com.oracle.graal.pointsto.meta.AnalysisType.forAllSuperTypes(AnalysisType.java:636)
	  at com.oracle.graal.pointsto.meta.AnalysisType.forAllSuperTypes(AnalysisType.java:632)
	  at com.oracle.graal.pointsto.meta.AnalysisType.registerAsReachable(AnalysisType.java:539)
	  at com.oracle.graal.pointsto.flow.MethodTypeFlowBuilder.registerUsedElements(MethodTypeFlowBuilder.java:234)
          ...

So it is a side effect of CommitMarkableResourceRecordRecoveryModule being initialized at image build time, and the only reason why it is initialized at image build time is the usage of CommitMarkableResourceRecordRecoveryModule in an instanceof type check at /~https://github.com/jbosstm/narayana/blob/c5f02d07edb34964b64341974ab689ea44536603/ArjunaJTA/jta/classes/com/arjuna/ats/internal/jta/resources/arjunacore/CommitMarkableResourceRecord.java#L138

This PR no longer marks types used only in an instanceof check as reachable - which means with this PR CommitMarkableResourceRecordRecoveryModule is no longer marked as reachable, therefore no longer initialized at build time, and therefore CoreEnvironmentBean.getProcessImplementation no longer does the lazy initialization at build time.

I see a couple of problems in Helidon here:

• The fact that CoreEnvironmentBean.getProcessImplementation works right now is a lucky coincidence and therefore very brittle. An explicit initialization at build time (or an explicit reflection registration so that initialization works at run time) seems warranted.
• Why is CommitMarkableResourceRecordRecoveryModule actually initialized at image build time? Because /~https://github.com/helidon-io/helidon/blob/helidon-3.x/microprofile/cdi/src/main/resources/META-INF/native-image/io.helidon.microprofile.cdi/helidon-microprofile-cdi/native-image.properties#L18 marks all classes as initialize at build time! That means as soon as helidon-microprofile-cdi.jar is on the class path, all classes (including all user application classes that Heldion has no knowledge about) are initialized at build time. I do not think that is intentional, and probably you are not even aware of that.

All of that can be fixed in a future version of Helidon. But how can we merge this PR without breaking all Helidon MP usages that are out there right now?

[AlexanderScherbatiy]

There is the case when building a native image for a JavaFX application pulls additional AWT classes.
The code responsible for it PlatformImpl.java#L964 calls checkForClass("javax.swing.JComponent") method which eventually calls Class.forName(classname, false, PlatformImpl.class.getClassLoader()).

The simple code which reproduces the behavior is:

public class CheckForClass {

    public static void main(String[] args) {
        boolean checkJComponent = checkForClass("javax.swing.JComponent");
        System.out.printf("check for JComponent class: %b%n", checkJComponent);
    }

    private static boolean checkForClass(String classname) {
        try {
            Class.forName(classname, false, CheckForClass.class.getClassLoader());
            return true;
        } catch (ClassNotFoundException cnfe) {
            return false;
        }
    }
}

Building the native image with option --diagnostics-mode

> native-image --no-fallback --diagnostics-mode CheckForClass
> ./checkforclass 
check for JComponent class: true

shows that class_initialization_dependencies_*.dot file contains links:

"java.awt.event.ComponentEvent" -> "java.awt.AWTEvent"
"java.awt.event.InputEvent" -> "java.awt.event.ComponentEvent"
"java.awt.AWTEvent$1" -> "java.lang.Object"
"java.awt.Toolkit$1" -> "java.lang.Object"

Does it mean that the native image includes classes like ComponentEvent and AWTEvent?
Should the fix work for this case as well so additional AWT dependencies are not pulled out?

[christianwimmer]

@AlexanderScherbatiy good find, and thanks for the reproducer. When constant folding the Class.forName, JComponent gets marked as reachable already during bytecode parsing - which is unnecessary because the class constant is already optimized away before the static analysis processes the method.

I'm working on a fix.