sun.java2d.cmm.lcms.LCMS#setTagDataNative substitution is broken on JDK17

[cstancu]

Describe the bug

On JDK17 sun.java2d.cmm.lcms.LCMS#setTagDataNative is declared as static. Prior to JDK17 this method was not static so now the Quarkus substitution leads to a build time error:

Error: Static modifier mismatch: private void io.quarkus.runtime.graal.Target_sun_java2d_cmm_lcms_LCMS.setTagDataNative(long,int,byte[]), static native void sun.java2d.cmm.lcms.LCMS.setTagDataNative(long,int,byte[])
com.oracle.svm.core.util.UserError$UserException: Static modifier mismatch: private void io.quarkus.runtime.graal.Target_sun_java2d_cmm_lcms_LCMS.setTagDataNative(long,int,byte[]), static native void sun.java2d.cmm.lcms.LCMS.setTagDataNative(long,int,byte[])
	at com.oracle.svm.core.util.UserError.abort(UserError.java:68)
	at com.oracle.svm.core.util.UserError.guarantee(UserError.java:96)
	at com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor.findOriginalMethod(AnnotationSubstitutionProcessor.java:725)
	at com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor.handleMethodInAliasClass(AnnotationSubstitutionProcessor.java:368)
	at com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor.handleAliasClass(AnnotationSubstitutionProcessor.java:340)
	at com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor.handleClass(AnnotationSubstitutionProcessor.java:310)
	at com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor.init(AnnotationSubstitutionProcessor.java:266)
	at com.oracle.svm.hosted.NativeImageGenerator.createDeclarativeSubstitutionProcessor(NativeImageGenerator.java:941)
	at com.oracle.svm.hosted.NativeImageGenerator.setupNativeImage(NativeImageGenerator.java:873)
	at com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:531)
	at com.oracle.svm.hosted.NativeImageGenerator.run(NativeImageGenerator.java:492)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner.buildImage(NativeImageGeneratorRunner.java:400)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner.build(NativeImageGeneratorRunner.java:566)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner.main(NativeImageGeneratorRunner.java:122)
	at com.oracle.svm.hosted.NativeImageGeneratorRunner$JDK9Plus.main(NativeImageGeneratorRunner.java:596)

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

This is currently reproducible only with JDK17 GraalVM builds.

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

The substitution should be version specific with an alternative declaration for JDK17 using onlyWith = JDK17OrLater.class.

[jaikiran]

Hello @cstancu,

I had a quick look at the Quarkus substitution class for this one https://github.com/quarkusio/quarkus/blob/main/core/runtime/src/main/java/io/quarkus/runtime/graal/LCMSSubstitutions.java#L8 and it doesn't seem straightforward to add some kind of logic which will allow rest of the substitution methods in that class be used in all Java versions, except for the setTagDataNative.

Substitution classes can't be extended either (and are mandated to be final). So I can't have a common base class for the rest of the substituted methods in there.

So I guess we just have to have 2 classes with all those substitutions copied over (and maintained) and mark one of those as @TargetClass(onlyWith=..... While I was at it, I was thinking perhaps instead of having the onlyWith usage, we could instead have a multi-release jar (in Quarkus) which then has Java 17 specific class which substitutes sun.java2d.cmm.lcms.LCMS, but looking at how substratevm "picks up" the classes from the class path (essentially file path traversal over zipfs)[1], I don't think multi-release jars (containing the substitutions) are supported in Graal VM. Is that right?

[1] https://github.com/oracle/graal/blob/master/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/AbstractNativeImageClassLoaderSupport.java#L213

[jaikiran]

Furthermore, I see that GraalVM itself has certain registrations it does for JNI for this specific sun.java2d.cmm.lcms.LCMS here https://github.com/oracle/graal/blob/master/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/JNIRegistrationAwt.java#L71. Should GraalVM itself be then doing any substitutions required for this class and perhaps Quarkus can completely remove these substitutions for this class going forward?

[gsmet]

I worked on #19534 last week but forgot to push it.

I think we should probably reconsider if these substitutions are even useful but it's gonna need some cautious work as we need to make sure our images don't become significantly larger for whatever reason.

So for now, I would be in favor of fixing the issue quickly so that we can benefit again from the GraalVM team testing us with GraalVM JDK 17.

Does it make sense?

[gsmet]

BTW, there are more changes than just the one mentioned here. I tried to catch them all but wasn't able to test things. I hope @zakkak will be able to help on this.

[jaikiran]

So for now, I would be in favor of fixing the issue quickly so that we can benefit again from the GraalVM team testing us with GraalVM JDK 17.

Does it make sense?

Yes, that sounds fine to me.

[gemmellr]

The changes for this break compatibility with GraalVM 20.3, as it lacks the JDK16OrEarlier check class. If thats expected its ok, but maybe it should be dropped from the 'supported versions' check so it fails in a more obvious way?

[zakkak]

@gemmellr this issue is fixed by #19647 which is expected to get merged soon!

[gemmellr]

Ah, actually I see this is likely being covered already in #19645

[gemmellr]

heh, ninja'd...yep, I didnt see it as I searched for graal and it didnt hit