[GR-41098] HashMap crashes on types that indirectly implement Comparable but aren't comparable

[kb-1000]

Describe the issue
HashMap (or ConcurrentHashMap).comparableClassFor does a check whether or not a class C implements exactly Comparable<C>, without recursively checking any superclasses or superinterface. This check may or may not be incorrect (not my call to make), but on native-image the behavior is overridden (in JavaUtilSubstitutions) to only check if C is assignable to Comparable, causing some code that works on a regular JVM to break on native-image. For example, DataFixerUpper as used in Minecraft relies on this behavior, causing the last entry in https://github.com/hpi-swa/native-minecraft-server/tree/b7300a343e213884135d314a67ac74c76dd8af3f#limitations-and-known-issues and the same issue in my own project for this.

Is disabling that generic check even still necessary? Don't we have generic info at runtime now?

Steps to reproduce the issue
Please include both build steps as well as run steps

1. Download HashMapTest.java
2. mvn dependency:get -Dartifact=org.apache.commons:commons-lang3:3.12.0
3. javac -cp ~/.m2/repository/org/apache/commons/commons-lang3/3.12.0/commons-lang3-3.12.0.jar HashMapTest.java
4. native-image -cp ~/.m2/repository/org/apache/commons/commons-lang3/3.12.0/commons-lang3-3.12.0.jar:. HashMapTest
5. ./hashmaptest

Describe GraalVM and your environment:

• GraalVM version (latest snapshot builds can be found here), or commit id if built from source: [e.g. EE 19.3, CE 20.0, CE 20.1.0-dev-20200304_0848] CE 22.2.0, EE 22.2.0
• JDK major version: [e.g.:8] 17
• OS: [e.g. macOS Catalina] Linux (Fedora 36)
• Architecture: [e.g.: AMD64] AMD64

More details
Consider adding the --native-image-info and --verbose flags when building your native image and paste output below.

Add any other information about the problem here. Especially important are stack traces or log output. Feel free to link to gists or to screenshots if necessary.

Exception in thread "main" java.lang.ClassCastException: java.util.ImmutableCollections$List12 cannot be cast to java.lang.Comparable
        at org.apache.commons.lang3.builder.CompareToBuilder.append(CompareToBuilder.java:426)
        at org.apache.commons.lang3.builder.CompareToBuilder.append(CompareToBuilder.java:377)
        at org.apache.commons.lang3.tuple.Pair.compareTo(Pair.java:133)
        at org.apache.commons.lang3.tuple.Pair.compareTo(Pair.java:41)
        at java.util.HashMap.compareComparables(HashMap.java:371)
        at java.util.HashMap$TreeNode.treeify(HashMap.java:2086)
        at java.util.HashMap.treeifyBin(HashMap.java:770)
        at java.util.HashMap.putVal(HashMap.java:642)
        at java.util.HashMap.put(HashMap.java:610)
        at HashMapTest.main(HashMapTest.java:9)

[fniephaus]

Thanks for the detailed analysis and the reproducer! It seems that the substitutions for comparableClassFor() are rather old and today, we can have the necessary metadata available at run time, that makes this work. Although the PR for fixing this is simple as it only removes the two substitution methods (see below), the change is will be merged after the 22.3 release branch off and thus will hopefully ship in 23.0. However, it should soon be possible to test it using a dev build.

graal/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JavaUtilSubstitutions.java

Lines 139 to 172 in c0eaabb

	@Substitute
	private static Class<?> comparableClassFor(Object x) {
	if (x instanceof Comparable) {
	/*
	* We cannot do all the generic interface checks that the original implementation is
	* doing, because we do not have the necessary metadata at run time.
	*/
	return x.getClass();
	}
	return null;
	}
	}
	@TargetClass(java.util.concurrent.ConcurrentHashMap.class)
	final class Target_java_util_concurrent_ConcurrentHashMap {
	@Alias @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset)//
	Target_java_util_concurrent_ConcurrentHashMap_KeySetView keySet;
	@Alias @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset)//
	Target_java_util_concurrent_ConcurrentHashMap_ValuesView values;
	@Alias @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset)//
	Target_java_util_concurrent_ConcurrentHashMap_EntrySetView entrySet;
	@Substitute
	private static Class<?> comparableClassFor(Object x) {
	if (x instanceof Comparable) {
	/*
	* We cannot do all the generic interface checks that the original implementation is
	* doing, because we do not have the necessary metadata at run time.
	*/
	return x.getClass();
	}
	return null;
	}

[fniephaus]

f1f1c0c fixes this and will ship with the 23.0 release.