[GR-40187] Dectect invalid use of SVM specific classes on image class- or module-path.

olpaw · olpaw · commit 90c1f568d629 · 2022-10-18T14:22:09.000Z
PullRequest: graal/12791
diff --git a/java-benchmarks/mx.java-benchmarks/mx_java_benchmarks.py b/java-benchmarks/mx.java-benchmarks/mx_java_benchmarks.py
@@ -371,6 +371,7 @@ def extra_image_build_argument(self, benchmark, args):
                 '-H:EnableURLProtocols=http',
                 '-H:NativeLinkerOption=-no-pie',
                 '-H:-UseServiceLoaderFeature',
+                '-H:+TolerateBuilderClassesOnImageClasspath', # needs to be removed once GR-41746 is fixed
                 '--add-exports=org.graalvm.sdk/org.graalvm.nativeimage.impl=ALL-UNNAMED',
                 '--add-exports=org.graalvm.nativeimage.base/com.oracle.svm.util=ALL-UNNAMED',
                 '--add-exports=org.graalvm.nativeimage.builder/com.oracle.svm.core.configure=ALL-UNNAMED',
diff --git a/substratevm/CHANGELOG.md b/substratevm/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 This changelog summarizes major changes to GraalVM Native Image.
 
+## Version 23.0.0
+* (GR-40187) Report invalid use of SVM specific classes on image class- or module-path as error. As a temporary workaround, -H:+TolerateBuilderClassesOnImageClasspath allows turning the error into a warning.
+
 ## Version 22.3.0
 * (GR-35721) Remove old build output style and the `-H:±BuildOutputUseNewStyle` option.
 * (GR-39390) (GR-39649) (GR-40033) Red Hat added support for the JFR events `JavaMonitorEnter`, `JavaMonitorWait`, and `ThreadSleep`.
diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py
@@ -100,7 +100,18 @@ def svm_unittest_config_participant(config):
 def classpath(args):
     if not args:
         return [] # safeguard against mx.classpath(None) behaviour
-    return mx.classpath(args, jdk=mx_compiler.jdk)
+
+    transitive_excludes = set()
+    def include_in_excludes(dep, dep_edge):
+        # We need to exclude on the granularity of mx.Project entries so that classpath()
+        # can also give us a builder-free classpath if args contains mx.Project entries.
+        if dep.isJavaProject() or dep.isDistribution():
+            transitive_excludes.add(dep)
+
+    implicit_excludes_deps = [mx.dependency(entry) for entry in mx_sdk_vm_impl.NativePropertiesBuildTask.implicit_excludes]
+    mx.walk_deps(implicit_excludes_deps, visit=include_in_excludes)
+    cpEntries = mx.classpath_entries(names=args, includeSelf=True, preferProjects=False, excludes=transitive_excludes)
+    return mx._entries_to_classpath(cpEntries=cpEntries, resolve=True, includeBootClasspath=False, jdk=mx_compiler.jdk, unique=False, ignoreStripped=False)
 
 def platform_name():
     return mx.get_os() + "-" + mx.get_arch()
@@ -614,7 +625,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, exclude_names=['substratevm:LIBRARY_SUPPORT'])
+        extra_image_args = mx.get_runtime_jvm_args(unittest_deps, jdk=mx_compiler.jdk, exclude_names=mx_sdk_vm_impl.NativePropertiesBuildTask.implicit_excludes)
         macro_junit = '--macro:junit'
         if force_builder_on_cp:
             macro_junit += 'cp'
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java
@@ -846,4 +846,7 @@ public Boolean getValueOrDefault(UnmodifiableEconomicMap<OptionKey<?>, Object> v
         }
     };
 
+    @Option(help = "Instead of abort, only warn if image builder classes are found on the image class-path.", type = Debug)//
+    public static final HostedOptionKey<Boolean> TolerateBuilderClassesOnImageClasspath = new HostedOptionKey<>(false);
+
 }
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageClassLoader.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageClassLoader.java
@@ -104,6 +104,7 @@ public void execute(Runnable task) {
             executor.shutdown();
             executor.awaitTermination(CLASS_LOADING_TIMEOUT_IN_MINUTES, TimeUnit.MINUTES);
         }
