diff --git a/compiler/src/jdk.graal.compiler.libgraal/src/jdk/graal/compiler/libgraal/LibGraalSubstitutions.java b/compiler/src/jdk.graal.compiler.libgraal/src/jdk/graal/compiler/libgraal/LibGraalSubstitutions.java index d2d831d18759..bed4b79393c8 100644 --- a/compiler/src/jdk.graal.compiler.libgraal/src/jdk/graal/compiler/libgraal/LibGraalSubstitutions.java +++ b/compiler/src/jdk.graal.compiler.libgraal/src/jdk/graal/compiler/libgraal/LibGraalSubstitutions.java @@ -24,18 +24,28 @@ */ package jdk.graal.compiler.libgraal; +import java.io.IOException; +import java.text.DateFormatSymbols; +import java.time.temporal.TemporalAccessor; +import java.util.Formatter; +import java.util.Locale; + import com.oracle.svm.core.annotate.Alias; import com.oracle.svm.core.annotate.RecomputeFieldValue; +import com.oracle.svm.core.annotate.Substitute; import com.oracle.svm.core.annotate.TargetClass; +import jdk.graal.compiler.debug.GraalError; +import jdk.graal.compiler.debug.PathUtilities; + class LibGraalSubstitutions { @TargetClass(className = "jdk.vm.ci.services.Services", onlyWith = LibGraalFeature.IsEnabled.class) static final class Target_jdk_vm_ci_services_Services { - /* - * Static final boolean field Services.IS_IN_NATIVE_IMAGE is used in many places in the - * JVMCI codebase to switch between the different implementations needed for regular use (a - * built-in module jdk.graal.compiler in the JVM) or as part of libgraal. + /** + * Static final boolean field {@code Services.IS_IN_NATIVE_IMAGE} is used in many places in + * the JVMCI codebase to switch between the different implementations needed for regular use + * (a built-in module {@code jdk.graal.compiler} in the JVM) or as part of libgraal. */ // Checkstyle: stop @Alias // @@ -47,11 +57,190 @@ static final class Target_jdk_vm_ci_services_Services { @TargetClass(className = "jdk.vm.ci.hotspot.Cleaner", onlyWith = LibGraalFeature.IsEnabled.class) static final class Target_jdk_vm_ci_hotspot_Cleaner { - /* - * Make package-private clean() accessible so that it can be called from - * LibGraalEntryPoints.doReferenceHandling(). + /** + * Make package-private {@code clean()} accessible so that it can be called from + * {@link LibGraalSupportImpl#doReferenceHandling()}. */ @Alias public static native void clean(); } + + /** + * There are no String-based class-lookups happening at libgraal runtime. Thus, we can safely + * prune all classloading-logic out of the image. + */ + @TargetClass(value = java.lang.Class.class, onlyWith = LibGraalFeature.IsEnabled.class) + static final class Target_java_lang_Class { + @Substitute + public static Class forName(String name, boolean initialize, ClassLoader loader) + throws ClassNotFoundException { + throw new ClassNotFoundException(name + " (class loading not supported in libgraal)"); + } + + @Substitute + private static Class forName(String className, Class caller) + throws ClassNotFoundException { + throw new ClassNotFoundException(className + " (class loading not supported in libgraal)"); + } + + @Substitute + public static Class forName(Module module, String name) { + return null; + } + } + + @TargetClass(value = java.lang.ClassLoader.class, onlyWith = LibGraalFeature.IsEnabled.class) + static final class Target_java_lang_ClassLoader { + @Substitute + public Class loadClass(String name) throws ClassNotFoundException { + throw new ClassNotFoundException(name + " (class loading not supported in libgraal)"); + } + + @Substitute + static Class findBootstrapClassOrNull(String name) { + return null; + } + } + + @TargetClass(className = "java.util.Formatter$FormatSpecifier", onlyWith = LibGraalFeature.IsEnabled.class) + static final class Target_java_util_Formatter_FormatSpecifier { + + /** + * Custom version of + * {@code java.util.Formatter.FormatSpecifier#localizedMagnitude(java.util.Formatter, java.lang.StringBuilder, java.lang.CharSequence, int, int, int, java.util.Locale)} + * where the given locale is unconditionally replaced with {@code null}). Since the original + * method was already able to accept `null` as locale, the substitution is straightforward. + * The substitution does not contain any code path that requires dynamic class or resource + * lookup. + */ + @Substitute + StringBuilder localizedMagnitude(Formatter fmt, StringBuilder sb, + CharSequence value, final int offset, int f, int width, + Locale unused) { + if (sb == null) { + sb = new StringBuilder(); + } + int begin = sb.length(); + + char zero = '0'; // getZero(l); + + // determine localized grouping separator and size + char grpSep = '\0'; + int grpSize = -1; + char decSep = '\0'; + + int len = value.length(); + int dot = len; + for (int j = offset; j < len; j++) { + if (value.charAt(j) == '.') { + dot = j; + break; + } + } + + if (dot < len) { + decSep = '.'; // getDecimalSeparator(l); + } + + if (Target_java_util_Formatter_Flags.contains(f, Target_java_util_Formatter_Flags.GROUP)) { + grpSep = ','; // getGroupingSeparator(l); + + Locale l = null; + if (l == null || l.equals(Locale.US)) { + grpSize = 3; + } else { + throw GraalError.shouldNotReachHere("localizedMagnitude with l != null"); + } + } + + // localize the digits inserting group separators as necessary + for (int j = offset; j < len; j++) { + if (j == dot) { + sb.append(decSep); + // no more group separators after the decimal separator + grpSep = '\0'; + continue; + } + + char c = value.charAt(j); + sb.append((char) ((c - '0') + zero)); + if (grpSep != '\0' && j != dot - 1 && ((dot - j) % grpSize == 1)) { + sb.append(grpSep); + } + } + + // apply zero padding + if (width > sb.length() && Target_java_util_Formatter_Flags.contains(f, Target_java_util_Formatter_Flags.ZERO_PAD)) { + String zeros = String.valueOf(zero).repeat(width - sb.length()); + sb.insert(begin, zeros); + } + + return sb; + } + + /** + * Custom version of + * {@code java.util.Formatter.FormatSpecifier#print(java.util.Formatter, java.time.temporal.TemporalAccessor, char, java.util.Locale)} + * where the given locale is unconditionally replaced with {@code null}). Since the original + * method was already able to accept `null` as locale, the substitution is straightforward. + * The substitution does not contain any code path that requires dynamic class or resource + * lookup. + */ + @Substitute + void print(Target_java_util_Formatter fmt, TemporalAccessor t, char c, Locale unused) throws IOException { + StringBuilder sb = new StringBuilder(); + print(fmt, sb, t, c, null); + // justify based on width + if (Target_java_util_Formatter_Flags.contains(flags, Target_java_util_Formatter_Flags.UPPERCASE)) { + appendJustified(fmt.a, sb.toString().toUpperCase(Locale.ROOT)); + } else { + appendJustified(fmt.a, sb); + } + } + + @Alias + native Appendable print(Target_java_util_Formatter fmt, StringBuilder sb, TemporalAccessor t, char c, + Locale l) throws IOException; + + @Alias + native void appendJustified(Appendable a, CharSequence cs) throws IOException; + + @Alias // + int flags; + } + + @TargetClass(className = "java.util.Formatter", onlyWith = LibGraalFeature.IsEnabled.class) + static final class Target_java_util_Formatter { + @Alias // + Appendable a; + } + + @TargetClass(className = "java.util.Formatter$Flags", onlyWith = LibGraalFeature.IsEnabled.class) + static final class Target_java_util_Formatter_Flags { + // Checkstyle: stop + @Alias // + static int ZERO_PAD; + @Alias // + static int GROUP; + @Alias // + static int UPPERCASE; + // Checkstyle: resume + + @Alias + static native boolean contains(int flags, int f); + } + + @TargetClass(value = java.text.DateFormatSymbols.class, onlyWith = LibGraalFeature.IsEnabled.class) + static final class Target_java_text_DateFormatSymbols { + /** + * {@link DateFormatSymbols#getInstance(Locale)} relies on String-based class-lookup (to + * find resource bundle {@code sun.text.resources.cldr.FormatData}) which we do not want to + * rely on at libgraal runtime because it increases image size too much. Instead, we return + * the DateFormatSymbols instance that we already have in the image heap. + */ + @Substitute + public static DateFormatSymbols getInstance(Locale unused) { + return PathUtilities.getSharedDateFormatSymbols(); + } + } } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/DebugOptions.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/DebugOptions.java index 8e7372082ebd..f1d130b1ae3a 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/DebugOptions.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/DebugOptions.java @@ -27,10 +27,10 @@ import static jdk.graal.compiler.debug.PathUtilities.createDirectories; import static jdk.graal.compiler.debug.PathUtilities.exists; import static jdk.graal.compiler.debug.PathUtilities.getAbsolutePath; +import static jdk.graal.compiler.debug.PathUtilities.getDateString; import static jdk.graal.compiler.debug.PathUtilities.getPath; import java.io.IOException; -import java.text.SimpleDateFormat; import java.util.Date; import jdk.graal.compiler.options.EnumMultiOptionKey; @@ -333,8 +333,7 @@ public static String getDumpDirectoryName(OptionValues options) { dumpDir = getPath(DumpPath.getValue(options)); } else { Date date = new Date(GraalServices.getGlobalTimeStamp()); - SimpleDateFormat formatter = new SimpleDateFormat("yyyy.MM.dd.HH.mm.ss.SSS"); - dumpDir = getPath(DumpPath.getValue(options), formatter.format(date)); + dumpDir = getPath(DumpPath.getValue(options), getDateString(date)); } dumpDir = getAbsolutePath(dumpDir); return dumpDir; diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/PathUtilities.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/PathUtilities.java index 85ed4c33327a..ecf061c8bc35 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/PathUtilities.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/PathUtilities.java @@ -34,8 +34,12 @@ import java.nio.file.InvalidPathException; import java.nio.file.OpenOption; import java.nio.file.Path; +import java.text.DateFormatSymbols; +import java.text.SimpleDateFormat; +import java.util.Date; import java.util.Iterator; import java.util.ServiceLoader; +import java.util.TimeZone; /** * Miscellaneous methods for modifying and generating file system paths. @@ -85,6 +89,34 @@ public static String getPath(String first, String... more) { return PROVIDER.getPath(first, more); } + /** + * This circumvents instantiating {@link SimpleDateFormat} at libgraal runtime. This is needed + * to avoid String-based class-lookup (to find resource bundle + * {@code sun.text.resources.cldr.FormatData} as part of {@link SimpleDateFormat} construction) + * and allows us to avoid class-lookup support in the image. + */ + private static final SimpleDateFormat SIMPLE_DATE_FORMAT = getSimpleDateFormat(); + + private static SimpleDateFormat getSimpleDateFormat() { + SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy.MM.dd.HH.mm.ss.SSS"); + simpleDateFormat.setTimeZone(TimeZone.getTimeZone("UTC")); + return simpleDateFormat; + } + + public static DateFormatSymbols getSharedDateFormatSymbols() { + // SimpleDateFormat is not thread-safe + synchronized (SIMPLE_DATE_FORMAT) { + return SIMPLE_DATE_FORMAT.getDateFormatSymbols(); + } + } + + public static String getDateString(Date date) { + // SimpleDateFormat is not thread-safe + synchronized (SIMPLE_DATE_FORMAT) { + return SIMPLE_DATE_FORMAT.format(date); + } + } + /** * Gets the absolute pathname of {@code path}. */ diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/printer/GraphPrinterDumpHandler.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/printer/GraphPrinterDumpHandler.java index 9d92d1106fe0..83ba7efe3936 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/printer/GraphPrinterDumpHandler.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/printer/GraphPrinterDumpHandler.java @@ -26,6 +26,7 @@ import static jdk.graal.compiler.debug.DebugConfig.asJavaMethod; import static jdk.graal.compiler.debug.DebugOptions.PrintUnmodifiedGraphs; +import static jdk.graal.compiler.debug.PathUtilities.getDateString; import java.io.IOException; import java.nio.channels.ClosedByInterruptException; @@ -346,7 +347,8 @@ private void openScope(DebugContext debug, String name, int inlineDepth, Map> classLoader() default NoClassLoaderProvider.class; - - /** - * Marker value for {@link #classLoader} that no custom classloader should be used. - * - * @since 24.2 - */ - interface NoClassLoaderProvider extends Supplier { - } - /** * Substitute only if all provided predicates are true (default: unconditional substitution that * is always included). diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py index 4d940e41be50..8b5712db4518 100644 --- a/substratevm/mx.substratevm/mx_substratevm.py +++ b/substratevm/mx.substratevm/mx_substratevm.py @@ -1677,6 +1677,17 @@ def prevent_build_path_in_libgraal(): '-H:+JNIEnhancedErrorCodes', '-H:InitialCollectionPolicy=LibGraal', + # A libgraal image contains classes with the same FQN loaded by different classloaders. + # I.e. the SVM runtime depends on + # - jdk.vm.ci.* classes loaded by the bootstrap classloader + # - jdk.graal.compiler.options.* classes loaded by the platform classloader + # - org.graalvm.collections.* classes loaded by the app classloader + # But potentially different versions of those classes are also loaded by the + # LibGraalClassLoader as part of the classes that libgraal consist of. + # Thus, we cannot use the naive default ClassForName implementation that only + # works if there are no two different classes in the image with the same FQN. + '-H:+ClassForNameRespectsClassLoader', + # Needed for initializing jdk.vm.ci.services.Services.IS_BUILDING_NATIVE_IMAGE. # Remove after JDK-8346781. '-Djdk.vm.ci.services.aot=true', diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java index 8160162bf045..4d0ff4c6d0c2 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java @@ -96,12 +96,6 @@ public boolean getAsBoolean() { } } - private ClassLoader libGraalLoader; - - public void setLibGraalLoader(ClassLoader libGraalLoader) { - this.libGraalLoader = libGraalLoader; - } - @Platforms(Platform.HOSTED_ONLY.class) public static ClassForNameSupport currentLayer() { return LayeredImageSingletonSupport.singleton().lookup(ClassForNameSupport.class, false, true); @@ -201,17 +195,6 @@ public void registerClass(ConfigurationCondition condition, Class clazz, Clas ConditionalRuntimeValue existingEntry = knownClasses.get(name); Object currentValue = existingEntry == null ? null : existingEntry.getValueUnconditionally(); - /* TODO: Remove workaround once GR-53985 is implemented */ - if (currentValue instanceof Class currentClazz && clazz.getClassLoader() != currentClazz.getClassLoader()) { - /* Ensure runtime lookup of LibGraalClassLoader classes */ - if (isLibGraalClass(currentClazz)) { - return; - } - if (isLibGraalClass(clazz)) { - currentValue = null; - } - } - if (currentValue == null || // never seen currentValue == NEGATIVE_QUERY || currentValue == clazz) { @@ -251,11 +234,6 @@ private void addKnownClass(String name, Consumer clazz) { - return libGraalLoader != null && clazz.getClassLoader() == libGraalLoader; - } - public static ConditionalRuntimeValue updateConditionalValue(ConditionalRuntimeValue existingConditionalValue, Object newValue, ConfigurationCondition additionalCondition) { if (existingConditionalValue == null) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ClassLoaderFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ClassLoaderFeature.java index 84f6ffaa7078..576240c97515 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ClassLoaderFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ClassLoaderFeature.java @@ -27,18 +27,14 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; -import org.graalvm.nativeimage.libgraal.hosted.LibGraalLoader; - import com.oracle.graal.pointsto.heap.ImageHeapConstant; import com.oracle.svm.core.BuildPhaseProvider; import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature; import com.oracle.svm.core.feature.InternalFeature; import com.oracle.svm.core.fieldvaluetransformer.FieldValueTransformerWithAvailability; import com.oracle.svm.core.fieldvaluetransformer.ObjectToConstantFieldValueTransformer; -import com.oracle.svm.core.hub.ClassForNameSupport; import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport; import com.oracle.svm.core.util.VMError; -import com.oracle.svm.hosted.FeatureImpl.DuringSetupAccessImpl; import com.oracle.svm.hosted.imagelayer.CrossLayerConstantRegistry; import com.oracle.svm.hosted.jdk.HostedClassLoaderPackageManagement; import com.oracle.svm.util.ReflectionUtil; @@ -123,11 +119,6 @@ public void duringSetup(DuringSetupAccess access) { var config = (FeatureImpl.DuringSetupAccessImpl) access; if (ImageLayerBuildingSupport.firstImageBuild()) { - LibGraalLoader libGraalLoader = ((DuringSetupAccessImpl) access).imageClassLoader.classLoaderSupport.getLibGraalLoader(); - if (libGraalLoader != null) { - ClassLoader libGraalClassLoader = (ClassLoader) libGraalLoader; - ClassForNameSupport.currentLayer().setLibGraalLoader(libGraalClassLoader); - } access.registerObjectReplacer(this::runtimeClassLoaderObjectReplacer); if (ImageLayerBuildingSupport.buildingInitialLayer()) { config.registerObjectReachableCallback(ClassLoader.class, (a1, classLoader, reason) -> { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/AnnotationSubstitutionProcessor.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/AnnotationSubstitutionProcessor.java index 35f036cc28bd..5ebb5d45388c 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/AnnotationSubstitutionProcessor.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/AnnotationSubstitutionProcessor.java @@ -46,7 +46,6 @@ import java.util.function.BooleanSupplier; import java.util.function.Function; import java.util.function.Predicate; -import java.util.function.Supplier; import org.graalvm.nativeimage.AnnotationAccess; import org.graalvm.nativeimage.ImageSingletons; @@ -1107,20 +1106,18 @@ Class findTargetClass(Class annotatedBaseClass, TargetClass target) { protected Class findTargetClass(Class annotatedBaseClass, TargetClass target, boolean checkOnlyWith) { return findTargetClass(TargetClass.class, TargetClass.NoClassNameProvider.class, - annotatedBaseClass, target, target.value(), target.className(), target.classNameProvider(), target.innerClass(), target.classLoader(), + annotatedBaseClass, target, target.value(), target.className(), target.classNameProvider(), target.innerClass(), checkOnlyWith ? target.onlyWith() : null); } protected Class findTargetClass(Class targetClass, Class noClassNameProviderClass, Class annotatedBaseClass, T target, Class value, String targetClassName, Class> classNameProvider, String[] innerClasses, - Class> classloaderSupplier, Class[] onlyWith) { + Class[] onlyWith) { Class holder; String className; - ClassLoader suppliedLoader = null; if (value != targetClass) { guarantee(targetClassName.isEmpty(), "Both class and class name specified for substitution"); guarantee(classNameProvider == noClassNameProviderClass, "Both class and classNameProvider specified for substitution"); - guarantee(classloaderSupplier == TargetClass.NoClassLoaderProvider.class, "Annotation attribute 'classLoader' requires use of 'className' or 'classNameProvider'"); holder = value; className = holder.getName(); @@ -1136,13 +1133,6 @@ protected Class findTargetClass(Class targetClass, Class noClassNam guarantee(!targetClassName.isEmpty(), "Neither class, className, nor classNameProvider specified for substitution"); className = targetClassName; } - if (classloaderSupplier != TargetClass.NoClassLoaderProvider.class) { - try { - suppliedLoader = ReflectionUtil.newInstance(classloaderSupplier).get(); - } catch (ReflectionUtilError ex) { - throw UserError.abort(ex.getCause(), "Cannot instantiate classloaderSupplier: %s. The class must have a parameterless constructor.", classloaderSupplier.getTypeName()); - } - } } if (onlyWith != null) { for (Class onlyWithClass : onlyWith) { @@ -1172,7 +1162,7 @@ protected Class findTargetClass(Class targetClass, Class noClassNam } if (holder == null) { - var substitutionsClassLoaders = suppliedLoader != null ? List.of(suppliedLoader) : imageClassLoader.classLoaderSupport.getClassLoaders(); + var substitutionsClassLoaders = imageClassLoader.classLoaderSupport.getClassLoaders(); for (ClassLoader substitutionsClassLoader : substitutionsClassLoaders) { try { holder = Class.forName(className, false, substitutionsClassLoader);