From 29c011f8b22d939376fb1aef0ea5131608315021 Mon Sep 17 00:00:00 2001 From: Timothy Hoffman <4001421+tim-hoffman@users.noreply.github.com> Date: Thu, 1 Jul 2021 16:18:29 -0500 Subject: [PATCH 1/2] fix a FlowAnalysis bug were non-returning loops were skipped --- src/main/java/soot/Body.java | 17 +- .../soot/toolkits/scalar/FlowAnalysis.java | 69 ++-- .../toolkits/scalar/SimpleLiveLocalsTest.java | 336 ++++++++++++++++++ 3 files changed, 375 insertions(+), 47 deletions(-) create mode 100644 src/test/java/soot/toolkits/scalar/SimpleLiveLocalsTest.java diff --git a/src/main/java/soot/Body.java b/src/main/java/soot/Body.java index 58279f63668..7413d6efb72 100644 --- a/src/main/java/soot/Body.java +++ b/src/main/java/soot/Body.java @@ -35,7 +35,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import soot.jimple.IdentityStmt; import soot.jimple.ParameterRef; import soot.jimple.ThisRef; import soot.options.Options; @@ -340,7 +339,7 @@ public Chain getTraps() { */ public Unit getThisUnit() { for (Unit u : getUnits()) { - if (u instanceof IdentityStmt && ((IdentityStmt) u).getRightOp() instanceof ThisRef) { + if (u instanceof IdentityUnit && ((IdentityUnit) u).getRightOp() instanceof ThisRef) { return u; } } @@ -354,7 +353,7 @@ public Unit getThisUnit() { * @return */ public Local getThisLocal() { - return (Local) (((IdentityStmt) getThisUnit()).getLeftOp()); + return (Local) (((IdentityUnit) getThisUnit()).getLeftOp()); } /** @@ -366,8 +365,8 @@ public Local getThisLocal() { */ public Local getParameterLocal(int i) { for (Unit s : getUnits()) { - if (s instanceof IdentityStmt) { - IdentityStmt is = (IdentityStmt) s; + if (s instanceof IdentityUnit) { + IdentityUnit is = (IdentityUnit) s; Value rightOp = is.getRightOp(); if (rightOp instanceof ParameterRef) { ParameterRef pr = (ParameterRef) rightOp; @@ -394,8 +393,8 @@ public List getParameterLocals() { Local[] res = new Local[numParams]; int numFound = 0; for (Unit u : getUnits()) { - if (u instanceof IdentityStmt) { - IdentityStmt is = (IdentityStmt) u; + if (u instanceof IdentityUnit) { + IdentityUnit is = (IdentityUnit) u; Value rightOp = is.getRightOp(); if (rightOp instanceof ParameterRef) { int idx = ((ParameterRef) rightOp).getIndex(); @@ -433,8 +432,8 @@ public List getParameterRefs() { Value[] res = new Value[numParams]; int numFound = 0; for (Unit u : getUnits()) { - if (u instanceof IdentityStmt) { - Value rightOp = ((IdentityStmt) u).getRightOp(); + if (u instanceof IdentityUnit) { + Value rightOp = ((IdentityUnit) u).getRightOp(); if (rightOp instanceof ParameterRef) { ParameterRef pr = (ParameterRef) rightOp; int idx = pr.getIndex(); diff --git a/src/main/java/soot/toolkits/scalar/FlowAnalysis.java b/src/main/java/soot/toolkits/scalar/FlowAnalysis.java index d37dbb3253c..8568f08f1b3 100644 --- a/src/main/java/soot/toolkits/scalar/FlowAnalysis.java +++ b/src/main/java/soot/toolkits/scalar/FlowAnalysis.java @@ -30,11 +30,11 @@ import java.util.HashMap; import java.util.HashSet; import java.util.IdentityHashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Queue; import java.util.RandomAccess; -import java.util.Set; import soot.baf.GotoInst; import soot.jimple.GotoStmt; @@ -129,59 +129,52 @@ List> newUniverse(DirectedGraph g, GraphView gv, F entryFl // out of universe node Entry superEntry = new Entry(null, null); - List entries = null; - List actualEntries = gv.getEntries(g); - - if (!actualEntries.isEmpty()) { - // normal cases: there is at least - // one return statement for a backward analysis - // or one entry statement for a forward analysis - entries = actualEntries; - } else { - // cases without any entry statement - + List entries; + { + List actualEntries = gv.getEntries(g); if (isForward) { - // case of a forward flow analysis on - // a method without any entry point - throw new RuntimeException("error: no entry point for method in forward analysis"); + if (!actualEntries.isEmpty()) { + // normal case: there is at least one entry statement + entries = actualEntries; + } else { + // error case: forward flow analysis on a method without any entry point + throw new RuntimeException("error: forward analysis on an empty entry set."); + } } else { - // case of backward analysis on - // a method which potentially has - // an infinite loop and no return statement - entries = new ArrayList(); - - // a single head is expected - assert g.getHeads().size() == 1; - D head = g.getHeads().get(0); - - // collect all 'goto' statements to catch the 'goto' from the infinite loop - Set visitedNodes = new HashSet(); - List workList = new ArrayList(); - workList.add(head); + // In a backward analysis, entry points must include any return + // statements that exist (i.e. the 'actualEntries') but there + // could also be infinite loops with no return statement that + // must be included in the analysis as well. + ArrayList extraEntries = new ArrayList(); + + // Collect all 'goto' statements to capture the 'goto' from infinite loop(s) + HashSet visitedNodes = new HashSet(); + LinkedList workList = new LinkedList(); + workList.addAll(g.getHeads()); for (D current; !workList.isEmpty();) { current = workList.remove(0); visitedNodes.add(current); - + // only add 'goto' statements if (current instanceof GotoInst || current instanceof GotoStmt) { - entries.add(current); + extraEntries.add(current); } - + for (D next : g.getSuccsOf(current)) { - if (visitedNodes.contains(next)) { - continue; + if (!visitedNodes.contains(next)) { + workList.add(next); } - workList.add(next); } } - - // - if (entries.isEmpty()) { + if (actualEntries.isEmpty() && extraEntries.isEmpty()) { throw new RuntimeException("error: backward analysis on an empty entry set."); } + entries = new ArrayList(actualEntries.size() + extraEntries.size()); + entries.addAll(actualEntries); + entries.addAll(extraEntries); } } - + visitEntry(visited, superEntry, entries); superEntry.inFlow = entryFlow; superEntry.outFlow = entryFlow; diff --git a/src/test/java/soot/toolkits/scalar/SimpleLiveLocalsTest.java b/src/test/java/soot/toolkits/scalar/SimpleLiveLocalsTest.java new file mode 100644 index 00000000000..c8f952533fe --- /dev/null +++ b/src/test/java/soot/toolkits/scalar/SimpleLiveLocalsTest.java @@ -0,0 +1,336 @@ +package soot.toolkits.scalar; + +/*- + * #%L + * Soot - a J*va Optimization Framework + * %% + * Copyright (C) 2021 Timothy Hoffman + * %% + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation, either version 2.1 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Lesser Public License for more details. + * + * You should have received a copy of the GNU General Lesser Public + * License along with this program. If not, see + * . + * #L% + */ +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Iterator; + +import org.junit.Assert; +import org.junit.Test; + +import soot.G; +import soot.Local; +import soot.RefType; +import soot.Scene; +import soot.SootClass; +import soot.SootMethod; +import soot.Unit; +import soot.VoidType; +import soot.baf.Baf; +import soot.baf.BafBody; +import soot.jimple.Jimple; +import soot.jimple.JimpleBody; +import soot.jimple.NullConstant; +import soot.jimple.Stmt; +import soot.options.Options; +import soot.toolkits.graph.ExceptionalUnitGraph; +import soot.util.Chain; +import soot.util.PhaseDumper; + +/** + * This test generates a few sample methods that contain a branch where one path returns and the other goes into an infinite + * loop. The {@link SimpleLiveLocals} analysis failed to compute accurate results for units on the infinite loop path + * because the {@link soot.toolkits.scalar.FlowAnalysis} did not attempt to search for a non-returning loop if the was at + * least one return statement found somewhere in the body. The tests are run on both the {@link JimpleBody} and the + * {@link BafBody} which also happened to present a case where several methods in the {@link soot.Body} class considered + * only {@link soot.jimple.IdentityStmt} rather than {@link soot.IdentityUnit} (thus excluding the other subclass, + * {@link soot.baf.IdentityInst} which are found in {@link BafBody} instances. + * + * @author Timothy Hoffman + */ +public class SimpleLiveLocalsTest { + + private static final boolean DEBUG_PRINT = false; + + static { + G.reset(); + Scene.v().loadBasicClasses(); + } + + /** + * Construct {@link #jBody} as the Jimple code for the following: + * + * 
+   * void run(Object running) {
+   *   if (running == null) {
+   *     for (;;) {
+   *       try {
+   *         this.run(running);
+   *       } catch (Throwable x) {
+   *       }
+   *     }
+   *   }
+   * }
+   *
+ */ + private static JimpleBody createA() { + final Jimple jimp = Jimple.v(); + final RefType objTy = Scene.v().getRefType("java.lang.Object"); + final RefType throwTy = Scene.v().getRefType("java.lang.Throwable"); + + final SootClass sc = new SootClass("SimpleLiveLocalsTestDummy"); + final SootMethod m = new SootMethod("run", Collections.singletonList(objTy), VoidType.v()); + sc.addMethod(m); + final JimpleBody jb = jimp.newBody(m); + jb.insertIdentityStmts(sc); + + // Create all of the statements + final Stmt[] stmts = new Stmt[6]; + final Local param0 = jb.getParameterLocal(0); + stmts[5] = jimp.newReturnVoidStmt(); + stmts[0] = jimp.newIfStmt(jimp.newNeExpr(param0, NullConstant.v()), stmts[5]); + stmts[1] = jimp.newInvokeStmt(jimp.newVirtualInvokeExpr(jb.getThisLocal(), m.makeRef(), param0)); + stmts[2] = jimp.newGotoStmt(stmts[1]); + Local exLoc = jimp.newLocal("$e", throwTy); + jb.getLocals().add(exLoc); + stmts[3] = jimp.newIdentityStmt(exLoc, jimp.newCaughtExceptionRef()); + stmts[4] = jimp.newGotoStmt(stmts[1]); + + // Add all statements to the body + jb.getUnits().getNonPatchingChain().addAll(Arrays.asList(stmts)); + + // Add the trap + jb.getTraps().add(jimp.newTrap(throwTy.getSootClass(), stmts[1], stmts[2], stmts[3])); + + return jb; + } + + @Test + public void testNonTerminatingBodyJimpleA() { + final JimpleBody body = createA(); + if (DEBUG_PRINT) { + System.out.println("[testNonTerminatingBodyJimpleA] JimpleBody = " + body); + } + + // Run SimpleLiveLocals + final ExceptionalUnitGraph graph = new ExceptionalUnitGraph(body); + final LiveLocals ll = new SimpleLiveLocals(graph); + if (DEBUG_PRINT) { + PhaseDumper.v().dumpGraph(graph, true); + } + + // Check LiveLocals result + final Local thisLoc = body.getThisLocal(); + final Local paramLoc = body.getParameterLocal(0); + final Chain units = body.getUnits().getNonPatchingChain(); + Assert.assertEquals(8, units.size()); + + final Iterator it = units.iterator(); + // this := @this + check(ll, it.next(), Arrays.asList(), Arrays.asList(thisLoc)); + // parameter0 := @parameter0 + check(ll, it.next(), Arrays.asList(thisLoc), Arrays.asList(thisLoc, paramLoc)); + // if parameter0 != null goto label4 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // this.run(parameter0) + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // goto label1 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // $e := @caughtexception + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // goto label1 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // return + check(ll, it.next(), Arrays.asList(), Arrays.asList()); + + Assert.assertFalse(it.hasNext()); + } + + @Test + public void testNonTerminatingBodyBafA() { + // Disable "bb" Pack for a direct translation from Jimple->Baf + Options.v().setPhaseOption("bb", "enabled:false"); + // Translate the body to Baf + final BafBody body = Baf.v().newBody(createA()); + if (DEBUG_PRINT) { + System.out.println("[testNonTerminatingBodyBafA] BafBody = " + body); + } + + // Run SimpleLiveLocals + final ExceptionalUnitGraph graph = new ExceptionalUnitGraph(body); + final LiveLocals ll = new SimpleLiveLocals(graph); + if (DEBUG_PRINT) { + PhaseDumper.v().dumpGraph(graph, true); + } + + // Check LiveLocals result + final Local thisLoc = body.getThisLocal(); + final Local paramLoc = body.getParameterLocal(0); + final Chain units = body.getUnits().getNonPatchingChain(); + Assert.assertEquals(11, units.size()); + + final Iterator it = units.iterator(); + // this := @this + check(ll, it.next(), Arrays.asList(), Arrays.asList(thisLoc)); + // parameter0 := @parameter0 + check(ll, it.next(), Arrays.asList(thisLoc), Arrays.asList(thisLoc, paramLoc)); + // load.r parameter0 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // ifnonnull label4 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // load.r this + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // load.r parameter0 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // invoke run(Object) + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // goto label1 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // store.r $e + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // goto label1 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // return + check(ll, it.next(), Arrays.asList(), Arrays.asList()); + + Assert.assertFalse(it.hasNext()); + } + + /** + * Construct {@link #jBody} as the Jimple code for the following: + * + * 
+   * void run(Object running) {
+   *   if (running == null) {
+   *     for (;;) {
+   *       this.run(running);
+   *     }
+   *   }
+   * }
+   * }
+   *
+ */ + private static JimpleBody createB() { + final Jimple jimp = Jimple.v(); + final RefType objTy = Scene.v().getRefType("java.lang.Object"); + + final SootClass sc = new SootClass("SimpleLiveLocalsTestDummy"); + final SootMethod m = new SootMethod("run", Collections.singletonList(objTy), VoidType.v()); + sc.addMethod(m); + final JimpleBody jb = jimp.newBody(m); + jb.insertIdentityStmts(sc); + + // Create all of the statements + final Stmt[] stmts = new Stmt[4]; + final Local param0 = jb.getParameterLocal(0); + stmts[3] = jimp.newReturnVoidStmt(); + stmts[0] = jimp.newIfStmt(jimp.newNeExpr(param0, NullConstant.v()), stmts[3]); + stmts[1] = jimp.newInvokeStmt(jimp.newVirtualInvokeExpr(jb.getThisLocal(), m.makeRef(), param0)); + stmts[2] = jimp.newGotoStmt(stmts[1]); + + // Add all statements to the body + jb.getUnits().getNonPatchingChain().addAll(Arrays.asList(stmts)); + + return jb; + } + + @Test + public void testNonTerminatingBodyJimpleB() { + final JimpleBody body = createB(); + if (DEBUG_PRINT) { + System.out.println("[testNonTerminatingBodyJimpleB] JimpleBody = " + body); + } + + // Run SimpleLiveLocals + final ExceptionalUnitGraph graph = new ExceptionalUnitGraph(body); + final LiveLocals ll = new SimpleLiveLocals(graph); + if (DEBUG_PRINT) { + PhaseDumper.v().dumpGraph(graph, true); + } + + // Check LiveLocals result + final Local thisLoc = body.getThisLocal(); + final Local paramLoc = body.getParameterLocal(0); + final Chain units = body.getUnits().getNonPatchingChain(); + Assert.assertEquals(6, units.size()); + + final Iterator it = units.iterator(); + // this := @this + check(ll, it.next(), Arrays.asList(), Arrays.asList(thisLoc)); + // parameter0 := @parameter0 + check(ll, it.next(), Arrays.asList(thisLoc), Arrays.asList(thisLoc, paramLoc)); + // if parameter0 != null goto label2 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // this.run(parameter0) + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // goto label1 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // return + check(ll, it.next(), Arrays.asList(), Arrays.asList()); + + Assert.assertFalse(it.hasNext()); + } + + @Test + public void testNonTerminatingBodyBafB() { + // Disable "bb" Pack for a direct translation from Jimple->Baf + Options.v().setPhaseOption("bb", "enabled:false"); + // Translate the body to Baf + final BafBody body = Baf.v().newBody(createB()); + if (DEBUG_PRINT) { + System.out.println("[testNonTerminatingBodyBafB] BafBody = " + body); + } + + // Run SimpleLiveLocals + final ExceptionalUnitGraph graph = new ExceptionalUnitGraph(body); + final LiveLocals ll = new SimpleLiveLocals(graph); + if (DEBUG_PRINT) { + PhaseDumper.v().dumpGraph(graph, true); + } + + // Check LiveLocals result + final Local thisLoc = body.getThisLocal(); + final Local paramLoc = body.getParameterLocal(0); + final Chain units = body.getUnits().getNonPatchingChain(); + Assert.assertEquals(9, units.size()); + + final Iterator it = units.iterator(); + // this := @this + check(ll, it.next(), Arrays.asList(), Arrays.asList(thisLoc)); + // parameter0 := @parameter0 + check(ll, it.next(), Arrays.asList(thisLoc), Arrays.asList(thisLoc, paramLoc)); + // load.r parameter0 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // ifnonnull label4 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // load.r this + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // load.r parameter0 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // invoke run(Object) + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // goto label1 + check(ll, it.next(), Arrays.asList(thisLoc, paramLoc), Arrays.asList(thisLoc, paramLoc)); + // return + check(ll, it.next(), Arrays.asList(), Arrays.asList()); + + Assert.assertFalse(it.hasNext()); + } + + private static void check(LiveLocals ll, Unit u, Collection expectBefore, Collection expectAfter) { + Assert.assertEquals("before(" + u + ")", new HashSet<>(expectBefore), new HashSet<>(ll.getLiveLocalsBefore(u))); + Assert.assertEquals("after(" + u + ")", new HashSet<>(expectAfter), new HashSet<>(ll.getLiveLocalsAfter(u))); + } +} From b3e7ff62711c877602a96fd1e6b63847dbb8ca9a Mon Sep 17 00:00:00 2001 From: Timothy Hoffman <4001421+tim-hoffman@users.noreply.github.com> Date: Thu, 1 Jul 2021 18:24:29 -0500 Subject: [PATCH 2/2] avoid duplicate adds which could lead to heap exhaustion on large methods --- .../soot/toolkits/scalar/FlowAnalysis.java | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/main/java/soot/toolkits/scalar/FlowAnalysis.java b/src/main/java/soot/toolkits/scalar/FlowAnalysis.java index 8568f08f1b3..53821ced5bb 100644 --- a/src/main/java/soot/toolkits/scalar/FlowAnalysis.java +++ b/src/main/java/soot/toolkits/scalar/FlowAnalysis.java @@ -149,20 +149,19 @@ List> newUniverse(DirectedGraph g, GraphView gv, F entryFl // Collect all 'goto' statements to capture the 'goto' from infinite loop(s) HashSet visitedNodes = new HashSet(); - LinkedList workList = new LinkedList(); - workList.addAll(g.getHeads()); - for (D current; !workList.isEmpty();) { - current = workList.remove(0); - visitedNodes.add(current); - - // only add 'goto' statements - if (current instanceof GotoInst || current instanceof GotoStmt) { - extraEntries.add(current); - } - - for (D next : g.getSuccsOf(current)) { - if (!visitedNodes.contains(next)) { - workList.add(next); + LinkedList workList = new LinkedList(g.getHeads()); + while (!workList.isEmpty()) { + D current = workList.remove(0); + if (visitedNodes.add(current)) { + // only add 'goto' statements + if (current instanceof GotoStmt || current instanceof GotoInst) { + extraEntries.add(current); + } + + for (D next : g.getSuccsOf(current)) { + if (!visitedNodes.contains(next)) { + workList.add(next); + } } } } @@ -173,6 +172,8 @@ List> newUniverse(DirectedGraph g, GraphView gv, F entryFl entries.addAll(actualEntries); entries.addAll(extraEntries); } + assert (!entries.isEmpty()); + assert (entries.stream().distinct().count() == entries.size()); } visitEntry(visited, superEntry, entries);