[GR-42996] Refactor shadow heap verification and fix lazy scanned values.

PullRequest: graal/15503

Author: cstancu

substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/heap/StandaloneHeapSnapshotVerifier.java

@@ -31,6 +31,7 @@
 import com.oracle.graal.pointsto.heap.HeapSnapshotVerifier;
 import com.oracle.graal.pointsto.heap.ImageHeap;
 import com.oracle.graal.pointsto.heap.ImageHeapScanner;
+import com.oracle.graal.pointsto.infrastructure.UniverseMetaAccess;
 import com.oracle.graal.pointsto.standalone.StandaloneObjectScanner;
 import com.oracle.graal.pointsto.util.CompletionExecutor;
 
@@ -40,7 +41,7 @@ public StandaloneHeapSnapshotVerifier(BigBang bb, ImageHeap imageHeap, ImageHeap
     }
 
     @Override
-    protected ObjectScanner installObjectScanner(CompletionExecutor executor) {
+    protected ObjectScanner installObjectScanner(UniverseMetaAccess metaAccess, CompletionExecutor executor) {
         StandaloneImageHeapScanner standaloneImageHeapScanner = (StandaloneImageHeapScanner) this.scanner;
         return new StandaloneObjectScanner(bb, executor, scannedObjects, new ScanningObserver(), standaloneImageHeapScanner.getShouldScanConstant(),
                         standaloneImageHeapScanner.getShouldScanField());

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java

@@ -204,10 +204,10 @@ public void runAnalysis(DebugContext debugContext, Function<AnalysisUniverse, Bo
     protected abstract CompletionExecutor.Timing getTiming();
 
     @SuppressWarnings("try")
-    private boolean analysisModified() throws InterruptedException {
+    private boolean analysisModified() {
         boolean analysisModified;
         try (Timer.StopTimer ignored = verifyHeapTimer.start()) {
-            analysisModified = universe.getHeapVerifier().requireAnalysisIteration(executor);
+            analysisModified = universe.getHeapVerifier().checkHeapSnapshot(metaAccess, executor, "after analysis", true);
         }
         /* Initialize for the next iteration. */
         executor.init(getTiming());

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java

@@ -98,7 +98,7 @@ public void scanBootImageHeapRoots(Comparator<AnalysisField> fieldComparator, Co
             fields = fieldsList;
         }
         for (AnalysisField field : fields) {
-            if (Modifier.isStatic(field.getModifiers()) && field.getJavaKind() == JavaKind.Object && field.isRead()) {
+            if (Modifier.isStatic(field.getModifiers()) && field.isRead()) {
                 execute(() -> scanRootField(field));
             }
         }
@@ -157,7 +157,9 @@ protected void scanField(AnalysisField field, JavaConstant receiver, ScanReason
                 /* The value is not available yet. */
                 return;
             }
-            JavaConstant fieldValue = bb.getUniverse().getHeapScanner().readFieldValue(field, receiver);
+            assert isUnwrapped(receiver);
+
+            JavaConstant fieldValue = readFieldValue(field, receiver);
             if (fieldValue == null) {
                 StringBuilder backtrace = new StringBuilder();
                 buildObjectBacktrace(bb, reason, backtrace);
@@ -179,20 +181,47 @@ protected void scanField(AnalysisField field, JavaConstant receiver, ScanReason
                  * referenced elements are being scanned.
                  */
                 scanConstant(fieldValue, reason);
+            } else if (fieldValue.getJavaKind().isNumericInteger()) {
+                scanningObserver.forPrimitiveFieldValue(receiver, field, fieldValue, reason);
             }
 
         } catch (UnsupportedFeatureException ex) {
             unsupportedFeatureDuringFieldScan(bb, field, receiver, ex, reason);
         }
     }
 
+    protected JavaConstant readFieldValue(AnalysisField field, JavaConstant receiver) {
+        return bb.getConstantReflectionProvider().readFieldValue(field, receiver);
+    }
+
+    /**
+     * Must unwrap the receiver if it is an ImageHeapConstant to scan the hosted value, if any, for
+     * verification, otherwise the verification just compares shadow heap with shadow heap for
+     * embedded roots, which is completely useless.
+     */
+    private static JavaConstant maybeUnwrap(JavaConstant receiver) {
+        if (receiver instanceof ImageHeapConstant heapConstant && heapConstant.getHostedObject() != null) {
+            return heapConstant.getHostedObject();
+        }
+        return receiver;
+    }
+
+    private static boolean isUnwrapped(JavaConstant receiver) {
+        if (receiver instanceof ImageHeapConstant heapConstant) {
+            // Non hosted backed ImageHeapConstant is considered unwrapped
+            return heapConstant.getHostedObject() == null;
+        }
+        return true;
+    }
+
     /**
      * Scans constant arrays, one element at the time.
      *
      * @param array the array to be scanned
      */
     protected final void scanArray(JavaConstant array, ScanReason prevReason) {
 
+        assert isUnwrapped(array);
         AnalysisType arrayType = bb.getMetaAccess().lookupJavaType(array);
         ScanReason reason = new ArrayScan(arrayType, array, prevReason);
 
@@ -246,13 +275,14 @@ public void scanConstant(JavaConstant value, ScanReason reason) {
             bb.registerTypeAsInHeap(bb.getMetaAccess().lookupJavaType(value), reason);
             return;
         }
-        Object valueObj = (value instanceof ImageHeapConstant) ? value : constantAsObject(bb, value);
+        JavaConstant unwrappedValue = maybeUnwrap(value);
+        Object valueObj = unwrappedValue instanceof ImageHeapConstant ? unwrappedValue : constantAsObject(bb, unwrappedValue);
         if (scannedObjects.putAndAcquire(valueObj) == null) {
             try {
-                scanningObserver.forScannedConstant(value, reason);
+                scanningObserver.forScannedConstant(unwrappedValue, reason);
             } finally {
                 scannedObjects.release(valueObj);
-                WorklistEntry worklistEntry = new WorklistEntry(value, reason);
+                WorklistEntry worklistEntry = new WorklistEntry(unwrappedValue, reason);
                 if (executor != null) {
                     executor.execute(debug -> doScan(worklistEntry));
                 } else {
@@ -339,12 +369,22 @@ public static String asString(BigBang bb, JavaConstant constant, boolean appendT
             return "null";
         }
         AnalysisType type = bb.getMetaAccess().lookupJavaType(constant);
-        if (constant instanceof ImageHeapConstant) {
-            // Checkstyle: allow Class.getSimpleName
-            return constant.getClass().getSimpleName() + "<" + type.toJavaName() + ">";
-            // Checkstyle: disallow Class.getSimpleName
+        JavaConstant hosted = constant;
+        if (constant instanceof ImageHeapConstant heapConstant) {
+            JavaConstant hostedObject = heapConstant.getHostedObject();
+            if (hostedObject == null) {
+                // Checkstyle: allow Class.getSimpleName
+                return constant.getClass().getSimpleName() + "<" + type.toJavaName() + ">";
+                // Checkstyle: disallow Class.getSimpleName
+            }
+            hosted = hostedObject;
+        }
+
+        if (hosted.getJavaKind().isPrimitive()) {
+            return hosted.toValueString();
         }
-        Object obj = constantAsObject(bb, constant);
+
+        Object obj = constantAsObject(bb, hosted);
         String str = type.toJavaName() + '@' + Integer.toHexString(System.identityHashCode(obj));
         if (appendToString) {
             try {
@@ -381,7 +421,7 @@ private void doScan(WorklistEntry entry) {
                 /* Scan constant's instance fields. */
                 for (ResolvedJavaField javaField : type.getInstanceFields(true)) {
                     AnalysisField field = (AnalysisField) javaField;
-                    if (field.getJavaKind() == JavaKind.Object && field.isRead()) {
+                    if (field.isRead()) {
                         assert !Modifier.isStatic(field.getModifiers());
                         scanField(field, entry.constant, entry.reason);
                     }
@@ -589,15 +629,21 @@ public String toString() {
 
     public static class ArrayScan extends ScanReason {
         final AnalysisType arrayType;
+        final int idx;
 
         public ArrayScan(AnalysisType arrayType, JavaConstant array, ScanReason previous) {
+            this(arrayType, array, previous, -1);
+        }
+
+        public ArrayScan(AnalysisType arrayType, JavaConstant array, ScanReason previous, int idx) {
             super(previous, array);
             this.arrayType = arrayType;
+            this.idx = idx;
         }
 
         @Override
         public String toString(BigBang bb) {
-            return "indexing into array " + asString(bb, constant);
+            return "indexing into array " + asString(bb, constant) + (idx != -1 ? " at index " + idx : "");
         }
 
         @Override

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanningObserver.java

@@ -51,6 +51,13 @@ default boolean forRelocatedPointerFieldValue(JavaConstant receiver, AnalysisFie
         return false;
     }
 
+    /**
+     * Hook for scanned value of primitive field.
+     */
+    default boolean forPrimitiveFieldValue(JavaConstant receiver, AnalysisField field, JavaConstant fieldValue, ScanReason reason) {
+        return false;
+    }
+
     /**
      * Hook for scanned null field value.
      */