From f8cfeeef534572588315780b33679463a46dabe0 Mon Sep 17 00:00:00 2001 From: Josef Eisl Date: Thu, 12 Sep 2024 17:02:23 +0200 Subject: [PATCH 1/7] svm: annotate DynamicHub#module with @Stable --- substratevm/mx.substratevm/suite.py | 1 + .../src/com/oracle/svm/core/hub/DynamicHub.java | 2 ++ 2 files changed, 3 insertions(+) diff --git a/substratevm/mx.substratevm/suite.py b/substratevm/mx.substratevm/suite.py index 73a8eb0b4e38..0bda22881ea3 100644 --- a/substratevm/mx.substratevm/suite.py +++ b/substratevm/mx.substratevm/suite.py @@ -307,6 +307,7 @@ "jdk.internal.ref", "jdk.internal.reflect", "jdk.internal.vm", + "jdk.internal.vm.annotation", "jdk.internal.util", "jdk.internal.org.objectweb.asm", ], diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java index 48f9ec7ec6aa..0e4af35c34ab 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java @@ -133,6 +133,7 @@ import jdk.internal.reflect.FieldAccessor; import jdk.internal.reflect.Reflection; import jdk.internal.reflect.ReflectionFactory; +import jdk.internal.vm.annotation.Stable; import sun.reflect.annotation.AnnotationType; import sun.reflect.generics.factory.GenericsFactory; import sun.reflect.generics.repository.ClassRepository; @@ -389,6 +390,7 @@ public final class DynamicHub implements AnnotatedElement, java.lang.reflect.Typ private CFunctionPointer[] vtable; /** Field used for module information access at run-time. */ + @Stable // private Module module; /** The class that serves as the host for the nest. All nestmates have the same host. */ From b54b7748840362c3fc282ab92ac3472395826df6 Mon Sep 17 00:00:00 2001 From: Josef Eisl Date: Thu, 12 Sep 2024 09:34:55 +0200 Subject: [PATCH 2/7] svm: no longer disable Module#ensureNativeAccess [JDK-8331671] --- .../oracle/svm/core/jdk/Target_java_lang_Module.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java index 45b81b5211fe..f831a908a7d1 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java @@ -51,15 +51,9 @@ public boolean isNativeAccessEnabled() { @TargetElement(onlyWith = JDK21OrEarlier.class) public native void ensureNativeAccess(Class owner, String methodName); - /** - * Allow all native access. This is a workaround for JDK-8331671 until we have a better - * solution. (GR-57608) - */ - @Substitute + @Alias @TargetElement(onlyWith = JDKLatest.class) - public void ensureNativeAccess(Class owner, String methodName, Class currentClass, boolean jni) { - /* Do Nothing */ - } + public native void ensureNativeAccess(Class owner, String methodName, Class currentClass, boolean jni); @Substitute @BasedOnJDKFile("https://github.com/openjdk/jdk/blob/jdk-24+22/src/hotspot/share/classfile/modules.cpp#L279-L478") From 210168757341973a681490ef913efc4384b0041f Mon Sep 17 00:00:00 2001 From: Josef Eisl Date: Fri, 30 Aug 2024 10:46:22 +0200 Subject: [PATCH 3/7] svm: add InvocationPlugin for Module.enableNativeAccess --- .../hosted/jdk/JDKInitializationFeature.java | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/JDKInitializationFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/JDKInitializationFeature.java index 53e8b4091a61..41f9c71b9ebe 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/JDKInitializationFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/JDKInitializationFeature.java @@ -24,17 +24,31 @@ */ package com.oracle.svm.hosted.jdk; +import java.lang.reflect.Field; + import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.impl.RuntimeClassInitializationSupport; +import com.oracle.svm.core.ParsingReason; import com.oracle.svm.core.TypeResult; import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature; import com.oracle.svm.core.feature.InternalFeature; import com.oracle.svm.hosted.FeatureImpl.AfterRegistrationAccessImpl; import com.oracle.svm.hosted.ImageClassLoader; - +import com.oracle.svm.util.ReflectionUtil; + +import jdk.graal.compiler.nodes.ValueNode; +import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration; +import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderContext; +import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugin; +import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugins; +import jdk.graal.compiler.nodes.util.ConstantFoldUtil; +import jdk.graal.compiler.phases.util.Providers; import jdk.graal.compiler.serviceprovider.JavaVersionUtil; +import jdk.vm.ci.meta.JavaConstant; +import jdk.vm.ci.meta.JavaKind; +import jdk.vm.ci.meta.ResolvedJavaMethod; @AutomaticallyRegisteredFeature public class JDKInitializationFeature implements InternalFeature { @@ -230,4 +244,59 @@ public void afterRegistration(AfterRegistrationAccess access) { currentHolderClass = imageClassLoader.findClass("jdk.internal.foreign.abi.fallback.FallbackLinker$%dHolder".formatted(i++)); } } + + @Override + public void registerInvocationPlugins(Providers providers, GraphBuilderConfiguration.Plugins plugins, ParsingReason reason) { + var enableNativeAccessClass = ReflectionUtil.lookupClass("java.lang.Module$EnableNativeAccess"); + InvocationPlugins.Registration r = new InvocationPlugins.Registration(plugins.getInvocationPlugins(), enableNativeAccessClass); + r.register(new ModuleEnableNativeAccessPlugin()); + } + + /** + * Inlines calls to {@code Module$EnableNativeAccess#isNativeAccessEnabled()} if and only if + * {@code Module#enableNativeAccess} is true. This is ok because the field is {@code @Stable}, + * meaning that a non-default value (i.e., {@code true}, will never change again. Thus, we can + * constant-fold the call to enable optimizations, most importantly dead code elimination. + */ + private static final class ModuleEnableNativeAccessPlugin extends InvocationPlugin.InlineOnlyInvocationPlugin { + + private static final Field ENABLE_NATIVE_ACCESS_FIELD = ReflectionUtil.lookupField(Module.class, "enableNativeAccess"); + + ModuleEnableNativeAccessPlugin() { + super("isNativeAccessEnabled", Module.class); + } + + /** + * See {@code java.lang.Module$EnableNativeAccess#isNativeAccessEnabled(Module target)}. + */ + @Override + public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode targetNode) { + JavaConstant moduleConstant = targetNode.asJavaConstant(); + if (moduleConstant != null) { + var enableNativeAccessField = b.getMetaAccess().lookupJavaField(ENABLE_NATIVE_ACCESS_FIELD); + if (enableNativeAccessField != null) { + var constant = ConstantFoldUtil.tryConstantFold(b.getConstantFieldProvider(), b.getConstantReflection(), b.getMetaAccess(), + enableNativeAccessField, moduleConstant, b.getOptions(), targetMethod); + /* + * ConstantFoldUtil.tryConstantFold adheres to the @Stable field semantics, + * i.e., it only constant folds if the field has a non-default value (in this + * case `true`). See + * jdk.graal.compiler.core.common.spi.JavaConstantFieldProvider# + * readConstantField. In other words, if the field is `false`, `constant` would + * be null. + */ + if (constant != null) { + /* + * Booleans are represented as int on the VM level so checking for int 1 + * instead of boolean true. + */ + assert constant.isJavaConstant() && constant.asJavaConstant().asInt() == 1 : "Must not constant fold if enableNativeAccess is false (@Stable semantics)"; + b.push(JavaKind.Boolean, b.add(constant)); + return true; + } + } + } + return false; + } + } } From bb8133d808f62e786d9d8bdee7f50eaa39649afa Mon Sep 17 00:00:00 2001 From: Josef Eisl Date: Tue, 12 Nov 2024 09:55:37 +0100 Subject: [PATCH 4/7] svm: add --enable-native-access hosted/api option No need to allow native access to the application modules in the driver. They will not be loaded on the boot module layer, so specifying them there has no effect anyways. --- .../core/NativeImageClassLoaderOptions.java | 5 +++++ .../com.oracle.svm.driver/resources/Help.txt | 3 --- .../svm/driver/DefaultOptionHandler.java | 20 ------------------- .../com/oracle/svm/driver/NativeImage.java | 9 ++------- 4 files changed, 7 insertions(+), 30 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/NativeImageClassLoaderOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/NativeImageClassLoaderOptions.java index cae1f4c4b0c7..6f0099706161 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/NativeImageClassLoaderOptions.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/NativeImageClassLoaderOptions.java @@ -48,6 +48,11 @@ public class NativeImageClassLoaderOptions { " can be ALL-UNNAMED to read all unnamed modules.")// public static final HostedOptionKey AddReads = new HostedOptionKey<>(AccumulatingLocatableMultiOptionValue.Strings.build()); + @APIOption(name = "enable-native-access", launcherOption = true, valueSeparator = {APIOption.WHITESPACE_SEPARATOR, '='})// + @Option(help = "A comma-separated list of modules that are permitted to perform restricted native operations." + + " The module name can also be ALL-UNNAMED.")// + public static final HostedOptionKey EnableNativeAccess = new HostedOptionKey<>(AccumulatingLocatableMultiOptionValue.Strings.build()); + @APIOption(name = "list-modules")// @Option(help = "List observable modules and exit.")// public static final HostedOptionKey ListModules = new HostedOptionKey<>(false); diff --git a/substratevm/src/com.oracle.svm.driver/resources/Help.txt b/substratevm/src/com.oracle.svm.driver/resources/Help.txt index d5c4e7a8cd54..f2f24406a48d 100644 --- a/substratevm/src/com.oracle.svm.driver/resources/Help.txt +++ b/substratevm/src/com.oracle.svm.driver/resources/Help.txt @@ -31,9 +31,6 @@ where options include: -J pass directly to the JVM running the image generator --diagnostics-mode enable diagnostics output: class initialization, substitutions, etc. --enable-preview allow classes to depend on preview features of this release - --enable-native-access [,...] - modules that are permitted to perform restricted native operations. - can also be ALL-UNNAMED. --verbose enable verbose output --version print product version and exit --help print this help message diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java index cfd259c77e54..ac574b77c091 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java @@ -47,8 +47,6 @@ class DefaultOptionHandler extends NativeImage.OptionHandler { /* Defunct legacy options that we have to accept to maintain backward compatibility */ private static final String noServerOption = "--no-server"; - private static final String nativeAccessOption = "--enable-native-access"; - DefaultOptionHandler(NativeImage nativeImage) { super(nativeImage); } @@ -130,15 +128,6 @@ public boolean consume(ArgumentQueue args) { args.poll(); nativeImage.addCustomJavaArgs("--enable-preview"); return true; - case nativeAccessOption: - args.poll(); - String modules = args.poll(); - if (modules == null) { - NativeImage.showError(nativeAccessOption + moduleSetModifierOptionErrorMessage); - } - nativeImage.addEnableNativeAccess(modules); - nativeImage.addEnableNativeAccess("org.graalvm.nativeimage.foreign"); - return true; } String singleArgClasspathPrefix = newStyleClasspathOptionName + "="; @@ -212,15 +201,6 @@ public boolean consume(ArgumentQueue args) { nativeImage.addLimitedModules(limitModulesArgs); return true; } - if (headArg.startsWith(nativeAccessOption + "=")) { - args.poll(); - String nativeAccessModules = headArg.substring(nativeAccessOption.length() + 1); - if (nativeAccessModules.isEmpty()) { - NativeImage.showError(headArg + moduleSetModifierOptionErrorMessage); - } - nativeImage.addCustomJavaArgs(headArg + ",org.graalvm.nativeimage.foreign"); - return true; - } return false; } diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java index 2830782dff53..5760be4fad64 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java @@ -305,7 +305,6 @@ private static String oR(OptionKey option) { private final List excludedConfigs = new ArrayList<>(); private final LinkedHashSet addModules = new LinkedHashSet<>(); private final LinkedHashSet limitModules = new LinkedHashSet<>(); - private final LinkedHashSet enableNativeAccessModules = new LinkedHashSet<>(); private long imageBuilderPid = -1; @@ -1381,8 +1380,8 @@ private int completeImageBuild() { if (config.modulePathBuild && !finalImageClasspath.isEmpty()) { imageBuilderJavaArgs.add(DefaultOptionHandler.addModulesOption + "=ALL-DEFAULT"); } - enableNativeAccessModules.addAll(getModulesFromPath(imageBuilderModulePath).keySet()); - assert !enableNativeAccessModules.isEmpty(); + // allow native access for all modules on the image builder module path + var enableNativeAccessModules = getModulesFromPath(imageBuilderModulePath).keySet(); imageBuilderJavaArgs.add("--enable-native-access=" + String.join(",", enableNativeAccessModules)); boolean useColorfulOutput = configureBuildOutput(); @@ -2057,10 +2056,6 @@ public void addLimitedModules(String limitModulesArg) { limitModules.addAll(Arrays.asList(SubstrateUtil.split(limitModulesArg, ","))); } - public void addEnableNativeAccess(String enableNativeAccessArg) { - enableNativeAccessModules.addAll(Arrays.asList(SubstrateUtil.split(enableNativeAccessArg, ","))); - } - void addImageBuilderClasspath(Path classpath) { imageBuilderClasspath.add(canonicalize(classpath)); } From 0cdf55aec867cef1b77f1d3aecef323ccdfdfb6c Mon Sep 17 00:00:00 2001 From: Josef Eisl Date: Tue, 12 Nov 2024 09:57:17 +0100 Subject: [PATCH 5/7] svm: set native access for the image run time modules --- .../oracle/svm/hosted/ModuleLayerFeature.java | 59 +++++++++++++------ .../hosted/NativeImageClassLoaderSupport.java | 33 ++++++++++- 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java index d42553a6112f..4ab506a555df 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java @@ -62,6 +62,7 @@ import org.graalvm.nativeimage.hosted.FieldValueTransformer; import com.oracle.graal.pointsto.meta.AnalysisType; +import com.oracle.svm.core.NativeImageClassLoaderOptions; import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.SubstrateUtil; import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature; @@ -130,27 +131,26 @@ public final class ModuleLayerFeature implements InternalFeature { public void duringSetup(DuringSetupAccess access) { FeatureImpl.DuringSetupAccessImpl accessImpl = (FeatureImpl.DuringSetupAccessImpl) access; moduleLayerFeatureUtils = new ModuleLayerFeatureUtils(accessImpl.imageClassLoader); - - RuntimeModuleSupport.instance().setHostedToRuntimeModuleMapper(moduleLayerFeatureUtils::getOrCreateRuntimeModuleForHostedModule); + RuntimeModuleSupport.instance().setHostedToRuntimeModuleMapper(m -> moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(m, accessImpl)); /* * Register an object replacer that will ensure all references to hosted module instances * are replaced with the appropriate runtime module instance. */ - access.registerObjectReplacer(this::replaceHostedModules); + access.registerObjectReplacer(source -> replaceHostedModules(source, accessImpl)); } - private Object replaceHostedModules(Object source) { + private Object replaceHostedModules(Object source, AnalysisAccessBase access) { if (source instanceof Module module) { - return moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(module); + return moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(module, access); } else if (source instanceof Class clazz) { /* * If the field Class(=DynamicHub).module is not reachable, we do not see all Module * instances directly. So we also need to scan the module in Class/DynamicHub objects. */ - moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(clazz.getModule()); + moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(clazz.getModule(), access); } else if (source instanceof DynamicHub hub) { - moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(hub.getModule()); + moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(hub.getModule(), access); } return source; } @@ -451,7 +451,7 @@ private ModuleLayer synthesizeRuntimeModuleLayer(List parentLayers, runtimeModuleLayer = moduleLayerFeatureUtils.createNewModuleLayerInstance(runtimeModuleLayerConfiguration); Map nameToModule = moduleLayerFeatureUtils.synthesizeNameToModule(accessImpl, runtimeModuleLayer, clf); for (Module syntheticModule : syntheticModules) { - Module runtimeSyntheticModule = moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(syntheticModule); + Module runtimeSyntheticModule = moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(syntheticModule, accessImpl); nameToModule.putIfAbsent(runtimeSyntheticModule.getName(), runtimeSyntheticModule); moduleLayerFeatureUtils.patchModuleLayerField(accessImpl, runtimeSyntheticModule, runtimeModuleLayer); } @@ -538,8 +538,10 @@ private void replicateNativeAccess(AfterAnalysisAccessImpl accessImpl, Set modulesPair : modulePairs.entrySet()) { Module hosted = modulesPair.getKey(); Module runtime = modulesPair.getValue(); - if (moduleLayerFeatureUtils.allowsNativeAccess(hosted)) { - moduleLayerFeatureUtils.setNativeAccess(accessImpl, runtime, true); + if (moduleLayerFeatureUtils.allowsNativeAccess(hosted) || moduleLayerFeatureUtils.isNativeAccessEnabledForRuntimeModule(runtime)) { + if (!moduleLayerFeatureUtils.allowsNativeAccess(runtime)) { + moduleLayerFeatureUtils.enableNativeAccess(accessImpl, runtime); + } } } @@ -630,10 +632,14 @@ private static final class ModuleLayerFeatureUtils { private final Field moduleLayerModulesField; private final Field moduleReferenceLocationField; private final Field moduleReferenceImplLocationField; + private final Set nativeAccessEnabled; ModuleLayerFeatureUtils(ImageClassLoader cl) { runtimeModules = new HashMap<>(); imageClassLoader = cl; + nativeAccessEnabled = NativeImageClassLoaderOptions.EnableNativeAccess.getValue().values().stream().flatMap(m -> Arrays.stream(SubstrateUtil.split(m, ","))) + .collect(Collectors.toSet()); + Method classGetDeclaredMethods0Method = ReflectionUtil.lookupMethod(Class.class, "getDeclaredFields0", boolean.class); try { ModuleSupport.accessModuleByClass(ModuleSupport.Access.OPEN, ModuleLayerFeature.class, Module.class); @@ -694,6 +700,15 @@ private static final class ModuleLayerFeatureUtils { } } + private boolean isNativeAccessEnabledForRuntimeBootLayerModule(String runtimeModuleName) { + return nativeAccessEnabled.contains(runtimeModuleName); + } + + private boolean isNativeAccessEnabledForRuntimeModule(Module runtimeModule) { + String runtimeModuleName = runtimeModule.getName(); + return RuntimeModuleSupport.instance().getBootLayer() == runtimeModule.getLayer() && isNativeAccessEnabledForRuntimeBootLayerModule(runtimeModuleName); + } + /** * A manual field lookup is necessary due to reflection filters present in newer JDK * versions. This method should be removed once {@link ReflectionUtil} becomes immune to @@ -797,7 +812,7 @@ public Module getRuntimeModuleForHostedModule(ClassLoader loader, String hostedM } } - public Module getOrCreateRuntimeModuleForHostedModule(Module hostedModule) { + public Module getOrCreateRuntimeModuleForHostedModule(Module hostedModule, AnalysisAccessBase access) { /* * Special module instances such as ALL_UNNAMED and EVERYONE_MODULE are not replicated * as they only serve as marker modules (all their fields are null, including the loader @@ -806,11 +821,13 @@ public Module getOrCreateRuntimeModuleForHostedModule(Module hostedModule) { if (hostedModule == allUnnamedModule || hostedModule == everyoneModule) { return hostedModule; } else { - return getOrCreateRuntimeModuleForHostedModule(hostedModule.getClassLoader(), hostedModule.getName(), hostedModule.getDescriptor()); + boolean enableNativeAccess = allowsNativeAccess(hostedModule) || isNativeAccessEnabledForRuntimeBootLayerModule(hostedModule.getName()); + return getOrCreateRuntimeModuleForHostedModule(hostedModule.getClassLoader(), hostedModule.getName(), hostedModule.getDescriptor(), access, enableNativeAccess); } } - public Module getOrCreateRuntimeModuleForHostedModule(ClassLoader loader, String hostedModuleName, ModuleDescriptor runtimeModuleDescriptor) { + public Module getOrCreateRuntimeModuleForHostedModule(ClassLoader loader, String hostedModuleName, ModuleDescriptor runtimeModuleDescriptor, AnalysisAccessBase access, + boolean enableNativeAccess) { synchronized (runtimeModules) { Module runtimeModule = getRuntimeModuleForHostedModule(loader, hostedModuleName, true); if (runtimeModule != null) { @@ -828,6 +845,9 @@ public Module getOrCreateRuntimeModuleForHostedModule(ClassLoader loader, String } runtimeModules.putIfAbsent(loader, new HashMap<>()); runtimeModules.get(loader).put(hostedModuleName, runtimeModule); + if (enableNativeAccess) { + enableNativeAccess(access, runtimeModule); + } return runtimeModule; } } @@ -855,7 +875,8 @@ Map synthesizeNameToModule(AnalysisAccessBase access, ModuleLaye ModuleDescriptor descriptor = mref.descriptor(); String name = descriptor.name(); ClassLoader loader = clf.apply(name); - Module m = getOrCreateRuntimeModuleForHostedModule(loader, name, descriptor); + boolean nativeAccess = false; + Module m = getOrCreateRuntimeModuleForHostedModule(loader, name, descriptor, access, nativeAccess); if (!descriptor.equals(m.getDescriptor())) { moduleDescriptorField.set(m, descriptor); access.rescanField(m, moduleDescriptorField); @@ -1123,11 +1144,15 @@ boolean allowsNativeAccess(Module module) { } - void setNativeAccess(AfterAnalysisAccessImpl accessImpl, Module module, boolean value) { + /** + * Allows the given module to perform native access. + */ + void enableNativeAccess(AnalysisAccessBase access, Module module) { + VMError.guarantee(!allowsNativeAccess(module), "Cannot reset native access"); assert moduleEnableNativeAccessField != null : "Only available on JDK19+"; try { - moduleEnableNativeAccessField.set(module, value); - accessImpl.rescanField(module, moduleEnableNativeAccessField); + moduleEnableNativeAccessField.set(module, true); + access.rescanField(module, moduleEnableNativeAccessField); } catch (IllegalAccessException e) { throw VMError.shouldNotReachHere("Failed to reflectively set Module.enableNativeAccess.", e); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java index ca7bc6fdbe3c..2b707166dce3 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java @@ -75,6 +75,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import com.oracle.svm.core.SubstrateUtil; import org.graalvm.collections.EconomicMap; import org.graalvm.collections.EconomicSet; import org.graalvm.collections.MapCursor; @@ -408,7 +409,29 @@ private static void implAddReadsAllUnnamed(Module module) { implAddReadsAllUnnamed.setAccessible(true); implAddReadsAllUnnamed.invoke(module); } catch (ReflectiveOperationException | NoSuchElementException e) { - VMError.shouldNotReachHere("Could reflectively call Module.implAddReadsAllUnnamed", e); + VMError.shouldNotReachHere("Could not reflectively call Module.implAddReadsAllUnnamed", e); + } + } + + static void implAddEnableNativeAccess(Module module) { + try { + Method implAddEnableNativeAccess = Module.class.getDeclaredMethod("implAddEnableNativeAccess"); + ModuleSupport.accessModuleByClass(ModuleSupport.Access.OPEN, NativeImageClassLoaderSupport.class, Module.class); + implAddEnableNativeAccess.setAccessible(true); + implAddEnableNativeAccess.invoke(module); + } catch (ReflectiveOperationException | NoSuchElementException e) { + VMError.shouldNotReachHere("Could not reflectively call Module.implAddEnableNativeAccess", e); + } + } + + static void implAddEnableNativeAccessToAllUnnamed() { + try { + Method implAddEnableNativeAccess = Module.class.getDeclaredMethod("implAddEnableNativeAccessToAllUnnamed"); + ModuleSupport.accessModuleByClass(ModuleSupport.Access.OPEN, NativeImageClassLoaderSupport.class, Module.class); + implAddEnableNativeAccess.setAccessible(true); + implAddEnableNativeAccess.invoke(null); + } catch (ReflectiveOperationException | NoSuchElementException e) { + VMError.shouldNotReachHere("Could not reflectively call Module.implAddEnableNativeAccessToAllUnnamed", e); } } @@ -465,6 +488,14 @@ void processClassLoaderOptions() { } } }); + NativeImageClassLoaderOptions.EnableNativeAccess.getValue(parsedHostedOptions).values().stream().flatMap(m -> Arrays.stream(SubstrateUtil.split(m, ","))).forEach(moduleName -> { + if ("ALL-UNNAMED".equals(moduleName)) { + implAddEnableNativeAccessToAllUnnamed(); + } else { + Module module = findModule(moduleName).orElseThrow(() -> userWarningModuleNotFound(NativeImageClassLoaderOptions.EnableNativeAccess, moduleName)); + implAddEnableNativeAccess(module); + } + }); } private static void warn(String m) { From 68c0f82af5a25abdf5d2590c770186641b3afa3c Mon Sep 17 00:00:00 2001 From: Josef Eisl Date: Thu, 28 Nov 2024 09:38:36 +0100 Subject: [PATCH 6/7] svm: add an @Alias for Module.layer to make it non-final --- .../svm/core/jdk/Target_java_lang_Module.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java index f831a908a7d1..06be0ad32c5a 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_lang_Module.java @@ -25,6 +25,7 @@ package com.oracle.svm.core.jdk; 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 com.oracle.svm.core.annotate.TargetElement; @@ -40,6 +41,19 @@ @SuppressWarnings("unused") @TargetClass(value = java.lang.Module.class) public final class Target_java_lang_Module { + + /** + * {@link Alias} to make {@code Module.layer} non-final. The actual run-time value is set via + * reflection in {@code ModuleLayerFeatureUtils#patchModuleLayerField}, which is called after + * analysis. Thus, we cannot leave it {@code final}, because the analysis might otherwise + * constant-fold the initial {@code null} value. Ideally, we would make it {@code @Stable}, but + * our substitution system currently does not allow this (GR-60154). + */ + @Alias // + @RecomputeFieldValue(isFinal = false, kind = RecomputeFieldValue.Kind.None) + // @Stable (no effect currently GR-60154) + private ModuleLayer layer; + @Substitute @TargetElement(onlyWith = ForeignDisabled.class) @SuppressWarnings("static-method") From b7a35bc51e15e56ca29fbe92bf0731a4205c95c5 Mon Sep 17 00:00:00 2001 From: Josef Eisl Date: Thu, 28 Nov 2024 14:18:42 +0100 Subject: [PATCH 7/7] svm: use ReflectionUtil in NativeImageClassLoaderSupport --- .../hosted/NativeImageClassLoaderSupport.java | 44 +++---------------- 1 file changed, 7 insertions(+), 37 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java index 2b707166dce3..3a23d5551afa 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java @@ -61,7 +61,6 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; -import java.util.NoSuchElementException; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -138,6 +137,10 @@ public final class NativeImageClassLoaderSupport { private final Set> classesToIncludeUnconditionally = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private final Method implAddReadsAllUnnamed = ReflectionUtil.lookupMethod(Module.class, "implAddReadsAllUnnamed"); + private final Method implAddEnableNativeAccess = ReflectionUtil.lookupMethod(Module.class, "implAddEnableNativeAccess"); + private final Method implAddEnableNativeAccessToAllUnnamed = ReflectionUtil.lookupMethod(Module.class, "implAddEnableNativeAccessToAllUnnamed"); + @SuppressWarnings("this-escape") protected NativeImageClassLoaderSupport(ClassLoader defaultSystemClassLoader, String[] classpath, String[] modulePath) { @@ -402,39 +405,6 @@ void serviceProvidersForEach(BiConsumer> action) { serviceProviders.forEach((key, val) -> action.accept(key, Collections.unmodifiableCollection(val))); } - private static void implAddReadsAllUnnamed(Module module) { - try { - Method implAddReadsAllUnnamed = Module.class.getDeclaredMethod("implAddReadsAllUnnamed"); - ModuleSupport.accessModuleByClass(ModuleSupport.Access.OPEN, NativeImageClassLoaderSupport.class, Module.class); - implAddReadsAllUnnamed.setAccessible(true); - implAddReadsAllUnnamed.invoke(module); - } catch (ReflectiveOperationException | NoSuchElementException e) { - VMError.shouldNotReachHere("Could not reflectively call Module.implAddReadsAllUnnamed", e); - } - } - - static void implAddEnableNativeAccess(Module module) { - try { - Method implAddEnableNativeAccess = Module.class.getDeclaredMethod("implAddEnableNativeAccess"); - ModuleSupport.accessModuleByClass(ModuleSupport.Access.OPEN, NativeImageClassLoaderSupport.class, Module.class); - implAddEnableNativeAccess.setAccessible(true); - implAddEnableNativeAccess.invoke(module); - } catch (ReflectiveOperationException | NoSuchElementException e) { - VMError.shouldNotReachHere("Could not reflectively call Module.implAddEnableNativeAccess", e); - } - } - - static void implAddEnableNativeAccessToAllUnnamed() { - try { - Method implAddEnableNativeAccess = Module.class.getDeclaredMethod("implAddEnableNativeAccessToAllUnnamed"); - ModuleSupport.accessModuleByClass(ModuleSupport.Access.OPEN, NativeImageClassLoaderSupport.class, Module.class); - implAddEnableNativeAccess.setAccessible(true); - implAddEnableNativeAccess.invoke(null); - } catch (ReflectiveOperationException | NoSuchElementException e) { - VMError.shouldNotReachHere("Could not reflectively call Module.implAddEnableNativeAccessToAllUnnamed", e); - } - } - protected List modulepath() { return Stream.concat(imagemp.stream(), buildmp.stream()).toList(); } @@ -481,7 +451,7 @@ void processClassLoaderOptions() { }); processOption(NativeImageClassLoaderOptions.AddReads).forEach(val -> { if (val.targetModules.isEmpty()) { - implAddReadsAllUnnamed(val.module); + ReflectionUtil.invokeMethod(implAddReadsAllUnnamed, val.module); } else { for (Module targetModule : val.targetModules) { Modules.addReads(val.module, targetModule); @@ -490,10 +460,10 @@ void processClassLoaderOptions() { }); NativeImageClassLoaderOptions.EnableNativeAccess.getValue(parsedHostedOptions).values().stream().flatMap(m -> Arrays.stream(SubstrateUtil.split(m, ","))).forEach(moduleName -> { if ("ALL-UNNAMED".equals(moduleName)) { - implAddEnableNativeAccessToAllUnnamed(); + ReflectionUtil.invokeMethod(implAddEnableNativeAccessToAllUnnamed, null); } else { Module module = findModule(moduleName).orElseThrow(() -> userWarningModuleNotFound(NativeImageClassLoaderOptions.EnableNativeAccess, moduleName)); - implAddEnableNativeAccess(module); + ReflectionUtil.invokeMethod(implAddEnableNativeAccess, module); } }); }