[GR-66161] Fix native tracing when metadata is missing.

PullRequest: graal/21162

Author: DSouzaM

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/ClassForNameSupport.java

@@ -411,7 +411,7 @@ private Object forName0(String className, ClassLoader classLoader) {
             /* Invalid class names always throw, no need for reflection data */
             return new ClassNotFoundException(className);
         }
-        if (MetadataTracer.Options.MetadataTracingSupport.getValue() && MetadataTracer.singleton().enabled()) {
+        if (MetadataTracer.enabled()) {
             // NB: the early returns above ensure we do not trace calls with bad type args.
             MetadataTracer.singleton().traceReflectionType(className);
         }
@@ -500,6 +500,12 @@ public static Throwable getSavedException(String className) {
      */
     public static boolean canUnsafeInstantiateAsInstance(DynamicHub hub) {
         Class<?> clazz = DynamicHub.toClass(hub);
+        if (MetadataTracer.enabled()) {
+            ConfigurationType type = MetadataTracer.singleton().traceReflectionType(clazz.getName());
+            if (type != null) {
+                type.setUnsafeAllocated();
+            }
+        }
         RuntimeConditionSet conditionSet = null;
         for (var singleton : layeredSingletons()) {
             conditionSet = singleton.unsafeInstantiatedClasses.get(clazz);
@@ -508,12 +514,6 @@ public static boolean canUnsafeInstantiateAsInstance(DynamicHub hub) {
             }
         }
         if (conditionSet != null) {
-            if (MetadataTracer.Options.MetadataTracingSupport.getValue() && MetadataTracer.singleton().enabled()) {
-                ConfigurationType type = MetadataTracer.singleton().traceReflectionType(clazz.getName());
-                if (type != null) {
-                    type.setUnsafeAllocated();
-                }
-            }
             return conditionSet.satisfied();
         }
         return false;

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java

@@ -709,7 +709,7 @@ private ReflectionMetadata reflectionMetadata() {
     }
 
     private void checkClassFlag(int mask, String methodName) {
-        if (MetadataTracer.Options.MetadataTracingSupport.getValue() && MetadataTracer.singleton().enabled()) {
+        if (MetadataTracer.enabled()) {
             traceClassFlagQuery(mask);
         }
         if (throwMissingRegistrationErrors() && !(isClassFlagSet(mask) && getConditions().satisfied())) {
@@ -1315,7 +1315,7 @@ private void checkField(String fieldName, Field field, boolean publicOnly) throw
         boolean throwMissingErrors = throwMissingRegistrationErrors();
         Class<?> clazz = DynamicHub.toClass(this);
 
-        if (MetadataTracer.Options.MetadataTracingSupport.getValue() && MetadataTracer.singleton().enabled()) {
+        if (MetadataTracer.enabled()) {
             traceFieldLookup(fieldName, field, publicOnly);
         }
 
@@ -1397,7 +1397,7 @@ private boolean checkExecutableExists(String methodName, Class<?>[] parameterTyp
         boolean throwMissingErrors = throwMissingRegistrationErrors();
         Class<?> clazz = DynamicHub.toClass(this);
 
-        if (MetadataTracer.Options.MetadataTracingSupport.getValue() && MetadataTracer.singleton().enabled()) {
+        if (MetadataTracer.enabled()) {
             traceMethodLookup(methodName, parameterTypes, method, publicOnly);
         }
 
@@ -1973,14 +1973,19 @@ public DynamicHub arrayType() {
         if (toClass(this) == void.class) {
             throw new UnsupportedOperationException(new IllegalArgumentException());
         }
+        if (MetadataTracer.enabled()) {
+            MetadataTracer.singleton().traceReflectionType(arrayTypeName());
+        }
         if (companion.arrayHub == null) {
-            MissingReflectionRegistrationUtils.reportClassAccess(getTypeName() + "[]");
-        } else if (MetadataTracer.Options.MetadataTracingSupport.getValue() && MetadataTracer.singleton().enabled()) {
-            MetadataTracer.singleton().traceReflectionType(companion.arrayHub.getTypeName());
+            MissingReflectionRegistrationUtils.reportClassAccess(arrayTypeName());
         }
         return companion.arrayHub;
     }
 
+    private String arrayTypeName() {
+        return getTypeName() + "[]";
+    }
+
     @KeepOriginal
     private native Class<?> elementType();
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JavaIOSubstitutions.java

@@ -65,12 +65,12 @@ static ObjectStreamClass lookup(Class<?> cl, boolean all) {
         }
 
         if (Serializable.class.isAssignableFrom(cl) && !cl.isArray()) {
+            if (MetadataTracer.enabled()) {
+                MetadataTracer.singleton().traceSerializationType(cl.getName());
+            }
             if (!DynamicHub.fromClass(cl).isRegisteredForSerialization()) {
                 MissingSerializationRegistrationUtils.reportSerialization(cl);
             }
-            if (MetadataTracer.Options.MetadataTracingSupport.getValue() && MetadataTracer.singleton().enabled()) {
-                MetadataTracer.singleton().traceSerializationType(cl.getName());
-            }
         }
 
         return Target_java_io_ObjectStreamClass_Caches.localDescs0.get(cl);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/JavaLangReflectSubstitutions.java

@@ -387,7 +387,7 @@ private static void set(Object a, int index, Object value) {
     @Substitute
     private static Object newArray(Class<?> componentType, int length)
                     throws NegativeArraySizeException {
-        if (MetadataTracer.Options.MetadataTracingSupport.getValue() && MetadataTracer.singleton().enabled()) {
+        if (MetadataTracer.enabled()) {
             MetadataTracer.singleton().traceReflectionType(componentType.arrayType().getName());
         }
         return KnownIntrinsics.unvalidatedNewArray(componentType, length);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Resources.java

@@ -54,6 +54,7 @@
 import org.graalvm.nativeimage.Platforms;
 import org.graalvm.nativeimage.impl.ConfigurationCondition;
 
+import com.oracle.svm.core.AlwaysInline;
 import com.oracle.svm.core.BuildPhaseProvider;
 import com.oracle.svm.core.ClassLoaderSupport.ConditionWithOrigin;
 import com.oracle.svm.core.MissingRegistrationUtils;
@@ -65,7 +66,6 @@
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
 import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
-import com.oracle.svm.core.jdk.resources.MissingResourceRegistrationError;
 import com.oracle.svm.core.jdk.resources.MissingResourceRegistrationUtils;
 import com.oracle.svm.core.jdk.resources.ResourceExceptionEntry;
 import com.oracle.svm.core.jdk.resources.ResourceStorageEntry;
@@ -428,54 +428,47 @@ private static boolean wasAlreadyInCanonicalForm(String resourceName, String can
         return resourceName.equals(canonicalResourceName) || removeTrailingSlash(resourceName).equals(canonicalResourceName);
     }
 
-    public static ResourceStorageEntryBase getAtRuntime(String name, boolean throwOnMissing) {
-        return getAtRuntime(null, name, throwOnMissing);
+    public static ResourceStorageEntryBase getAtRuntime(String name) {
+        return getAtRuntime(null, name, false);
     }
 
     /**
-     * If {@code throwOnMissing} is false, we have to distinguish an entry that was in the metadata
-     * from one that was not, so the caller can correctly throw the
-     * {@link MissingResourceRegistrationError}. This is needed because different modules can be
-     * tried on the same resource name, causing an unexpected exception if we throw directly.
+     * Looks up a resource from {@code module} with name {@code resourceName}.
+     * <p>
+     * The {@code probe} parameter indicates whether the caller is probing for the existence of a
+     * resource. If {@code probe} is true, failed resource lookups return will not throw missing
+     * registration errors and may instead return {@link #MISSING_METADATA_MARKER}.
+     * <p>
+     * Tracing note: When this method is used for probing, only successful metadata matches will be
+     * traced. If a probing result is {@link #MISSING_METADATA_MARKER}, the caller must explicitly
+     * trace the missing metadata.
      */
-    public static ResourceStorageEntryBase getAtRuntime(Module module, String resourceName, boolean throwOnMissing) {
+    public static ResourceStorageEntryBase getAtRuntime(Module module, String resourceName, boolean probe) {
         VMError.guarantee(ImageInfo.inImageRuntimeCode(), "This function should be used only at runtime.");
         String canonicalResourceName = NativeImageResourcePathRepresentation.toCanonicalForm(resourceName);
         String moduleName = moduleName(module);
         ConditionalRuntimeValue<ResourceStorageEntryBase> entry = getEntry(module, canonicalResourceName);
         if (entry == null) {
             if (MissingRegistrationUtils.throwMissingRegistrationErrors()) {
-                for (var r : layeredSingletons()) {
-                    MapCursor<RequestedPattern, RuntimeConditionSet> cursor = r.requestedPatterns.getEntries();
-                    while (cursor.advance()) {
-                        RequestedPattern moduleResourcePair = cursor.getKey();
-                        if (Objects.equals(moduleName, moduleResourcePair.module) &&
-                                        ((matchResource(moduleResourcePair.resource, resourceName) || matchResource(moduleResourcePair.resource, canonicalResourceName)) &&
-                                                        cursor.getValue().satisfied())) {
-                            return null;
-                        }
-                    }
-
-                    String glob = GlobUtils.transformToTriePath(resourceName, moduleName);
-                    String canonicalGlob = GlobUtils.transformToTriePath(canonicalResourceName, moduleName);
-                    GlobTrieNode<ConditionWithOrigin> globsTrie = r.getResourcesTrieRoot();
-                    if (CompressedGlobTrie.match(globsTrie, glob) ||
-                                    CompressedGlobTrie.match(globsTrie, canonicalGlob)) {
-                        return null;
-                    }
-                    return missingMetadata(module, canonicalGlob, throwOnMissing);
+                if (missingResourceMatchesIncludePattern(resourceName, moduleName) || missingResourceMatchesIncludePattern(canonicalResourceName, moduleName)) {
+                    // This resource name matches a pattern/glob from the provided metadata, but no
+                    // resource with the name actually exists. Do not report missing metadata.
+                    traceResource(resourceName, moduleName);
+                    return null;
                 }
-
-                return missingMetadata(module, resourceName, throwOnMissing);
+                traceResourceMissingMetadata(resourceName, moduleName, probe);
+                return missingMetadata(module, resourceName, probe);
             } else {
+                // NB: Without exact reachability metadata, resource include patterns are not
+                // stored in the image heap, so we cannot reliably identify if the resource was
+                // included at build time. Assume it is missing.
+                traceResourceMissingMetadata(resourceName, moduleName, probe);
                 return null;
             }
         }
-        if (MetadataTracer.Options.MetadataTracingSupport.getValue() && MetadataTracer.singleton().enabled()) {
-            MetadataTracer.singleton().traceResource(resourceName, moduleName);
-        }
+        traceResource(resourceName, moduleName);
         if (!entry.getConditions().satisfied()) {
-            return missingMetadata(module, resourceName, throwOnMissing);
+            return missingMetadata(module, resourceName, probe);
         }
 
         ResourceStorageEntryBase unconditionalEntry = entry.getValue();
@@ -503,6 +496,51 @@ public static ResourceStorageEntryBase getAtRuntime(Module module, String resour
         return unconditionalEntry;
     }
 
+    @AlwaysInline("tracing should fold away when disabled")
+    private static void traceResource(String resourceName, String moduleName) {
+        if (MetadataTracer.enabled()) {
+            MetadataTracer.singleton().traceResource(resourceName, moduleName);
+        }
+    }
+
+    @AlwaysInline("tracing should fold away when disabled")
+    private static void traceResourceMissingMetadata(String resourceName, String moduleName) {
+        traceResourceMissingMetadata(resourceName, moduleName, false);
+    }
+
+    @AlwaysInline("tracing should fold away when disabled")
+    private static void traceResourceMissingMetadata(String resourceName, String moduleName, boolean probe) {
+        if (MetadataTracer.enabled() && !probe) {
+            // Do not trace missing metadata for probing queries, otherwise we'll trace an entry for
+            // every module. The caller is responsible for tracing missing entries if it uses
+            // probing.
+            MetadataTracer.singleton().traceResource(resourceName, moduleName);
+        }
+    }
+
+    /**
+     * Checks whether the given missing resource is matched by a pattern/glob registered at build
+     * time. In such a case, we should not report missing metadata.
+     */
+    private static boolean missingResourceMatchesIncludePattern(String resourceName, String moduleName) {
+        VMError.guarantee(MissingRegistrationUtils.throwMissingRegistrationErrors(), "include patterns are only stored in the image with exact reachability metadata");
+        String glob = GlobUtils.transformToTriePath(resourceName, moduleName);
+        for (var r : layeredSingletons()) {
+            MapCursor<RequestedPattern, RuntimeConditionSet> cursor = r.requestedPatterns.getEntries();
+            while (cursor.advance()) {
+                RequestedPattern moduleResourcePair = cursor.getKey();
+                if (Objects.equals(moduleName, moduleResourcePair.module) && matchResource(moduleResourcePair.resource, resourceName) && cursor.getValue().satisfied()) {
+                    return true;
+                }
+            }
+
+            if (CompressedGlobTrie.match(r.getResourcesTrieRoot(), glob)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private static ConditionalRuntimeValue<ResourceStorageEntryBase> getEntry(Module module, String canonicalResourceName) {
         for (var r : layeredSingletons()) {
             ConditionalRuntimeValue<ResourceStorageEntryBase> entry = r.resources.get(createStorageKey(module, canonicalResourceName));
@@ -513,8 +551,8 @@ private static ConditionalRuntimeValue<ResourceStorageEntryBase> getEntry(Module
         return null;
     }
 
-    private static ResourceStorageEntryBase missingMetadata(Module module, String resourceName, boolean throwOnMissing) {
-        if (throwOnMissing) {
+    private static ResourceStorageEntryBase missingMetadata(Module module, String resourceName, boolean probe) {
+        if (!probe) {
             MissingResourceRegistrationUtils.reportResourceAccess(module, resourceName);
         }
         return MISSING_METADATA_MARKER;
@@ -544,42 +582,45 @@ public static URL createURL(Module module, String resourceName) {
         return urls.hasMoreElements() ? urls.nextElement() : null;
     }
 
-    public static InputStream createInputStream(String resourceName) {
-        return createInputStream(null, resourceName);
-    }
-
     /* Avoid pulling in the URL class when only an InputStream is needed. */
     public static InputStream createInputStream(Module module, String resourceName) {
         if (resourceName == null) {
             return null;
         }
+        ResourceStorageEntryBase entry = findResourceForInputStream(module, resourceName);
+        if (entry == MISSING_METADATA_MARKER) {
+            traceResourceMissingMetadata(resourceName, moduleName(module));
+            MissingResourceRegistrationUtils.reportResourceAccess(module, resourceName);
+            return null;
+        } else if (entry == null) {
+            return null;
+        }
+        List<byte[]> data = entry.getData();
+        return data.isEmpty() ? null : new ByteArrayInputStream(data.get(0));
+    }
 
-        ResourceStorageEntryBase entry = getAtRuntime(module, resourceName, false);
-        boolean isInMetadata = entry != MISSING_METADATA_MARKER;
-        if (moduleName(module) == null && (entry == MISSING_METADATA_MARKER || entry == null)) {
+    private static ResourceStorageEntryBase findResourceForInputStream(Module module, String resourceName) {
+        ResourceStorageEntryBase result = getAtRuntime(module, resourceName, true);
+        if (moduleName(module) == null && (result == MISSING_METADATA_MARKER || result == null)) {
             /*
              * If module is not specified or is an unnamed module and entry was not found as
              * classpath-resource we have to search for the resource in all modules in the image.
              */
             for (Module m : RuntimeModuleSupport.singleton().getBootLayer().modules()) {
-                entry = getAtRuntime(m, resourceName, false);
+                ResourceStorageEntryBase entry = getAtRuntime(m, resourceName, true);
                 if (entry != MISSING_METADATA_MARKER) {
-                    isInMetadata = true;
-                }
-                if (entry != null && entry != MISSING_METADATA_MARKER) {
-                    break;
+                    if (entry != null) {
+                        // resource found
+                        return entry;
+                    } else {
+                        // found a negative query. remember this result but keep trying in case some
+                        // other module supplies an actual resource.
+                        result = null;
+                    }
                 }
             }
         }
-
-        if (!isInMetadata) {
-            MissingResourceRegistrationUtils.reportResourceAccess(module, resourceName);
-        }
-        if (entry == null || entry == MISSING_METADATA_MARKER) {
-            return null;
-        }
-        List<byte[]> data = entry.getData();
-        return data.isEmpty() ? null : new ByteArrayInputStream(data.get(0));
+        return result;
     }
 
     public static Enumeration<URL> createURLs(String resourceName) {
@@ -595,23 +636,24 @@ public static Enumeration<URL> createURLs(Module module, String resourceName) {
 
         List<URL> resourcesURLs = new ArrayList<>();
         String canonicalResourceName = NativeImageResourcePathRepresentation.toCanonicalForm(resourceName);
-        boolean shouldAppendTrailingSlash = hasTrailingSlash(resourceName);
+        if (hasTrailingSlash(resourceName)) {
+            canonicalResourceName += "/";
+        }
 
         /* If moduleName was unspecified we have to consider all modules in the image */
         if (moduleName(module) == null) {
             for (Module m : RuntimeModuleSupport.singleton().getBootLayer().modules()) {
-                ResourceStorageEntryBase entry = getAtRuntime(m, resourceName, false);
-                if (entry == MISSING_METADATA_MARKER) {
-                    continue;
+                ResourceStorageEntryBase entry = getAtRuntime(m, resourceName, true);
+                if (entry != MISSING_METADATA_MARKER) {
+                    missingMetadata = false;
+                    addURLEntries(resourcesURLs, (ResourceStorageEntry) entry, m, canonicalResourceName);
                 }
-                missingMetadata = false;
-                addURLEntries(resourcesURLs, (ResourceStorageEntry) entry, m, shouldAppendTrailingSlash ? canonicalResourceName + '/' : canonicalResourceName);
             }
         }
-        ResourceStorageEntryBase explicitEntry = getAtRuntime(module, resourceName, false);
+        ResourceStorageEntryBase explicitEntry = getAtRuntime(module, resourceName, true);
         if (explicitEntry != MISSING_METADATA_MARKER) {
             missingMetadata = false;
-            addURLEntries(resourcesURLs, (ResourceStorageEntry) explicitEntry, module, shouldAppendTrailingSlash ? canonicalResourceName + '/' : canonicalResourceName);
+            addURLEntries(resourcesURLs, (ResourceStorageEntry) explicitEntry, module, canonicalResourceName);
         }
 
         if (missingMetadata) {