[GR-27034] Making static analysis deterministic.

Maja Skoko · Maja Skoko · commit e03ab4a11d07 · 2023-04-16T23:42:04.000Z
PullRequest: graal/13943
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java
@@ -553,7 +553,7 @@ public String getName() {
 
     @Override
     public WrappedSignature getSignature() {
-        return getUniverse().lookup(wrapped.getSignature(), getDeclaringClass().getWrappedWithResolve());
+        return getUniverse().lookup(wrapped.getSignature(), wrapped.getDeclaringClass());
     }
 
     @Override
@@ -669,7 +669,7 @@ public ProfilingInfo getProfilingInfo(boolean includeNormal, boolean includeOSR)
 
     @Override
     public ConstantPool getConstantPool() {
-        return getUniverse().lookup(wrapped.getConstantPool(), getDeclaringClass().getWrappedWithResolve());
+        return getUniverse().lookup(wrapped.getConstantPool(), wrapped.getDeclaringClass());
     }
 
     @Override
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/CallTreePrinter.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/CallTreePrinter.java
@@ -113,6 +113,7 @@ static class MethodNodeReference implements Node {
         public String format() {
             return methodNode.method.format(METHOD_FORMAT) + " id-ref=" + methodNode.id;
         }
+
     }
 
     static class MethodNode implements Node {
@@ -193,11 +194,16 @@ public void buildCallTree() {
                 }
             }
         }
+
         roots.sort(methodComparator);
         for (AnalysisMethod m : roots) {
             methodToNode.put(m, new MethodNode(m, true));
         }
