[GR-57608] Support --enable-native-access hosted option in native image

PullRequest: graal/18660

Author: zapster

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",
                 ],

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/NativeImageClassLoaderOptions.java

@@ -48,6 +48,11 @@ public class NativeImageClassLoaderOptions {
                     " <target-module> can be ALL-UNNAMED to read all unnamed modules.")//
     public static final HostedOptionKey<AccumulatingLocatableMultiOptionValue.Strings> 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<AccumulatingLocatableMultiOptionValue.Strings> EnableNativeAccess = new HostedOptionKey<>(AccumulatingLocatableMultiOptionValue.Strings.build());
+
     @APIOption(name = "list-modules")//
     @Option(help = "List observable modules and exit.")//
     public static final HostedOptionKey<Boolean> ListModules = new HostedOptionKey<>(false);

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. */

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")
@@ -51,15 +65,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")

substratevm/src/com.oracle.svm.driver/resources/Help.txt

@@ -31,9 +31,6 @@ where options include:
     -J<flag>              pass <flag> 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 <module name>[,<module name>...]
-                          modules that are permitted to perform restricted native operations.
-                          <module name> can also be ALL-UNNAMED.
     --verbose             enable verbose output
     --version             print product version and exit
     --help                print this help message

substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/DefaultOptionHandler.java

@@ -47,8 +47,6 @@ class DefaultOptionHandler extends NativeImage.OptionHandler<NativeImage> {
     /* 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;
     }
 

substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java

@@ -305,7 +305,6 @@ private static <T> String oR(OptionKey<T> option) {
     private final List<ExcludeConfig> excludedConfigs = new ArrayList<>();
     private final LinkedHashSet<String> addModules = new LinkedHashSet<>();
     private final LinkedHashSet<String> limitModules = new LinkedHashSet<>();
-    private final LinkedHashSet<String> 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));
     }

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<ModuleLayer> parentLayers,
             runtimeModuleLayer = moduleLayerFeatureUtils.createNewModuleLayerInstance(runtimeModuleLayerConfiguration);
             Map<String, Module> 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<Modul
         for (Map.Entry<Module, Module> 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<String> 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<String, Module> 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);
             }