+        classLoaderSupport.reportBuilderClassesInApplication();
     }
 
     private void findSystemElements(Class<?> systemClass) {
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
@@ -73,6 +73,7 @@
 
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.EconomicSet;
+import org.graalvm.collections.MapCursor;
 import org.graalvm.collections.Pair;
 import org.graalvm.compiler.options.OptionKey;
 import org.graalvm.compiler.options.OptionValues;
@@ -103,6 +104,7 @@ public class NativeImageClassLoaderSupport {
     private final EconomicMap<URI, EconomicSet<String>> classes;
     private final EconomicMap<URI, EconomicSet<String>> packages;
     private final EconomicSet<String> emptySet;
+    private final EconomicSet<URI> builderURILocations;
 
     private final ClassPathClassLoader classPathClassLoader;
     private final ClassLoader classLoader;
@@ -124,6 +126,7 @@ protected NativeImageClassLoaderSupport(ClassLoader defaultSystemClassLoader, St
         classes = EconomicMap.create();
         packages = EconomicMap.create();
         emptySet = EconomicSet.create();
+        builderURILocations = EconomicSet.create();
 
         classPathClassLoader = new ClassPathClassLoader(Util.verifyClassPathAndConvertToURLs(classpath), defaultSystemClassLoader);
 
@@ -142,6 +145,7 @@ protected NativeImageClassLoaderSupport(ClassLoader defaultSystemClassLoader, St
                         .map(Path::of)
                         .map(Util::toRealPath)
                         .collect(Collectors.toUnmodifiableList());
+        buildcp.stream().map(Path::toUri).forEach(builderURILocations::add);
 
         imagemp = Arrays.stream(modulePath)
                         .map(Path::of)
@@ -650,10 +654,16 @@ private void initModule(ModuleReference moduleReference) {
             }
             try (ModuleReader moduleReader = moduleReference.open()) {
                 Module module = optionalModule.get();
+                var container = moduleReference.location().orElseThrow();
+                if (ModuleLayer.boot().equals(module.getLayer())) {
+                    builderURILocations.add(container);
+                }
                 moduleReader.list().forEach(moduleResource -> {
-                    if (moduleResource.endsWith(CLASS_EXTENSION)) {
-                        currentlyProcessedEntry = moduleReferenceLocation + "/" + moduleResource;
-                        executor.execute(() -> handleClassFileName(moduleReference.location().orElseThrow(), module, moduleResource, '/'));
+                    char fileSystemSeparatorChar = '/';
+                    String className = extractClassName(moduleResource, fileSystemSeparatorChar);
+                    if (className != null) {
+                        currentlyProcessedEntry = moduleReferenceLocation + fileSystemSeparatorChar + moduleResource;
+                        executor.execute(() -> handleClassFileName(container, module, className));
                     }
                     entriesProcessed.increment();
                 });
@@ -715,9 +725,10 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) th
                 public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
                     assert !excludes.contains(file.getParent()) : "Visiting file '" + file + "' with excluded parent directory";
                     String fileName = root.relativize(file).toString();
-                    if (fileName.endsWith(CLASS_EXTENSION)) {
+                    String className = extractClassName(fileName, fileSystemSeparatorChar);
+                    if (className != null) {
                         currentlyProcessedEntry = file.toUri().toString();
-                        executor.execute(() -> handleClassFileName(container, null, fileName, fileSystemSeparatorChar));
+                        executor.execute(() -> handleClassFileName(container, null, className));
                     }
                     entriesProcessed.increment();
                     return FileVisitResult.CONTINUE;
@@ -739,35 +750,34 @@ public FileVisitResult visitFileFailed(Path file, IOException exc) {
 
         /**
          * Take a file name from a possibly-multi-versioned jar file and remove the versioning
-         * information. See https://docs.oracle.com/javase/9/docs/api/java/util/jar/JarFile.html for
-         * the specification of the versioning strings.
-         *
+         * information. See
+         * <a href="https://docs.oracle.com/javase/9/docs/api/java/util/jar/JarFile.html">...</a>
+         * for the specification of the versioning strings.
+         * <p>
          * Then, depend on the JDK class loading mechanism to prefer the appropriately-versioned
          * class when the class is loaded. The same class name be loaded multiple times, but each
          * request will return the same appropriately-versioned class. If a higher-versioned class
          * is not available in a lower-versioned JDK, a ClassNotFoundException will be thrown, which
          * will be handled appropriately.
          */
-        private String strippedClassFileName(String fileName) {
-            final String versionedPrefix = "META-INF/versions/";
-            final String versionedSuffix = "/";
+        private String extractClassName(String fileName, char fileSystemSeparatorChar) {
+            if (!fileName.endsWith(CLASS_EXTENSION)) {
+                return null;
+            }
+            String versionedPrefix = "META-INF/versions/";
+            String versionedSuffix = "/";
             String result = fileName;
             if (fileName.startsWith(versionedPrefix)) {
                 final int versionedSuffixIndex = fileName.indexOf(versionedSuffix, versionedPrefix.length());
                 if (versionedSuffixIndex >= 0) {
                     result = fileName.substring(versionedSuffixIndex + versionedSuffix.length());
                 }
             }
-            return result.substring(0, result.length() - CLASS_EXTENSION.length());
+            String strippedClassFileName = result.substring(0, result.length() - CLASS_EXTENSION.length());
+            return strippedClassFileName.equals("module-info") ? null : strippedClassFileName.replace(fileSystemSeparatorChar, '.');
         }
 
-        private void handleClassFileName(URI container, Module module, String fileName, char fileSystemSeparatorChar) {
-            String strippedClassFileName = strippedClassFileName(fileName);
-            if (strippedClassFileName.equals("module-info")) {
-                return;
-            }
-
-            String className = strippedClassFileName.replace(fileSystemSeparatorChar, '.');
+        private void handleClassFileName(URI container, Module module, String className) {
             synchronized (classes) {
                 EconomicSet<String> classNames = classes.get(container);
                 if (classNames == null) {
@@ -800,4 +810,37 @@ private void handleClassFileName(URI container, Module module, String fileName,
             }
         }
     }
+
+    public void reportBuilderClassesInApplication() {
+        EconomicMap<URI, EconomicSet<String>> builderClasses = EconomicMap.create();
+        EconomicMap<URI, EconomicSet<String>> applicationClasses = EconomicMap.create();
+        MapCursor<URI, EconomicSet<String>> classesEntries = classes.getEntries();
+        while (classesEntries.advance()) {
+            var destinationMap = builderURILocations.contains(classesEntries.getKey()) ? builderClasses : applicationClasses;
+            destinationMap.put(classesEntries.getKey(), classesEntries.getValue());
+        }
+        boolean tolerateViolations = SubstrateOptions.TolerateBuilderClassesOnImageClasspath.getValue(parsedHostedOptions);
+        MapCursor<URI, EconomicSet<String>> applicationClassesEntries = applicationClasses.getEntries();
+        while (applicationClassesEntries.advance()) {
+            var applicationClassContainer = applicationClassesEntries.getKey();
+            for (String applicationClass : applicationClassesEntries.getValue()) {
+                MapCursor<URI, EconomicSet<String>> builderClassesEntries = builderClasses.getEntries();
+                while (builderClassesEntries.advance()) {
+                    var builderClassContainer = builderClassesEntries.getKey();
+                    if (builderClassesEntries.getValue().contains(applicationClass)) {
+                        String message = String.format("Class-path entry %s contains class %s. This class is part of the image builder itself (in %s) and must not be passed via -cp.",
+                                        applicationClassContainer, applicationClass, builderClassContainer);
+                        if (!tolerateViolations) {
+                            String errorMessage = String.join(" ", message,
+                                            "This can be caused by a fat-jar that illegally includes svm.jar (or graal-sdk.jar) due to its build-time dependency on it.",
+                                            "As a temporary workaround, %s allows turning this error into a warning.");
+                            throw UserError.abort(errorMessage, SubstrateOptionsParser.commandArgument(SubstrateOptions.TolerateBuilderClassesOnImageClasspath, "+"));
+                        } else {
+                            System.out.println("Warning: " + message);
+                        }
+                    }
+                }
+            }
+        }
+    }
 }
diff --git a/substratevm/src/com.oracle.svm.truffle.nfi.posix/src/com/oracle/svm/truffle/nfi/posix/PosixTruffleNFIFeature.java b/substratevm/src/com.oracle.svm.truffle.nfi.posix/src/com/oracle/svm/truffle/nfi/posix/PosixTruffleNFIFeature.java
@@ -110,7 +110,7 @@ private static PointerBase dlmopen(Lmid_t lmid, String filename, int mode) {
     @Platforms(Platform.LINUX.class)
     private static PointerBase loadLibraryInNamespace(long nativeContext, String name, int mode) {
         assert (mode & isolatedNamespaceFlag) == 0;
-        Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextLinux context = SubstrateUtil.cast(getContext(nativeContext), Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextLinux.class);
+        var context = SubstrateUtil.cast(getContext(nativeContext), Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Linux.class);
 
         // Double-checked locking on the NFI context instance.
         long namespaceId = context.isolatedNamespaceId;
diff --git a/substratevm/src/com.oracle.svm.truffle.nfi.posix/src/com/oracle/svm/truffle/nfi/posix/Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Linux.java b/substratevm/src/com.oracle.svm.truffle.nfi.posix/src/com/oracle/svm/truffle/nfi/posix/Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Linux.java
@@ -34,15 +34,15 @@
 
 @Platforms(Platform.LINUX.class)
 @TargetClass(className = "com.oracle.truffle.nfi.backend.libffi.LibFFIContext", onlyWith = TruffleNFIFeature.IsEnabled.class)
-final class Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextLinux {
+final class Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Linux {
 
     // Checkstyle: stop
     @Alias volatile long isolatedNamespaceId;
 
     @Alias @InjectAccessors(IsolatedAccessor.class) int ISOLATED_NAMESPACE;
 
     static class IsolatedAccessor {
-        static int getISOLATED_NAMESPACE(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextLinux ctx) {
+        static int getISOLATED_NAMESPACE(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Linux ctx) {
             return PosixTruffleNFISupport.isolatedNamespaceFlag;
         }
     }
diff --git a/substratevm/src/com.oracle.svm.truffle.nfi.posix/src/com/oracle/svm/truffle/nfi/posix/Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix.java b/substratevm/src/com.oracle.svm.truffle.nfi.posix/src/com/oracle/svm/truffle/nfi/posix/Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix.java
@@ -35,24 +35,24 @@
 
 @TargetClass(className = "com.oracle.truffle.nfi.backend.libffi.LibFFIContext", onlyWith = TruffleNFIFeature.IsEnabled.class)
 @Platforms({Platform.LINUX.class, Platform.DARWIN.class})
-final class Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix {
+final class Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix {
 
     // Checkstyle: stop
     static class RTLDAccessor {
 
-        static int getRTLD_GLOBAL(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix ctx) {
+        static int getRTLD_GLOBAL(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix ctx) {
             return Dlfcn.RTLD_GLOBAL();
         }
 
-        static int getRTLD_LOCAL(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix ctx) {
+        static int getRTLD_LOCAL(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix ctx) {
             return Dlfcn.RTLD_LOCAL();
         }
 
-        static int getRTLD_LAZY(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix ctx) {
+        static int getRTLD_LAZY(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix ctx) {
             return Dlfcn.RTLD_LAZY();
         }
 
-        static int getRTLD_NOW(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix ctx) {
+        static int getRTLD_NOW(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix ctx) {
             return Dlfcn.RTLD_NOW();
         }
     }
diff --git a/vm/mx.vm/mx_vm_gate.py b/vm/mx.vm/mx_vm_gate.py
@@ -476,7 +476,7 @@ def build_tests_image(image_dir, options, unit_tests=None, additional_deps=None,
 
         if additional_deps:
             build_deps = build_deps + additional_deps
-        extra_image_args = mx.get_runtime_jvm_args(build_deps, jdk=mx_compiler.jdk)
+        extra_image_args = mx.get_runtime_jvm_args(build_deps, jdk=mx_compiler.jdk, exclude_names=mx_sdk_vm_impl.NativePropertiesBuildTask.implicit_excludes)
         tests_image = native_image(build_options + extra_image_args)
         import configparser
         artifacts = configparser.RawConfigParser(allow_no_value=True)

Original file line number	Diff line number	Diff line change
`@@ -846,4 +846,7 @@ public Boolean getValueOrDefault(UnmodifiableEconomicMap<OptionKey<?>, Object> v`
`846`	`846`	`}`
`847`	`847`	`};`
`848`	`848`
	`849`	`+ @Option(help = "Instead of abort, only warn if image builder classes are found on the image class-path.", type = Debug)//`
	`850`	`+ public static final HostedOptionKey<Boolean> TolerateBuilderClassesOnImageClasspath = new HostedOptionKey<>(false);`
	`851`	`+`
`849`	`852`	`}`
Original file line number	Diff line number	Diff line change
`@@ -104,6 +104,7 @@ public void execute(Runnable task) {`
`104`	`104`	`executor.shutdown();`
`105`	`105`	`executor.awaitTermination(CLASS_LOADING_TIMEOUT_IN_MINUTES, TimeUnit.MINUTES);`
`106`	`106`	`}`
	`107`	`+ classLoaderSupport.reportBuilderClassesInApplication();`
`107`	`108`	`}`
`108`	`109`
`109`	`110`	`private void findSystemElements(Class<?> systemClass) {`
Original file line number	Diff line number	Diff line change
`@@ -34,15 +34,15 @@`
`34`	`34`
`35`	`35`	`@Platforms(Platform.LINUX.class)`
`36`	`36`	`@TargetClass(className = "com.oracle.truffle.nfi.backend.libffi.LibFFIContext", onlyWith = TruffleNFIFeature.IsEnabled.class)`
`37`		`-final class Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextLinux {`
	`37`	`+final class Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Linux {`
`38`	`38`
`39`	`39`	`// Checkstyle: stop`
`40`	`40`	`@Alias volatile long isolatedNamespaceId;`
`41`	`41`
`42`	`42`	`@Alias @InjectAccessors(IsolatedAccessor.class) int ISOLATED_NAMESPACE;`
`43`	`43`
`44`	`44`	`static class IsolatedAccessor {`
`45`		`- static int getISOLATED_NAMESPACE(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextLinux ctx) {`
	`45`	`+ static int getISOLATED_NAMESPACE(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Linux ctx) {`
`46`	`46`	`return PosixTruffleNFISupport.isolatedNamespaceFlag;`
`47`	`47`	`}`
`48`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,24 +35,24 @@`
`35`	`35`
`36`	`36`	`@TargetClass(className = "com.oracle.truffle.nfi.backend.libffi.LibFFIContext", onlyWith = TruffleNFIFeature.IsEnabled.class)`
`37`	`37`	`@Platforms({Platform.LINUX.class, Platform.DARWIN.class})`
`38`		`-final class Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix {`
	`38`	`+final class Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix {`
`39`	`39`
`40`	`40`	`// Checkstyle: stop`
`41`	`41`	`static class RTLDAccessor {`
`42`	`42`
`43`		`- static int getRTLD_GLOBAL(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix ctx) {`
	`43`	`+ static int getRTLD_GLOBAL(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix ctx) {`
`44`	`44`	`return Dlfcn.RTLD_GLOBAL();`
`45`	`45`	`}`
`46`	`46`
`47`		`- static int getRTLD_LOCAL(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix ctx) {`
	`47`	`+ static int getRTLD_LOCAL(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix ctx) {`
`48`	`48`	`return Dlfcn.RTLD_LOCAL();`
`49`	`49`	`}`
`50`	`50`
`51`		`- static int getRTLD_LAZY(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix ctx) {`
	`51`	`+ static int getRTLD_LAZY(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix ctx) {`
`52`	`52`	`return Dlfcn.RTLD_LAZY();`
`53`	`53`	`}`
`54`	`54`
`55`		`- static int getRTLD_NOW(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContextPosix ctx) {`
	`55`	`+ static int getRTLD_NOW(@SuppressWarnings("unused") Target_com_oracle_truffle_nfi_backend_libffi_LibFFIContext_Posix ctx) {`
`56`	`56`	`return Dlfcn.RTLD_NOW();`
`57`	`57`	`}`
`58`	`58`	`}`