From 59e1e1b2d7a2f72864052d4aa969b655c74b6e3c Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 29 Dec 2023 10:09:20 -0800 Subject: [PATCH] Update R8 and enable Dalvik tests on JDK 21 (#1353) Updating to the latest R8 version lets us run the Dalvik tests on JDK 21. We disable one check due to https://issuetracker.google.com/issues/316744331; we'll have to update R8 again once that is fixed. We also needed to tweak how we construct the test jar for use with Dalvik. My best guess is some classes caused issues with D8 before, but they no longer do so. Finally, we performed some cleanup in `JVMLDalvikComparisonTest` to make failures easier to understand. Fixes #1349 --- core/build.gradle.kts | 9 +- dalvik/build.gradle.kts | 6 -- .../callGraph/JVMLDalvikComparisonTest.java | 87 ++++++++++--------- gradle/libs.versions.toml | 2 +- 4 files changed, 51 insertions(+), 53 deletions(-) diff --git a/core/build.gradle.kts b/core/build.gradle.kts index f032777f5c..0abacb3d3d 100644 --- a/core/build.gradle.kts +++ b/core/build.gradle.kts @@ -312,10 +312,10 @@ artifacts.add(collectTestDataJar.name, collectTestData.map { it.destinationDirec //////////////////////////////////////////////////////////////////////// // -// collect "com.ibm.wala.core.testdata_1.0.0a.jar" +// collect "com.ibm.wala.core.testdata_1.0.0a.jar" for Dalvik tests // -val collectTestDataA by +val collectTestDataAForDalvik by tasks.registering(Jar::class) { archiveFileName = "com.ibm.wala.core.testdata_1.0.0a.jar" from(compileTestSubjectsJava) @@ -323,9 +323,8 @@ val collectTestDataA by includeEmptyDirs = false destinationDirectory = layout.buildDirectory.dir(name) exclude( + // This is an invalid class so don't include it; it causes D8 to crash "**/CodeDeleted.class", - "**/SortingExample.class", - "**/A.class", ) } @@ -386,7 +385,7 @@ val dalvikTestResources: Configuration by configurations.creating { isCanBeResol listOf( collectJLex, - collectTestDataA, + collectTestDataAForDalvik, downloadJavaCup, extractBcel, ) diff --git a/dalvik/build.gradle.kts b/dalvik/build.gradle.kts index f74a6a27b6..4d8891fb04 100644 --- a/dalvik/build.gradle.kts +++ b/dalvik/build.gradle.kts @@ -192,12 +192,6 @@ if (isWindows) tasks.named("test") { exclude("**/droidbench/**") } else sourceSets.test.configure { resources.srcDir(unpackDroidBench) } tasks.named("test") { - if (JavaVersion.current() == JavaVersion.VERSION_21) { - // Disable the task for JDK 21 for now. We have test failures due to a required r8 upgrade - // that introduces some new behaviors we don't expect - // See https://github.com/wala/WALA/issues/1349 - enabled = false - } maxHeapSize = "800M" outputs.files( diff --git a/dalvik/src/test/java/com/ibm/wala/dalvik/test/callGraph/JVMLDalvikComparisonTest.java b/dalvik/src/test/java/com/ibm/wala/dalvik/test/callGraph/JVMLDalvikComparisonTest.java index ae17ad99bc..70877ad97b 100644 --- a/dalvik/src/test/java/com/ibm/wala/dalvik/test/callGraph/JVMLDalvikComparisonTest.java +++ b/dalvik/src/test/java/com/ibm/wala/dalvik/test/callGraph/JVMLDalvikComparisonTest.java @@ -26,11 +26,8 @@ import com.ibm.wala.ipa.callgraph.Entrypoint; import com.ibm.wala.ipa.callgraph.impl.Util; import com.ibm.wala.ipa.callgraph.propagation.InstanceKey; -import com.ibm.wala.ipa.callgraph.propagation.LocalPointerKey; import com.ibm.wala.ipa.callgraph.propagation.PointerAnalysis; -import com.ibm.wala.ipa.callgraph.propagation.ReturnValueKey; import com.ibm.wala.ipa.callgraph.propagation.SSAPropagationCallGraphBuilder; -import com.ibm.wala.ipa.callgraph.propagation.cfa.ExceptionReturnValueKey; import com.ibm.wala.ipa.cha.ClassHierarchy; import com.ibm.wala.ipa.cha.ClassHierarchyException; import com.ibm.wala.ipa.cha.ClassHierarchyFactory; @@ -40,15 +37,12 @@ import com.ibm.wala.types.MethodReference; import com.ibm.wala.util.CancelException; import com.ibm.wala.util.collections.HashSetFactory; -import com.ibm.wala.util.collections.Iterator2Collection; import com.ibm.wala.util.collections.Pair; import com.ibm.wala.util.intset.IntSet; import com.ibm.wala.util.intset.IntSetUtil; import com.ibm.wala.util.intset.MutableIntSet; -import com.ibm.wala.util.intset.OrdinalSet; import java.io.File; import java.io.IOException; -import java.util.Iterator; import java.util.Set; import org.junit.jupiter.api.Test; @@ -103,6 +97,22 @@ private static Set> edgeDiff( private static void test(String mainClass, String javaScopeFile) throws IllegalArgumentException, IOException, CancelException, ClassHierarchyException { + test(mainClass, javaScopeFile, false); + } + + /** + * Run tests to compare the call graphs computing with the JVM bytecode frontend vs the Dalvik + * frontend + * + * @param mainClass main class for the test + * @param javaScopeFile scope file for the test + * @param allowExtraJavaCGEdges if true, allow extra edges in the JVM bytecode frontend call + * graph. This flag is temporarily required due to issues with JLex caused by + * https://issuetracker.google.com/issues/316744331. TODO: remove this flag once the D8 issue + * is fixed. + */ + private static void test(String mainClass, String javaScopeFile, boolean allowExtraJavaCGEdges) + throws IllegalArgumentException, IOException, CancelException, ClassHierarchyException { Pair> java = makeJavaBuilder(javaScopeFile, mainClass); AnalysisScope javaScope = java.fst.getClassHierarchy().getScope(); @@ -114,13 +124,15 @@ private static void test(String mainClass, String javaScopeFile) Set androidMethods = applicationMethods(android.fst); Set javaMethods = applicationMethods(java.fst); - Iterator> javaExtraEdges = - edgeDiff(java.fst, android.fst, false).iterator(); - assert !checkEdgeDiff(android, androidMethods, javaMethods, javaExtraEdges); + if (!allowExtraJavaCGEdges) { + Set> javaExtraEdges = edgeDiff(java.fst, android.fst, false); + assert !checkEdgeDiff(android, androidMethods, javaMethods, javaExtraEdges) + : "found extra edges in Java call graph"; + } - Iterator> androidExtraEdges = - edgeDiff(android.fst, java.fst, true).iterator(); - assert !checkEdgeDiff(java, javaMethods, androidMethods, androidExtraEdges); + Set> androidExtraEdges = edgeDiff(android.fst, java.fst, true); + assert !checkEdgeDiff(java, javaMethods, androidMethods, androidExtraEdges) + : "found extra edges in Android call graph"; checkSourceLines(java.fst, android.fst); } @@ -177,36 +189,29 @@ private static void checkSourceLines(CallGraph java, CallGraph android) { } private static boolean checkEdgeDiff( - Pair> android, - Set androidMethods, - Set javaMethods, - Iterator> javaExtraEdges) { + Pair> firstResult, + Set methodsInFirst, + Set methodsInSecond, + Set> extraEdgesInSecond) { boolean fail = false; - if (javaExtraEdges.hasNext()) { + if (!extraEdgesInSecond.isEmpty()) { fail = true; - Set javaExtraNodes = HashSetFactory.make(javaMethods); - javaExtraNodes.removeAll(androidMethods); - - System.err.println(Iterator2Collection.toSet(javaExtraEdges)); - System.err.println(javaExtraNodes); - - System.err.println(android.fst); - - for (CGNode n : android.fst) { - System.err.println("### " + n); - if (n.getIR() != null) { - System.err.println(n.getIR()); - - System.err.println("return: " + android.snd.getPointsToSet(new ReturnValueKey(n))); - System.err.println( - "exceptions: " + android.snd.getPointsToSet(new ExceptionReturnValueKey(n))); - for (int i = 1; i < n.getIR().getSymbolTable().getMaxValueNumber(); i++) { - LocalPointerKey x = new LocalPointerKey(n, i); - OrdinalSet s = android.snd.getPointsToSet(x); - if (s != null && !s.isEmpty()) { - System.err.println(i + ": " + s); - } - } + Set extraMethodsInSecond = HashSetFactory.make(methodsInSecond); + extraMethodsInSecond.removeAll(methodsInFirst); + + System.err.println(extraEdgesInSecond); + System.err.println(extraMethodsInSecond); + + CallGraph firstCG = firstResult.fst; + for (Pair p : extraEdgesInSecond) { + System.err.println("### " + p.fst); + System.err.println("### " + p.snd); + System.err.println("### " + p.fst.getIR()); + System.err.println("===="); + Set nodes = firstCG.getNodes(p.fst.getMethod().getReference()); + for (CGNode n : nodes) { + System.err.println("### " + n); + System.err.println("### " + n.getIR()); } } } @@ -216,7 +221,7 @@ private static boolean checkEdgeDiff( @Test public void testJLex() throws ClassHierarchyException, IllegalArgumentException, IOException, CancelException { - test(TestConstants.JLEX_MAIN, TestConstants.JLEX); + test(TestConstants.JLEX_MAIN, TestConstants.JLEX, true); } @Test diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 57abd804ad..5766673be4 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -6,7 +6,7 @@ ktfmt = "0.44" spotless = "6.23.3" [libraries] -android-tools = "com.android.tools:r8:2.2.42" +android-tools = "com.android.tools:r8:8.2.39" ant = "org.apache.ant:ant:1.10.14" assertj-core = "org.assertj:assertj-core:3.24.2" commons-cli = "commons-cli:commons-cli:1.5.0"