[GR-33052] Make mx native-unittest work with builder running on modul…

…e-path.

PullRequest: graal/9529

compiler/src/org.graalvm.compiler.options.jdk11/src/org/graalvm/compiler/options/ModuleSupport.java

@@ -28,7 +28,7 @@
 
 public class ModuleSupport {
 
-    static final boolean USE_NI_JPMS = System.getenv().getOrDefault("USE_NATIVE_IMAGE_JAVA_PLATFORM_MODULE_SYSTEM", "false").toLowerCase().equals("true");
+    public static final boolean USE_NI_JPMS = System.getenv().getOrDefault("USE_NATIVE_IMAGE_JAVA_PLATFORM_MODULE_SYSTEM", "false").toLowerCase().equals("true");
 
     static Iterable<OptionDescriptors> getOptionsLoader() {
         /*

compiler/src/org.graalvm.compiler.options/src/org/graalvm/compiler/options/ModuleSupport.java

@@ -28,7 +28,7 @@
 
 public class ModuleSupport {
 
-    static final boolean USE_NI_JPMS;
+    public static final boolean USE_NI_JPMS;
 
     static {
         USE_NI_JPMS = false;

sdk/mx.sdk/suite.py

@@ -366,6 +366,7 @@
           "org.graalvm.nativeimage.c.constant",
           "org.graalvm.nativeimage.c",
           "org.graalvm.nativeimage",
+          "org.graalvm.nativeimage.impl", # Uses of org.graalvm.nativeimage.impl.RuntimeSerializationSupport
           "org.graalvm.nativeimage.impl.clinit", # class initialization instrumentation
           "org.graalvm.polyglot.proxy",
           "org.graalvm.polyglot.io",

substratevm/ci_includes/gate.hocon

@@ -49,7 +49,7 @@ builds += [
       USE_NATIVE_IMAGE_JAVA_PLATFORM_MODULE_SYSTEM : "true"
     }
     run: [
-      ${svm-cmd-gate} ["build,hellomodule"]
+      ${svm-cmd-gate} ["build,hellomodule,test"]
     ]
   }
   ${oraclejdk8} ${svm-common-linux-gate} ${eclipse} ${jdt} ${linux-deploy} {

substratevm/mx.substratevm/macro-junit.properties

@@ -2,6 +2,8 @@
 
 ImageName = svmjunit
 
+ImageClasspath = ${.}/junit-tool.jar:${.}/junit.jar:${.}/hamcrest.jar
+
 Args = -H:Features=com.oracle.svm.junit.JUnitFeature \
        -H:Class=com.oracle.svm.junit.SVMJUnitRunner \
        -H:TestFile=${*} \

substratevm/mx.substratevm/mx_substratevm.py

@@ -465,7 +465,7 @@ def dummy_harness(test_deps, vm_launcher, vm_args):
             mx.abort('No matching unit tests found. Skip image build and execution.')
         with open(unittest_file, 'r') as f:
             mx.log('Building junit image for matching: ' + ' '.join(l.rstrip() for l in f))
-        extra_image_args = mx.get_runtime_jvm_args(unittest_deps, jdk=mx_compiler.jdk)
+        extra_image_args = mx.get_runtime_jvm_args(unittest_deps, jdk=mx_compiler.jdk, exclude_names=['substratevm:LIBRARY_SUPPORT'])
         unittest_image = native_image(['-ea', '-esa'] + build_args + extra_image_args + ['--macro:junit=' + unittest_file, '-H:Path=' + junit_test_dir])
         mx.log('Running: ' + ' '.join(map(pipes.quote, [unittest_image] + run_args)))
         mx.run([unittest_image] + run_args)
@@ -953,7 +953,7 @@ def _native_image_launcher_extra_jvm_args():
     license_files=[],
     third_party_license_files=[],
     dependencies=['SubstrateVM'],
-    builder_jar_distributions=['mx:JUNIT_TOOL', 'mx:JUNIT', 'mx:HAMCREST'],
+    jar_distributions=['mx:JUNIT_TOOL', 'mx:JUNIT', 'mx:HAMCREST'],
     support_distributions=['substratevm:NATIVE_IMAGE_JUNIT_SUPPORT'],
     jlink=False,
 ))

substratevm/mx.substratevm/suite.py

@@ -1,6 +1,6 @@
 # pylint: disable=line-too-long
 suite = {
-    "mxversion": "5.301.0",
+    "mxversion": "5.308.1",
     "name": "substratevm",
     "version" : "21.3.0",
     "release" : False,
@@ -1078,16 +1078,21 @@
             "moduleInfo" : {
                 "name" : "org.graalvm.nativeimage.builder",
                 "exports" : [
+                    "com.oracle.svm.core.configure", # even Feature impls on class-path need access, thus unqualified
+                    "com.oracle.svm.core.jdk", # Uses of com.oracle.svm.core.jdk.StackTraceUtils
+                    "com.oracle.svm.core.snippets", # Uses of com.oracle.svm.core.snippets.KnownIntrinsics
+                    "com.oracle.svm.core", # Uses of com.oracle.svm.core.TypeResult
+                    "com.oracle.svm.core.util", # Uses of com.oracle.svm.core.util.VMError
                     "com.oracle.svm.hosted                        to java.base",
                     "com.oracle.svm.hosted.agent                  to java.instrument",
-                    "com.oracle.svm.core.graal.thread             to jdk.internal.vm.compiler",
-                    "com.oracle.svm.core.classinitialization      to jdk.internal.vm.compiler",
                     "com.oracle.svm.truffle.api                   to org.graalvm.truffle",
-                    "* to org.graalvm.nativeimage.driver,org.graalvm.nativeimage.configure,org.graalvm.nativeimage.librarysupport,org.graalvm.nativeimage.llvm,org.graalvm.nativeimage.agent.jvmtibase,org.graalvm.nativeimage.agent.tracing,org.graalvm.nativeimage.agent.diagnostics,com.oracle.svm.svm_enterprise",
+                    "* to jdk.internal.vm.compiler,org.graalvm.nativeimage.driver,org.graalvm.nativeimage.configure,org.graalvm.nativeimage.librarysupport,org.graalvm.nativeimage.llvm,org.graalvm.nativeimage.agent.jvmtibase,org.graalvm.nativeimage.agent.tracing,org.graalvm.nativeimage.agent.diagnostics,com.oracle.svm.svm_enterprise",
                 ],
                 "opens" : [
                     "com.oracle.svm.core.nodes                    to jdk.internal.vm.compiler",
                     "com.oracle.svm.core.graal.nodes              to jdk.internal.vm.compiler",
+                    "com.oracle.svm.core.graal.snippets           to jdk.internal.vm.compiler",
+                    "com.oracle.svm.hosted.fieldfolding           to jdk.internal.vm.compiler",
                 ],
                 "requires": [
                     "java.management",

substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/SignatureUtil.java

@@ -27,6 +27,11 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.graalvm.nativeimage.hosted.Feature;
+
+import com.oracle.svm.core.annotate.AutomaticFeature;
+import com.oracle.svm.util.ModuleSupport;
+
 import jdk.vm.ci.meta.JavaKind;
 import jdk.vm.ci.meta.MetaUtil;
 
@@ -95,3 +100,13 @@ public static String toInternalClassName(String qualifiedForNameString) {
         return s;
     }
 }
+
+@AutomaticFeature
+class SignatureUtilFeature implements Feature {
+    @Override
+    public void afterRegistration(AfterRegistrationAccess access) {
+        if (!access.getApplicationClassPath().isEmpty()) {
+            ModuleSupport.exportAndOpenPackageToClass("jdk.internal.vm.ci", "jdk.vm.ci.meta", false, SignatureUtil.class);
+        }
+    }
+}

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

@@ -25,7 +25,6 @@
 package com.oracle.svm.driver;
 
 import java.io.File;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.HashSet;
 
@@ -85,19 +84,20 @@ private void applyEnabled(MacroOption.EnabledOption enabledOption, String argume
             enabledOption.forEachPropertyValue(config, "ImageBuilderBootClasspath8", entry -> nativeImage.addImageBuilderBootClasspath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX);
         }
 
-        if (!enabledOption.forEachPropertyValue(config, "ImageBuilderClasspath", entry -> nativeImage.addImageBuilderClasspath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX)) {
-            Path builderJarsDirectory = imageJarsDirectory.resolve("builder");
-            if (Files.isDirectory(builderJarsDirectory)) {
-                NativeImage.getJars(builderJarsDirectory).forEach(nativeImage::addImageBuilderClasspath);
+        enabledOption.forEachPropertyValue(config, "ImageBuilderClasspath", entry -> nativeImage.addImageBuilderClasspath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX);
+
+        boolean explicitImageModulePath = enabledOption.forEachPropertyValue(
+                        config, "ImageModulePath", entry -> nativeImage.addImageModulePath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX);
+        boolean explicitImageClasspath = enabledOption.forEachPropertyValue(
+                        config, "ImageClasspath", entry -> nativeImage.addImageClasspath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX);
+        if (!explicitImageModulePath && !explicitImageClasspath) {
+            if (NativeImage.USE_NI_JPMS) {
+                NativeImage.getJars(imageJarsDirectory).forEach(nativeImage::addImageModulePath);
+            } else {
+                NativeImage.getJars(imageJarsDirectory).forEach(nativeImage::addImageClasspath);
             }
         }
 
-        if (!enabledOption.forEachPropertyValue(config, "ImageClasspath", entry -> nativeImage.addImageClasspath(ClasspathUtils.stringToClasspath(entry)), PATH_SEPARATOR_REGEX)) {
-            NativeImage.getJars(imageJarsDirectory).forEach(nativeImage::addImageClasspath);
-        }
-
-        enabledOption.forEachPropertyValue(config, "ImageModulePath", entry -> nativeImage.addImageModulePath(ClasspathUtils.stringToClasspath(entry), true), PATH_SEPARATOR_REGEX);
-
         String imageName = enabledOption.getProperty(config, "ImageName");
         if (imageName != null) {
             nativeImage.addPlainImageBuilderArg(nativeImage.oHName + imageName);

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

@@ -233,11 +233,11 @@ private static <T> String oR(OptionKey<T> option) {
     private static final String pKeyNativeImageArgs = "NativeImageArgs";
 
     private final ArrayList<String> imageBuilderArgs = new ArrayList<>();
+    private final LinkedHashSet<Path> imageBuilderModulePath = new LinkedHashSet<>();
     private final LinkedHashSet<Path> imageBuilderClasspath = new LinkedHashSet<>();
     private final LinkedHashSet<Path> imageBuilderBootClasspath = new LinkedHashSet<>();
     private final ArrayList<String> imageBuilderJavaArgs = new ArrayList<>();
     private final LinkedHashSet<Path> imageClasspath = new LinkedHashSet<>();
-    private final LinkedHashSet<Path> imageProvidedClasspath = new LinkedHashSet<>();
     private final LinkedHashSet<Path> imageModulePath = new LinkedHashSet<>();
     private final ArrayList<String> customJavaArgs = new ArrayList<>();
     private final ArrayList<String> customImageBuilderArgs = new ArrayList<>();
@@ -564,15 +564,16 @@ public List<Path> getBuilderCLibrariesPaths() {
 
         @Override
         public List<Path> getImageProvidedClasspath() {
-            return getJars(rootDir.resolve(Paths.get("lib", "svm")));
+            return getImageProvidedJars();
         }
 
         @Override
         public List<Path> getImageProvidedModulePath() {
-            if (USE_NI_JPMS) {
-                return getJars(rootDir.resolve(Paths.get("lib", "svm")));
-            }
-            return Collections.emptyList();
+            return getImageProvidedJars();
+        }
+
+        private List<Path> getImageProvidedJars() {
+            return getJars(rootDir.resolve(Paths.get("lib", "svm")));
         }
 
         @Override
@@ -888,19 +889,13 @@ private void prepareImageBuildArgs() {
          */
         addImageBuilderJavaArgs("-Xshare:off");
         config.getBuilderClasspath().forEach(this::addImageBuilderClasspath);
-        config.getImageProvidedClasspath().forEach(this::addImageProvidedClasspath);
 
         if (config.getBuilderInspectServerPath() != null) {
             addPlainImageBuilderArg(oHInspectServerContentPath + config.getBuilderInspectServerPath());
         }
 
         if (config.useJavaModules()) {
-            String modulePath = config.getBuilderModulePath().stream()
-                            .map(this::canonicalize).map(Path::toString)
-                            .collect(Collectors.joining(File.pathSeparator));
-            if (!modulePath.isEmpty()) {
-                addImageBuilderJavaArgs(Arrays.asList("--module-path", modulePath));
-            }
+            config.getBuilderModulePath().forEach(this::addImageBuilderModulePath);
             String upgradeModulePath = config.getBuilderUpgradeModulePath().stream()
                             .map(p -> canonicalize(p).toString())
                             .collect(Collectors.joining(File.pathSeparator));
@@ -1223,22 +1218,25 @@ private int completeImageBuild() {
             showError(leftoverArgs.stream().collect(Collectors.joining(", ", prefix, "")));
         }
 
+        LinkedHashSet<Path> finalImageModulePath = new LinkedHashSet<>(imageModulePath);
         LinkedHashSet<Path> finalImageClasspath = new LinkedHashSet<>(imageBuilderBootClasspath);
         finalImageClasspath.addAll(imageClasspath);
-        if (!finalImageClasspath.isEmpty()) {
-            finalImageClasspath.addAll(imageProvidedClasspath);
-        }
 
-        LinkedHashSet<Path> finalImageModulePath = new LinkedHashSet<>(imageModulePath);
-        if (!finalImageModulePath.isEmpty()) {
-            finalImageModulePath.addAll(config.getImageProvidedModulePath());
+        List<Path> imageProvidedJars;
+        if (USE_NI_JPMS) {
+            imageProvidedJars = config.getImageProvidedModulePath();
+            finalImageModulePath.addAll(imageProvidedJars);
+        } else {
+            imageProvidedJars = config.getImageProvidedClasspath();
+            finalImageClasspath.addAll(imageProvidedJars);
         }
+        imageProvidedJars.forEach(this::processClasspathNativeImageMetaInf);
 
         if (!config.buildFallbackImage() && imageBuilderArgs.contains(oHFallbackThreshold + SubstrateOptions.ForceFallback)) {
             /* Bypass regular build and proceed with fallback image building */
             return 2;
         }
-        return buildImage(imageBuilderJavaArgs, imageBuilderBootClasspath, imageBuilderClasspath, imageBuilderArgs, finalImageClasspath, finalImageModulePath);
+        return buildImage(imageBuilderJavaArgs, imageBuilderBootClasspath, imageBuilderClasspath, imageBuilderModulePath, imageBuilderArgs, finalImageClasspath, finalImageModulePath);
     }
 
     private static String getLocationAgnosticArgPrefix(String argPrefix) {
@@ -1395,7 +1393,8 @@ public void run() {
         }
     }
 
-    protected int buildImage(List<String> javaArgs, LinkedHashSet<Path> bcp, LinkedHashSet<Path> cp, ArrayList<String> imageArgs, LinkedHashSet<Path> imagecp, LinkedHashSet<Path> imagemp) {
+    protected int buildImage(List<String> javaArgs, LinkedHashSet<Path> bcp, LinkedHashSet<Path> cp, LinkedHashSet<Path> mp, ArrayList<String> imageArgs, LinkedHashSet<Path> imagecp,
+                    LinkedHashSet<Path> imagemp) {
         /* Construct ProcessBuilder command from final arguments */
         List<String> command = new ArrayList<>();
         command.add(canonicalize(config.getJavaExecutable()).toString());
@@ -1407,6 +1406,11 @@ protected int buildImage(List<String> javaArgs, LinkedHashSet<Path> bcp, LinkedH
         if (!cp.isEmpty()) {
             command.addAll(Arrays.asList("-cp", cp.stream().map(Path::toString).collect(Collectors.joining(File.pathSeparator))));
         }
+        if (!mp.isEmpty()) {
+            List<String> strings = Arrays.asList("--module-path", mp.stream().map(Path::toString).collect(Collectors.joining(File.pathSeparator)));
+            command.addAll(strings);
+        }
+
         if (USE_NI_JPMS) {
             command.addAll(Arrays.asList("--module", DEFAULT_GENERATOR_MODULE_NAME + "/" + DEFAULT_GENERATOR_CLASS_NAME));
         } else {
@@ -1544,6 +1548,10 @@ Path canonicalize(Path path, boolean strict) {
         }
     }
 
+    public void addImageBuilderModulePath(Path modulePathEntry) {
+        imageBuilderModulePath.add(canonicalize(modulePathEntry));
+    }
+
     void addImageBuilderClasspath(Path classpath) {
         imageBuilderClasspath.add(canonicalize(classpath));
     }
@@ -1602,20 +1610,6 @@ void addPlainImageBuilderArg(String plainArg) {
         imageBuilderArgs.add(plainArg);
     }
 
-    /**
-     * For adding classpath elements that are only on the classpath in the context of native-image
-     * building. I.e. that are not on the classpath when the application would be run with the java
-     * command. (library-support.jar)
-     */
-    private void addImageProvidedClasspath(Path classpath) {
-        VMError.guarantee(imageClasspath.isEmpty() && customImageClasspath.isEmpty());
-        Path classpathEntry = canonicalize(classpath);
-        if (imageProvidedClasspath.add(classpathEntry)) {
-            processManifestMainAttributes(classpathEntry, this::handleClassPathAttribute);
-            processClasspathNativeImageMetaInf(classpathEntry);
-        }
-    }
-
     /**
      * For adding classpath elements that are automatically put on the image-classpath.
      */
@@ -1666,6 +1660,10 @@ LinkedHashSet<String> getBuilderModuleNames() {
         return builderModuleNames;
     }
 
+    void addImageModulePath(Path modulePathEntry) {
+        addImageModulePath(modulePathEntry, true);
+    }
+
     void addImageModulePath(Path modulePathEntry, boolean strict) {
         Path mpEntry;
         try {

substratevm/src/com.oracle.svm.hosted.jdk11/src/com/oracle/svm/hosted/jdk11/NativeImageClassLoaderSupportJDK11OrLater.java

@@ -31,11 +31,13 @@
 import java.lang.module.ModuleFinder;
 import java.lang.module.ModuleReader;
 import java.lang.module.ModuleReference;
+import java.lang.reflect.Method;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -52,6 +54,7 @@
 import com.oracle.svm.core.util.VMError;
 import com.oracle.svm.hosted.AbstractNativeImageClassLoaderSupport;
 import com.oracle.svm.hosted.ImageClassLoader;
+import com.oracle.svm.util.ModuleSupport;
 
 import jdk.internal.module.Modules;
 
@@ -73,6 +76,8 @@ public NativeImageClassLoaderSupportJDK11OrLater(ClassLoader defaultSystemClassL
         adjustBootLayerQualifiedExports(moduleLayer);
         moduleLayerForImageBuild = moduleLayer;
         classLoader = getSingleClassloader(moduleLayer);
+
+        adjustLibrarySupportReadAllUnnamed();
     }
 
     private static ModuleLayer createModuleLayer(Path[] modulePaths, ClassLoader parent) {
@@ -123,9 +128,35 @@ private ClassLoader getSingleClassloader(ModuleLayer moduleLayer) {
         return singleClassloader;
     }
 
+    private void adjustLibrarySupportReadAllUnnamed() {
+        /*
+         * org.graalvm.nativeimage.librarysupport depends on junit. Unfortunately using junit as
+         * automatic module is not possible because we have to support non-modularized tests as
+         * well. Thus, we have to explicitly allow org.graalvm.nativeimage.librarysupport to read
+         * ALL-UNNAMED so that junit from classpath is accessible to it.
+         *
+         * This workaround can be replaced with --add-reads use once GR-33504 is implemented.
+         */
+        Optional<Module> librarySupportModule = findModule("org.graalvm.nativeimage.librarysupport");
+        if (!librarySupportModule.isEmpty()) {
+            try {
+                Module moduleLibrarySupport = librarySupportModule.get();
+                Method implAddReadsAllUnnamed = Module.class.getDeclaredMethod("implAddReadsAllUnnamed");
+                ModuleSupport.openModuleByClass(Module.class, NativeImageClassLoaderSupportJDK11OrLater.class);
+                implAddReadsAllUnnamed.setAccessible(true);
+                implAddReadsAllUnnamed.invoke(moduleLibrarySupport);
+            } catch (ReflectiveOperationException | NoSuchElementException e) {
+                VMError.shouldNotReachHere("Could not adjust org.graalvm.nativeimage.librarysupport to read all unnamed modules", e);
+            }
+        } else {
+            VMError.guarantee(!org.graalvm.compiler.options.ModuleSupport.USE_NI_JPMS,
+                            "Image-builder on module-path requires module org.graalvm.nativeimage.librarysupport");
+        }
+    }
+
     @Override
     protected List<Path> modulepath() {
-        return Stream.concat(buildmp.stream(), imagemp.stream()).collect(Collectors.toList());
+        return Stream.concat(imagemp.stream(), buildmp.stream()).collect(Collectors.toList());
     }
 
     @Override

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/AbstractNativeImageClassLoaderSupport.java

@@ -88,7 +88,7 @@ protected AbstractNativeImageClassLoaderSupport(ClassLoader defaultSystemClassLo
     }
 
     List<Path> classpath() {
-        return Stream.concat(buildcp.stream(), imagecp.stream()).collect(Collectors.toList());
+        return Stream.concat(imagecp.stream(), buildcp.stream()).collect(Collectors.toList());
     }
 
     List<Path> applicationClassPath() {