-        /* Walk the call graph starting from the roots, do a breadth-first tree reduction. */
+
+        /*
+         * Walk the call graph starting from the roots (deterministically sorted), do a
+         * breadth-first tree reduction.
+         */
         ArrayDeque<MethodNode> workList = new ArrayDeque<>(methodToNode.values());
 
         while (!workList.isEmpty()) {
@@ -210,10 +216,17 @@ public void buildCallTree() {
             for (var invokeInfo : node.method.getInvokes()) {
                 invokeInfos.add(invokeInfo);
             }
+
+            /*
+             * In order to have deterministic order of invokes we sort them by position and names.
+             * In case of Lambda names we avoid the non-deterministic hash part while sorting.
+             */
             invokeInfos.sort(invokeInfoComparator);
+
             for (var invokeInfo : invokeInfos) {
                 processInvoke(invokeInfo, node, workList);
             }
+
         }
     }
 
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java
@@ -58,12 +58,27 @@ public class ReportUtils {
     static final String CHILD = "\u251c\u2500\u2500 "; // "|-- "
     static final String LAST_CHILD = "\u2514\u2500\u2500 "; // "`-- "
 
-    public static final Comparator<ResolvedJavaMethod> methodComparator = Comparator.comparing(m -> m.format("%H.%n(%p)"));
+    public static final Comparator<ResolvedJavaMethod> methodComparator = Comparator.comparing(m -> m.format("%H.%n(%P):%R"));
     static final Comparator<AnalysisField> fieldComparator = Comparator.comparing(f -> f.format("%H.%n"));
-    static final Comparator<InvokeInfo> invokeInfoComparator = Comparator.comparing(i -> i.getTargetMethod().format("%H.%n(%p)"));
-    static final Comparator<BytecodePosition> positionMethodComparator = Comparator.comparing(pos -> pos.getMethod().format("%H.%n(%p)"));
+    static final Comparator<InvokeInfo> invokeInfoBCIComparator = Comparator.comparing(i -> i.getPosition().getBCI());
+    static final Comparator<InvokeInfo> invokeInfoComparator = invokeInfoBCIComparator.thenComparing(i -> comparingMethodNames(i.getTargetMethod()));
+    static final Comparator<BytecodePosition> positionMethodComparator = Comparator.comparing(pos -> pos.getMethod().format("%H.%n(%P):%R"));
     static final Comparator<BytecodePosition> positionComparator = positionMethodComparator.thenComparing(pos -> pos.getBCI());
 
+    /**
+     *
+     * Lambda function names are still not completely deterministic e.g. in name
+     * Lambda$7ad16f47b695d909/0x00000007c0b4c630.accept(java.lang.Object):void hash part is not
+     * deterministic yet. In order to avoid comparing based on that part, we need to eliminate hash
+     * part from name of lambda function. To read more about Lambda names check GH issue
+     * https://github.com/openjdk/jdk/pull/10024/.
+     *
+     */
+    private static String comparingMethodNames(AnalysisMethod method) {
+        String methodName = method.format("%H.%n(%P):%R");
+        return methodName.contains("$$Lambda$") ? methodName.replaceAll("/[0-9a-fA-Fx]*\\.", ".") : methodName;
+    }
+
     public static Path report(String description, String path, String name, String extension, Consumer<PrintWriter> reporter) {
         return report(description, path, name, extension, reporter, true);
     }
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java
@@ -31,6 +31,7 @@
 import static org.graalvm.compiler.replacements.StandardGraphBuilderPlugins.registerInvocationPlugins;
 
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -55,6 +56,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BooleanSupplier;
 
+import com.oracle.graal.pointsto.reports.ReportUtils;
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.EconomicSet;
 import org.graalvm.collections.MapCursor;
@@ -1735,63 +1737,70 @@ public Map<ArtifactType, List<Path>> getBuildArtifacts() {
     }
 
     private void printTypes() {
+        String reportsPath = SubstrateOptions.reportsPath();
+        ReportUtils.report("print types", reportsPath, "universe_analysis", "txt",
+                        writer -> printTypes(writer));
+    }
+
+    private void printTypes(PrintWriter writer) {
+
         for (HostedType type : hUniverse.getTypes()) {
-            System.out.format("%8d %s  ", type.getTypeID(), type.toJavaName(true));
+            writer.format("%8d %s  ", type.getTypeID(), type.toJavaName(true));
             if (type.getSuperclass() != null) {
-                System.out.format("extends %d %s  ", type.getSuperclass().getTypeID(), type.getSuperclass().toJavaName(false));
+                writer.format("extends %d %s  ", type.getSuperclass().getTypeID(), type.getSuperclass().toJavaName(false));
             }
             if (type.getInterfaces().length > 0) {
-                System.out.print("implements ");
+                writer.print("implements ");
                 String sep = "";
                 for (HostedInterface interf : type.getInterfaces()) {
-                    System.out.format("%s%d %s", sep, interf.getTypeID(), interf.toJavaName(false));
+                    writer.format("%s%d %s", sep, interf.getTypeID(), interf.toJavaName(false));
                     sep = ", ";
                 }
-                System.out.print("  ");
+                writer.print("  ");
             }
 
             if (type.getWrapped().isInstantiated()) {
-                System.out.print("instantiated  ");
+                writer.print("instantiated  ");
             }
             if (type.getWrapped().isReachable()) {
-                System.out.print("reachable  ");
+                writer.print("reachable  ");
             }
 
-            System.out.format("type check start %d range %d slot # %d ", type.getTypeCheckStart(), type.getTypeCheckRange(), type.getTypeCheckSlot());
-            System.out.format("type check slots %s  ", slotsToString(type.getTypeCheckSlots()));
+            writer.format("type check start %d range %d slot # %d ", type.getTypeCheckStart(), type.getTypeCheckRange(), type.getTypeCheckSlot());
+            writer.format("type check slots %s  ", slotsToString(type.getTypeCheckSlots()));
             // if (type.findLeafConcreteSubtype() != null) {
-            // System.out.format("unique %d %s ", type.findLeafConcreteSubtype().getTypeID(),
+            // writer.format("unique %d %s ", type.findLeafConcreteSubtype().getTypeID(),
             // type.findLeafConcreteSubtype().toJavaName(false));
             // }
 
             int le = type.getHub().getLayoutEncoding();
             if (LayoutEncoding.isPrimitive(le)) {
-                System.out.print("primitive  ");
+                writer.print("primitive  ");
             } else if (LayoutEncoding.isInterface(le)) {
-                System.out.print("interface  ");
+                writer.print("interface  ");
             } else if (LayoutEncoding.isAbstract(le)) {
-                System.out.print("abstract  ");
+                writer.print("abstract  ");
             } else if (LayoutEncoding.isPureInstance(le)) {
-                System.out.format("instance size %d  ", LayoutEncoding.getPureInstanceAllocationSize(le).rawValue());
+                writer.format("instance size %d  ", LayoutEncoding.getPureInstanceAllocationSize(le).rawValue());
             } else if (LayoutEncoding.isArrayLike(le)) {
                 String arrayType = LayoutEncoding.isHybrid(le) ? "hybrid" : "array";
                 String elements = LayoutEncoding.isArrayLikeWithPrimitiveElements(le) ? "primitives" : "objects";
-                System.out.format("%s containing %s, base %d shift %d scale %d  ", arrayType, elements, LayoutEncoding.getArrayBaseOffset(le).rawValue(), LayoutEncoding.getArrayIndexShift(le),
+                writer.format("%s containing %s, base %d shift %d scale %d  ", arrayType, elements, LayoutEncoding.getArrayBaseOffset(le).rawValue(), LayoutEncoding.getArrayIndexShift(le),
                                 LayoutEncoding.getArrayIndexScale(le));
 
             } else {
                 throw VMError.shouldNotReachHereUnexpectedInput(le); // ExcludeFromJacocoGeneratedReport
             }
 
-            System.out.println();
+            writer.println();
 
             for (HostedType sub : type.getSubTypes()) {
-                System.out.format("               s %d %s%n", sub.getTypeID(), sub.toJavaName(false));
+                writer.format("               s %d %s%n", sub.getTypeID(), sub.toJavaName(false));
             }
             if (type.isInterface()) {
                 for (HostedMethod method : hUniverse.getMethods()) {
                     if (method.getDeclaringClass().equals(type)) {
-                        printMethod(method, -1);
+                        printMethod(writer, method, -1);
                     }
                 }
 
@@ -1801,41 +1810,42 @@ private void printTypes() {
                 fields = Arrays.copyOf(fields, fields.length);
                 Arrays.sort(fields, Comparator.comparing(HostedField::toString));
                 for (HostedField field : fields) {
-                    System.out.println("               f " + field.getLocation() + ": " + field.format("%T %n"));
+                    writer.println("               f " + field.getLocation() + ": " + field.format("%T %n"));
                 }
                 HostedMethod[] vtable = type.getVTable();
                 for (int i = 0; i < vtable.length; i++) {
                     if (vtable[i] != null) {
-                        printMethod(vtable[i], i);
+                        printMethod(writer, vtable[i], i);
                     }
                 }
                 for (HostedMethod method : hUniverse.getMethods()) {
                     if (method.getDeclaringClass().equals(type) && !method.hasVTableIndex()) {
-                        printMethod(method, -1);
+                        printMethod(writer, method, -1);
                     }
                 }
             }
         }
+
     }
 
-    private static void printMethod(HostedMethod method, int vtableIndex) {
+    private static void printMethod(PrintWriter writer, HostedMethod method, int vtableIndex) {
         if (vtableIndex != -1) {
-            System.out.print("               v " + vtableIndex + " ");
+            writer.print("               v " + vtableIndex + " ");
         } else {
-            System.out.print("               m ");
+            writer.print("               m ");
         }
         if (method.hasVTableIndex()) {
-            System.out.print(method.getVTableIndex() + " ");
+            writer.print(method.getVTableIndex() + " ");
         }
-        System.out.print(method.format("%r %n(%p)") + ": " + method.getImplementations().length + " [");
+        writer.print(method.format("%r %n(%p)") + ": " + method.getImplementations().length + " [");
         if (method.getImplementations().length <= 10) {
             String sep = "";
             for (HostedMethod impl : method.getImplementations()) {
-                System.out.print(sep + impl.getDeclaringClass().toJavaName(false));
+                writer.print(sep + impl.getDeclaringClass().toJavaName(false));
                 sep = ", ";
             }
         }
-        System.out.println("]");
+        writer.println("]");
     }
 
     private static String slotsToString(short[] slots) {
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageCodeCache.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageCodeCache.java
@@ -28,6 +28,7 @@
 import static com.oracle.svm.core.util.VMError.shouldNotReachHere;
 import static com.oracle.svm.core.util.VMError.shouldNotReachHereUnexpectedInput;
 
+import java.io.PrintWriter;
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Executable;
 import java.lang.reflect.Field;
@@ -48,6 +49,7 @@
 import java.util.concurrent.ForkJoinPool;
 import java.util.stream.Collectors;
 
+import com.oracle.graal.pointsto.reports.ReportUtils;
 import org.graalvm.collections.Pair;
 import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
 import org.graalvm.compiler.code.CompilationResult;
@@ -165,7 +167,8 @@ public Pair<HostedMethod, CompilationResult> getLastCompilation() {
     }
 
     protected List<Pair<HostedMethod, CompilationResult>> computeCompilationOrder(Map<HostedMethod, CompilationResult> compilationMap) {
-        return compilationMap.entrySet().stream().map(e -> Pair.create(e.getKey(), e.getValue())).collect(Collectors.toList());
+        return compilationMap.entrySet().stream().map(e -> Pair.create(e.getKey(), e.getValue())).sorted(Comparator.comparing(o -> o.getLeft().wrapped.format("%H.%n(%P):%R")))
+                        .collect(Collectors.toList());
     }
 
     public List<Pair<HostedMethod, CompilationResult>> getOrderedCompilations() {
@@ -587,24 +590,32 @@ public Path[] getCCInputFiles(Path tempDirectory, String imageName) {
     public abstract List<ObjectFile.Symbol> getSymbols(ObjectFile objectFile);
 
     public void printCompilationResults() {
-        System.out.println("--- compiled methods");
+        String reportsPath = SubstrateOptions.reportsPath();
+        ReportUtils.report("compilation results", reportsPath, "universe_compilation", "txt",
+                        writer -> printCompilationResults(writer));
+    }
+
+    private void printCompilationResults(PrintWriter writer) {
+
+        writer.println("--- compiled methods");
         for (Pair<HostedMethod, CompilationResult> pair : getOrderedCompilations()) {
             HostedMethod method = pair.getLeft();
             CompilationResult result = pair.getRight();
-            System.out.format("%8d %5d %s: frame %d%n", method.getCodeAddressOffset(), result.getTargetCodeSize(), method.format("%H.%n(%p)"), result.getTotalFrameSize());
+            writer.format("%8d %5d %s: frame %d%n", method.getCodeAddressOffset(), result.getTargetCodeSize(), method.format("%H.%n(%p)"), result.getTotalFrameSize());
         }
-        System.out.println("--- vtables:");
+        writer.println("--- vtables:");
         for (HostedType type : imageHeap.getUniverse().getTypes()) {
             for (int i = 0; i < type.getVTable().length; i++) {
                 HostedMethod method = type.getVTable()[i];
                 if (method != null) {
                     CompilationResult comp = compilationResultFor(type.getVTable()[i]);
                     if (comp != null) {
-                        System.out.format("%d %s @ %d: %s = 0x%x%n", type.getTypeID(), type.toJavaName(false), i, method.format("%r %n(%p)"), method.getCodeAddressOffset());
+                        writer.format("%d %s @ %d: %s = 0x%x%n", type.getTypeID(), type.toJavaName(false), i, method.format("%r %n(%p)"), method.getCodeAddressOffset());
                     }
                 }
             }
         }
+
     }
 
     private static class HostedFrameInfoCustomization extends FrameInfoEncoder.SourceFieldsFromMethod {
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/TypeCheckBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/TypeCheckBuilder.java
@@ -174,7 +174,8 @@ private static short getShortValue(int intValue) {
 
     /**
      * Calculating a sorted list based on the height of each node. This allows one to compute many
-     * graph traits in one iteration of the nodes.
+     * graph traits in one iteration of the nodes. When height are same, elements of list are sorted
+     * by name.
      */
     private static List<HostedType> generateHeightOrder(List<HostedType> roots, Map<HostedType, List<HostedType>> subtypeMap) {
 
@@ -188,8 +189,9 @@ private static List<HostedType> generateHeightOrder(List<HostedType> roots, Map<
             generateHeightOrderHelper(0, root, subtypeMap, heightMap, allTypes);
         }
 
-        /* Now create a sorted array from this information. */
-        return allTypes.stream().sorted(Comparator.comparingInt(heightMap::get)).collect(Collectors.toList());
+        /* Create a sorted array from this information. */
+        Comparator<HostedType> comparator = Comparator.<HostedType> comparingInt(heightMap::get).thenComparing(HostedUniverse.TYPE_COMPARATOR);
+        return allTypes.stream().sorted(comparator).collect(Collectors.toList());
     }
 
     /**

Original file line number	Diff line number	Diff line change
`@@ -553,7 +553,7 @@ public String getName() {`
`553`	`553`
`554`	`554`	`@Override`
`555`	`555`	`public WrappedSignature getSignature() {`
`556`		`- return getUniverse().lookup(wrapped.getSignature(), getDeclaringClass().getWrappedWithResolve());`
	`556`	`+ return getUniverse().lookup(wrapped.getSignature(), wrapped.getDeclaringClass());`
`557`	`557`	`}`
`558`	`558`
`559`	`559`	`@Override`
`@@ -669,7 +669,7 @@ public ProfilingInfo getProfilingInfo(boolean includeNormal, boolean includeOSR)`
`669`	`669`
`670`	`670`	`@Override`
`671`	`671`	`public ConstantPool getConstantPool() {`
`672`		`- return getUniverse().lookup(wrapped.getConstantPool(), getDeclaringClass().getWrappedWithResolve());`
	`672`	`+ return getUniverse().lookup(wrapped.getConstantPool(), wrapped.getDeclaringClass());`
`673`	`673`	`}`
`674`	`674`
`675`	`675`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -113,6 +113,7 @@ static class MethodNodeReference implements Node {`
`113`	`113`	`public String format() {`
`114`	`114`	`return methodNode.method.format(METHOD_FORMAT) + " id-ref=" + methodNode.id;`
`115`	`115`	`}`
	`116`	`+`
`116`	`117`	`}`
`117`	`118`
`118`	`119`	`static class MethodNode implements Node {`
`@@ -193,11 +194,16 @@ public void buildCallTree() {`
`193`	`194`	`}`
`194`	`195`	`}`
`195`	`196`	`}`
	`197`	`+`
`196`	`198`	`roots.sort(methodComparator);`
`197`	`199`	`for (AnalysisMethod m : roots) {`
`198`	`200`	`methodToNode.put(m, new MethodNode(m, true));`
`199`	`201`	`}`
`200`		`- /* Walk the call graph starting from the roots, do a breadth-first tree reduction. */`
	`202`	`+`
	`203`	`+ /*`
	`204`	`+ * Walk the call graph starting from the roots (deterministically sorted), do a`
	`205`	`+ * breadth-first tree reduction.`
	`206`	`+ */`
`201`	`207`	`ArrayDeque<MethodNode> workList = new ArrayDeque<>(methodToNode.values());`
`202`	`208`
`203`	`209`	`while (!workList.isEmpty()) {`
`@@ -210,10 +216,17 @@ public void buildCallTree() {`
`210`	`216`	`for (var invokeInfo : node.method.getInvokes()) {`
`211`	`217`	`invokeInfos.add(invokeInfo);`
`212`	`218`	`}`
	`219`	`+`
	`220`	`+ /*`
	`221`	`+ * In order to have deterministic order of invokes we sort them by position and names.`
	`222`	`+ * In case of Lambda names we avoid the non-deterministic hash part while sorting.`
	`223`	`+ */`
`213`	`224`	`invokeInfos.sort(invokeInfoComparator);`
	`225`	`+`
`214`	`226`	`for (var invokeInfo : invokeInfos) {`
`215`	`227`	`processInvoke(invokeInfo, node, workList);`
`216`	`228`	`}`
	`229`	`+`
`217`	`230`	`}`
`218`	`231`	`}`
`219`	`232`