diff --git a/shared-test-resources/bugfixes/ConditionalBranchFolderTest.class b/shared-test-resources/bugfixes/ConditionalBranchFolderTest.class new file mode 100644 index 0000000000..6acf22e302 Binary files /dev/null and b/shared-test-resources/bugfixes/ConditionalBranchFolderTest.class differ diff --git a/shared-test-resources/bugfixes/ConditionalBranchFolderTest.java b/shared-test-resources/bugfixes/ConditionalBranchFolderTest.java new file mode 100644 index 0000000000..a0e0296a29 --- /dev/null +++ b/shared-test-resources/bugfixes/ConditionalBranchFolderTest.java @@ -0,0 +1,55 @@ +public class ConditionalBranchFolderTest { + void tc1() { + boolean bool = true; + if (!bool) { + System.out.println("False 1"); + } else if (bool) { + if (bool){ + System.out.println("lets see"); + } + System.out.println("mid"); + } + if (!bool) { + System.out.println("False 2"); + } + } + + void tc2() { + boolean bool = true; + try { + if (bool) { + throw new Exception("True"); + } + } catch (Exception e) { + e.printStackTrace(); + } + } + + void tc3() { + boolean bool = false; + try { + if (bool) { + throw new Exception("True"); + } + } catch (Exception e) { + e.printStackTrace(); + } + } + + void tc4() { + int x = 10; + boolean bool = true; + if(x > 5) { + try { + System.out.println("Try Block"); + if (bool) { + System.out.println("True inside Try"); + } + } catch (Exception e) { + e.printStackTrace(); + } + } + System.out.println(""); + } + +} \ No newline at end of file diff --git a/shared-test-resources/bugfixes/LineIterator.class b/shared-test-resources/bugfixes/LineIterator.class new file mode 100644 index 0000000000..d8195b9951 Binary files /dev/null and b/shared-test-resources/bugfixes/LineIterator.class differ diff --git a/shared-test-resources/bugfixes/LineIterator.java b/shared-test-resources/bugfixes/LineIterator.java new file mode 100644 index 0000000000..e1e300eb89 --- /dev/null +++ b/shared-test-resources/bugfixes/LineIterator.java @@ -0,0 +1,85 @@ +import java.io.BufferedReader; +import java.io.Closeable; +import java.io.IOException; +import java.io.Reader; +import java.util.Iterator; +import java.util.NoSuchElementException; + +public class LineIterator implements Iterator, Closeable { + private final BufferedReader bufferedReader; + private String cachedLine; + private boolean finished; + + public LineIterator(Reader reader) throws IllegalArgumentException { + if (reader == null) { + throw new IllegalArgumentException("Reader must not be null"); + } else { + if (reader instanceof BufferedReader) { + this.bufferedReader = (BufferedReader)reader; + } else { + this.bufferedReader = new BufferedReader(reader); + } + + } + } + + public boolean hasNext() { + if (this.cachedLine != null) { + return true; + } else if (this.finished) { + return false; + } else { + try { + String line; + do { + line = this.bufferedReader.readLine(); + if (line == null) { + this.finished = true; + return false; + } + } while(!this.isValidLine(line)); + + this.cachedLine = line; + return true; + } catch (IOException var2) { + IOException ioe = var2; + // IOUtils.closeQuietly(this, ioe::addSuppressed); + throw new IllegalStateException(ioe); + } + } + } + + protected boolean isValidLine(String line) { + return true; + } + + public String next() { + return this.nextLine(); + } + + public String nextLine() { + if (!this.hasNext()) { + throw new NoSuchElementException("No more lines"); + } else { + String currentLine = this.cachedLine; + this.cachedLine = null; + return currentLine; + } + } + + public void close() throws IOException { + this.finished = true; + this.cachedLine = null; + // IOUtils.close(this.bufferedReader); + } + + public void remove() { + throw new UnsupportedOperationException("remove not supported"); + } + + /** @deprecated */ + @Deprecated + public static void closeQuietly(LineIterator iterator) { + // IOUtils.closeQuietly(iterator); + } +} diff --git a/shared-test-resources/bugfixes/NestedTryCatchFinally.java b/shared-test-resources/bugfixes/NestedTryCatchFinally.java new file mode 100644 index 0000000000..938700fd60 --- /dev/null +++ b/shared-test-resources/bugfixes/NestedTryCatchFinally.java @@ -0,0 +1,25 @@ +import java.io.FileInputStream; +import java.io.File; +import java.io.ObjectInputStream; + +public class NestedTryCatchFinally { + + private static String test0(File storedResults) throws Exception { + try { + FileInputStream file = new FileInputStream(storedResults); + try { + ObjectInputStream stream = new ObjectInputStream(file); + try { + return (String) stream.readObject(); + } finally { + stream.close(); + } + } finally { + file.close(); + } + } catch (Exception e) { + throw new Exception(e); + } + } + +} \ No newline at end of file diff --git a/sootup.core/src/main/java/sootup/core/graph/MutableBlockStmtGraph.java b/sootup.core/src/main/java/sootup/core/graph/MutableBlockStmtGraph.java index 119c7d75fb..4fc5a38c09 100644 --- a/sootup.core/src/main/java/sootup/core/graph/MutableBlockStmtGraph.java +++ b/sootup.core/src/main/java/sootup/core/graph/MutableBlockStmtGraph.java @@ -577,10 +577,22 @@ public void removeBlock(BasicBlock block) { blockOf.clearPredecessorBlocks(); blockOf.clearSuccessorBlocks(); blockOf.clearExceptionalSuccessorBlocks(); - blocks.remove(blockOf); } + @Override + public void replaceStmt(@Nonnull Stmt oldStmt, @Nonnull Stmt newStmt) { + Pair blockPair = stmtToBlock.get(oldStmt); + if (blockPair == null) { + // Stmt does not exist in the graph + throw new IllegalArgumentException("splitStmt does not exist in this block!"); + } + MutableBasicBlock block = blockPair.getRight(); + block.replaceStmt(oldStmt, newStmt); + stmtToBlock.remove(oldStmt); + stmtToBlock.put(newStmt, blockPair); + } + @Override public void addNode(@Nonnull Stmt stmt, @Nonnull Map exceptions) { Pair blockPair = stmtToBlock.get(stmt); @@ -1276,6 +1288,24 @@ protected void putEdge_internal(@Nonnull Stmt stmtA, int succesorIdx, @Nonnull S } } + @Override + public void unLinkNodes(@Nonnull Stmt from, @Nonnull Stmt to) { + Pair blockOfFromPair = stmtToBlock.get(from); + if (blockOfFromPair == null) { + throw new IllegalArgumentException("stmt '" + from + "' does not exist in this StmtGraph!"); + } + MutableBasicBlock blockOfFrom = blockOfFromPair.getRight(); + Pair blockOfToPair = stmtToBlock.get(to); + if (blockOfToPair == null) { + throw new IllegalArgumentException("stmt '" + to + "' does not exist in this StmtGraph!"); + } + MutableBasicBlock blockOfTo = blockOfToPair.getRight(); + + // Unlink 2 blocks + blockOfFrom.removeFromSuccessorBlocks(blockOfTo); + blockOfTo.removePredecessorBlock(blockOfFrom); + } + @Override public List removeEdge(@Nonnull Stmt from, @Nonnull Stmt to) { Pair blockOfFromPair = stmtToBlock.get(from); diff --git a/sootup.core/src/main/java/sootup/core/graph/MutableStmtGraph.java b/sootup.core/src/main/java/sootup/core/graph/MutableStmtGraph.java index 48ebfe01a5..3a32457aef 100644 --- a/sootup.core/src/main/java/sootup/core/graph/MutableStmtGraph.java +++ b/sootup.core/src/main/java/sootup/core/graph/MutableStmtGraph.java @@ -45,6 +45,9 @@ public void addNode(@Nonnull Stmt stmt) { addNode(stmt, Collections.emptyMap()); } + /** replace a "oldStmt" with "newStmt" in the StmtGraph */ + public abstract void replaceStmt(@Nonnull Stmt oldStmt, @Nonnull Stmt newStmt); + /** inserts a "stmt" with exceptional flows "traps" into the StmtGraph */ public abstract void addNode(@Nonnull Stmt stmt, @Nonnull Map traps); @@ -107,6 +110,8 @@ public void setEdges(@Nonnull BranchingStmt from, @Nonnull Stmt... targets) { setEdges(from, Arrays.asList(targets)); } + public abstract void unLinkNodes(@Nonnull Stmt from, @Nonnull Stmt to); + /** * removes the current outgoing flows of "from" to "to" * diff --git a/sootup.core/src/main/java/sootup/core/graph/StmtGraph.java b/sootup.core/src/main/java/sootup/core/graph/StmtGraph.java index 3a484d7f90..a8a47d9d82 100644 --- a/sootup.core/src/main/java/sootup/core/graph/StmtGraph.java +++ b/sootup.core/src/main/java/sootup/core/graph/StmtGraph.java @@ -21,6 +21,8 @@ * #L% */ +import static sootup.core.jimple.common.stmt.FallsThroughStmt.FALLTSTHROUH_IDX; + import com.google.common.collect.Iterators; import java.io.PrintWriter; import java.io.StringWriter; @@ -492,178 +494,222 @@ public List getTraps() { } } + protected static class IteratorFrame + implements Comparable, Iterable> { + private final int weightSum; + private final Deque> sequence; + + protected IteratorFrame(Deque> sequence, int weightSum) { + this.weightSum = weightSum; + this.sequence = sequence; + } + + @Override + public int compareTo(IteratorFrame o) { + return Integer.compare(weightSum, o.weightSum); + } + + public int getWeight() { + return weightSum; + } + + public int size() { + return sequence.size(); + } + + @Nonnull + @Override + public Iterator> iterator() { + return sequence.iterator(); + } + } + /** Iterates over the blocks */ protected class BlockGraphIterator implements Iterator> { - @Nonnull private final ArrayDeque> trapHandlerBlocks = new ArrayDeque<>(); + private final PriorityQueue worklist = new PriorityQueue<>(); + private IteratorFrame itFrame; + private Iterator> itBlock; - @Nonnull private final ArrayDeque> nestedBlocks = new ArrayDeque<>(); - @Nonnull private final ArrayDeque> otherBlocks = new ArrayDeque<>(); - @Nonnull private final Set> iteratedBlocks; + @Nonnull + private final Map, Deque>> fallsThroughSequenceMap = + new IdentityHashMap<>(); + + @Nonnull private final Set> seenTargets; + private int iterations = 0; + private int minValue = 0; + private int maxValue = 0; public BlockGraphIterator() { + System.out.println("==============0"); final Collection> blocks = getBlocks(); - iteratedBlocks = new LinkedHashSet<>(blocks.size(), 1); + seenTargets = new LinkedHashSet<>(blocks.size(), 1); Stmt startingStmt = getStartingStmt(); if (startingStmt != null) { final BasicBlock startingBlock = getStartingStmtBlock(); - updateFollowingBlocks(startingBlock); - nestedBlocks.addFirst(startingBlock); + itFrame = new IteratorFrame(calculateFallsThroughSequence(startingBlock), 0); + itBlock = itFrame.iterator(); + updateFollowingBlocks(startingBlock, 0); } } - @Nullable - private BasicBlock retrieveNextBlock() { - BasicBlock nextBlock; - do { - if (!nestedBlocks.isEmpty()) { - nextBlock = nestedBlocks.pollFirst(); - } else if (!trapHandlerBlocks.isEmpty()) { - nextBlock = trapHandlerBlocks.pollFirst(); - } else if (!otherBlocks.isEmpty()) { - nextBlock = otherBlocks.pollFirst(); - } else { - Collection> blocks = getBlocks(); - if (iteratedBlocks.size() < blocks.size()) { - // graph is not connected! iterate/append all not connected blocks at the end in no - // particular order. - for (BasicBlock block : blocks) { - if (!iteratedBlocks.contains(block)) { - nestedBlocks.addLast(block); - } - } - if (!nestedBlocks.isEmpty()) { - return nestedBlocks.pollFirst(); - } - } + protected Deque> calculateFallsThroughSequence(@Nonnull BasicBlock param) { + Deque> basicBlockSequence = fallsThroughSequenceMap.get(param); + if (basicBlockSequence != null) { + return basicBlockSequence; + } - return null; + Deque> blockSequence = new ArrayDeque<>(); + + BasicBlock continousBlockSequenceHeadCandidate = param; + // TODO: [ms] looks ugly.. simplify readability of the loop! + // find the leader of the Block Sequence (connected via FallsthroughStmts) + while (true) { + blockSequence.addFirst(continousBlockSequenceHeadCandidate); + final List> itPreds = + continousBlockSequenceHeadCandidate.getPredecessors(); + BasicBlock continousBlockTailCandidate = continousBlockSequenceHeadCandidate; + final Optional> fallsthroughPredOpt = + itPreds.stream() + .filter( + b -> + b.getTail().fallsThrough() + && b.getSuccessors().get(FALLTSTHROUH_IDX) + == continousBlockTailCandidate) + .findAny(); + if (!fallsthroughPredOpt.isPresent()) { + break; } + BasicBlock predecessorBlock = fallsthroughPredOpt.get(); + if (predecessorBlock.getTail().fallsThrough() + && predecessorBlock.getSuccessors().get(FALLTSTHROUH_IDX) + == continousBlockSequenceHeadCandidate) { + // TODO: [ms] seems tautologic + continousBlockSequenceHeadCandidate = predecessorBlock; + } else { + break; + } + } + + // iterate to the end of the sequence + BasicBlock continousBlockSequenceTailCandidate = param; + while (continousBlockSequenceTailCandidate.getTail().fallsThrough()) { + continousBlockSequenceTailCandidate = + continousBlockSequenceTailCandidate.getSuccessors().get(0); + blockSequence.addLast(continousBlockSequenceTailCandidate); + } - // skip retrieved nextBlock if its already returned - } while (iteratedBlocks.contains(nextBlock)); - return nextBlock; + // cache calculated sequence for every block in the sequence + for (BasicBlock basicBlock : blockSequence) { + fallsThroughSequenceMap.put(basicBlock, blockSequence); + seenTargets.add(basicBlock); + } + return blockSequence; } @Override @Nonnull public BasicBlock next() { - BasicBlock currentBlock = retrieveNextBlock(); - if (currentBlock == null) { - throw new NoSuchElementException("Iterator has no more Blocks."); + if (!itBlock.hasNext()) { + itFrame = worklist.poll(); + if (itFrame == null) { + throw new NoSuchElementException("Iterator has no more Blocks."); + } + itBlock = itFrame.iterator(); } - updateFollowingBlocks(currentBlock); - iteratedBlocks.add(currentBlock); + + BasicBlock currentBlock = itBlock.next(); + System.out.println(++iterations + ": "); + updateFollowingBlocks(currentBlock, itFrame.getWeight()); return currentBlock; } - private void updateFollowingBlocks(BasicBlock currentBlock) { - // collect traps + private void updateFollowingBlocks(BasicBlock currentBlock, int currentWeight) { + final Stmt tailStmt = currentBlock.getTail(); - for (Map.Entry> entry : - currentBlock.getExceptionalSuccessors().entrySet()) { - BasicBlock trapHandlerBlock = entry.getValue(); - trapHandlerBlocks.addLast(trapHandlerBlock); - nestedBlocks.addFirst(trapHandlerBlock); + final List> successors = currentBlock.getSuccessors(); + final int startIdx = tailStmt.fallsThrough() ? 1 : 0; + + // handle the branching successor(s) + for (int i = startIdx; i < successors.size(); i++) { + // find the leader/beginning block of a continuous sequence of Blocks (connected via + // FallsThroughStmts) + final BasicBlock successorBlock = successors.get(i); + + enqueueWeighted(currentWeight+=getBlocks().size(), currentBlock, successorBlock); } - final List> successors = currentBlock.getSuccessors(); + for (BasicBlock trapHandlerBlock : currentBlock.getExceptionalSuccessors().values()) { + enqueueWeighted(currentWeight+=getBlocks().size(), currentBlock, trapHandlerBlock); + } + } - for (int i = successors.size() - 1; i >= 0; i--) { - if (i == 0 && tailStmt.fallsThrough()) { - // non-branching successors i.e. not a BranchingStmt or is the first successor (i.e. its - // false successor) of - // JIfStmt - nestedBlocks.addFirst(successors.get(0)); - } else { + private void enqueueWeighted(int currentWeight, BasicBlock currentBlock, BasicBlock successorBlock) { + if (seenTargets.contains(successorBlock)) { + return; + } - // create the longest FallsThroughStmt sequence possible - final BasicBlock successorBlock = successors.get(i); - BasicBlock leaderOfFallsthroughBlocks = successorBlock; - while (true) { - final List> itPreds = - leaderOfFallsthroughBlocks.getPredecessors(); - - BasicBlock finalLeaderOfFallsthroughBlocks = leaderOfFallsthroughBlocks; - final Optional> fallsthroughPredOpt = - itPreds.stream() - .filter( - b -> - b.getTail().fallsThrough() - && b.getSuccessors().get(0) == finalLeaderOfFallsthroughBlocks) - .findAny(); - if (!fallsthroughPredOpt.isPresent()) { - break; - } - BasicBlock predecessorBlock = fallsthroughPredOpt.get(); - if (predecessorBlock.getTail().fallsThrough() - && predecessorBlock.getSuccessors().get(0) == leaderOfFallsthroughBlocks) { - leaderOfFallsthroughBlocks = predecessorBlock; - } else { - break; - } - } + iterations++; + Deque> blockSequence = calculateFallsThroughSequence(successorBlock); + BasicBlock lastBlockOfSequence = blockSequence.getLast(); + boolean isSequenceTailExceptionFree = + lastBlockOfSequence.getExceptionalSuccessors().isEmpty(); - // find a return Stmt inside the current Block - Stmt succTailStmt = successorBlock.getTail(); - boolean hasNoSuccessorStmts = succTailStmt.getExpectedSuccessorCount() == 0; - boolean isExceptionFree = successorBlock.getExceptionalSuccessors().isEmpty(); - - boolean isLastStmtCandidate = hasNoSuccessorStmts && isExceptionFree; - // remember branching successors - if (tailStmt instanceof JGotoStmt) { - if (isLastStmtCandidate) { - nestedBlocks.removeFirstOccurrence(currentBlock); - otherBlocks.addLast(leaderOfFallsthroughBlocks); - } else { - otherBlocks.addFirst(leaderOfFallsthroughBlocks); - } - } else if (!nestedBlocks.contains(leaderOfFallsthroughBlocks)) { - // JSwitchStmt, JIfStmt - if (isLastStmtCandidate) { - nestedBlocks.addLast(leaderOfFallsthroughBlocks); - } else { - nestedBlocks.addFirst(leaderOfFallsthroughBlocks); - } - } + + if(currentBlock.getTail() instanceof JIfStmt && currentBlock.getTail() instanceof JSwitchStmt){ + // currentWeight = minValue-getBlocks().size(); + } else if(currentBlock.getTail() instanceof JGotoStmt){ + // currentWeight++; + } + + + int categoryWeight = 2; + if (isSequenceTailExceptionFree) { + if (lastBlockOfSequence.getTail() instanceof JReturnStmt + || lastBlockOfSequence.getTail() instanceof JReturnVoidStmt) { + // biggest number, yet - only bigger weight if there follows another JReturn(Void)Stmt + categoryWeight = 0; + } else { + categoryWeight = 1; } } + + IteratorFrame frame = new IteratorFrame(blockSequence, (currentWeight-iterations) - categoryWeight*getBlocks().size() ); + worklist.add(frame); + minValue = Math.min(frame.getWeight(), minValue); + maxValue = Math.max(frame.getWeight(), maxValue); + System.out.println("weight: " +(currentWeight-iterations)+ " it:"+iterations + " sum:" +frame.weightSum + " :: " +currentBlock +" => " + blockSequence); } @Override public boolean hasNext() { - final boolean hasIteratorMoreElements; - BasicBlock b = retrieveNextBlock(); - if (b != null) { - // reinsert at FIRST position -> not great for performance - but easier handling in - // next() - nestedBlocks.addFirst(b); - hasIteratorMoreElements = true; - } else { - hasIteratorMoreElements = false; + // "assertion" that all elements are iterated + if (itBlock.hasNext()) { + return true; } - // "assertion" that all elements are iterated - if (!hasIteratorMoreElements) { - final int returnedSize = iteratedBlocks.size(); - final Collection> blocks = getBlocks(); - final int actualSize = blocks.size(); - if (returnedSize != actualSize) { - String info = - blocks.stream() - .filter(n -> !iteratedBlocks.contains(n)) - .map(BasicBlock::getStmts) - .collect(Collectors.toList()) - .toString(); - throw new IllegalStateException( - "There are " - + (actualSize - returnedSize) - + " Blocks that are not iterated! i.e. the StmtGraph is not connected from its startingStmt!" - + info - + DotExporter.createUrlToWebeditor(StmtGraph.this)); - } + if (!worklist.isEmpty()) { + return true; } - return hasIteratorMoreElements; + + final Collection> blocks = getBlocks(); + final int actualSize = blocks.size(); + if (seenTargets.size() != actualSize) { + String info = + blocks.stream() + .filter(n -> !seenTargets.contains(n)) + .map(BasicBlock::getStmts) + .collect(Collectors.toList()) + .toString(); + throw new IllegalStateException( + "There are " + + (actualSize - seenTargets.size()) + + " Blocks that are not iterated! i.e. the StmtGraph is not connected from its startingStmt!" + + info + + DotExporter.createUrlToWebeditor(StmtGraph.this)); + } + + return false; } } diff --git a/sootup.core/src/main/java/sootup/core/jimple/common/stmt/FallsThroughStmt.java b/sootup.core/src/main/java/sootup/core/jimple/common/stmt/FallsThroughStmt.java index 0568cdcaa4..a205a0ecb3 100644 --- a/sootup.core/src/main/java/sootup/core/jimple/common/stmt/FallsThroughStmt.java +++ b/sootup.core/src/main/java/sootup/core/jimple/common/stmt/FallsThroughStmt.java @@ -25,6 +25,8 @@ /** as an equivalent to BranchingStmt */ public interface FallsThroughStmt extends Stmt { + int FALLTSTHROUH_IDX = 0; + // has to return true in subclasses! // hint: this class can't be abstract and method final because of JIfStmt which would need // FallsThrough and BranchingStmt as parent. diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/TryCatchFinallyTests.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/TryCatchFinallyTests.java new file mode 100644 index 0000000000..5c0dd9f219 --- /dev/null +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/TryCatchFinallyTests.java @@ -0,0 +1,71 @@ +package sootup.java.bytecode; + +import categories.TestCategories; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import com.google.common.collect.Lists; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import sootup.core.inputlocation.AnalysisInputLocation; +import sootup.core.jimple.basic.Trap; +import sootup.core.model.SourceType; +import sootup.core.signatures.MethodSignature; +import sootup.java.bytecode.inputlocation.PathBasedAnalysisInputLocation; +import sootup.java.core.interceptors.LocalSplitter; +import sootup.java.core.interceptors.TypeAssigner; +import sootup.java.core.views.JavaView; + +@Tag(TestCategories.JAVA_8_CATEGORY) +public class TryCatchFinallyTests { + + @Test + public void testTryWithResourcesFinally() { + Path classFilePath = + Paths.get("../shared-test-resources/bugfixes/TryWithResourcesFinally.class"); + + AnalysisInputLocation inputLocation = + new PathBasedAnalysisInputLocation.ClassFileBasedAnalysisInputLocation( + classFilePath, "", SourceType.Application); + JavaView view = new JavaView(Collections.singletonList(inputLocation)); + + MethodSignature methodSignature = + view.getIdentifierFactory() + .parseMethodSignature(""); + List traps = view.getMethod(methodSignature).get().getBody().getTraps(); + } + + @Test + public void testNestedTryCatchFinally() { + Path classFilePath = Paths.get("../shared-test-resources/bugfixes/NestedTryCatchFinally.class"); + + AnalysisInputLocation inputLocation = + new PathBasedAnalysisInputLocation.ClassFileBasedAnalysisInputLocation( + classFilePath, "", SourceType.Application); + JavaView view = new JavaView(Collections.singletonList(inputLocation)); + + MethodSignature methodSignature = + view.getIdentifierFactory() + .parseMethodSignature(""); + List traps = view.getMethod(methodSignature).get().getBody().getTraps(); + } + + @Test + public void testTryCatchFinallyIterator() { + Path classFilePath = Paths.get("../shared-test-resources/bugfixes/LineIterator.class"); + + AnalysisInputLocation inputLocation = + new PathBasedAnalysisInputLocation.ClassFileBasedAnalysisInputLocation( + classFilePath, "", SourceType.Application, Arrays.asList(new LocalSplitter(), new TypeAssigner())); + JavaView view = new JavaView(Collections.singletonList(inputLocation)); + view.getClasses().forEach(clazz -> { + clazz.getMethods().forEach(method -> { + view.getMethod(method.getSignature()).get().getBody().getTraps(); + }); + }); + } + +} diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/ConditionalBranchFolderTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/ConditionalBranchFolderTest.java index 4de56f3674..0ee14583ed 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/ConditionalBranchFolderTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/ConditionalBranchFolderTest.java @@ -1,14 +1,18 @@ package sootup.java.bytecode.interceptors; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import categories.TestCategories; -import java.util.Arrays; -import java.util.Collections; -import java.util.Set; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import sootup.core.graph.MutableStmtGraph; +import sootup.core.inputlocation.AnalysisInputLocation; import sootup.core.jimple.Jimple; import sootup.core.jimple.basic.Local; import sootup.core.jimple.basic.StmtPositionInfo; @@ -19,12 +23,15 @@ import sootup.core.jimple.common.stmt.JIfStmt; import sootup.core.jimple.common.stmt.Stmt; import sootup.core.model.Body; +import sootup.core.model.SourceType; import sootup.core.signatures.MethodSignature; import sootup.core.signatures.PackageName; import sootup.core.util.ImmutableUtils; import sootup.core.util.Utils; +import sootup.java.bytecode.inputlocation.PathBasedAnalysisInputLocation; import sootup.java.core.JavaIdentifierFactory; import sootup.java.core.interceptors.ConditionalBranchFolder; +import sootup.java.core.interceptors.CopyPropagator; import sootup.java.core.language.JavaJimple; import sootup.java.core.types.JavaClassType; import sootup.java.core.views.JavaView; @@ -33,6 +40,9 @@ @Tag(TestCategories.JAVA_8_CATEGORY) public class ConditionalBranchFolderTest { + Path classFilePath = + Paths.get("../shared-test-resources/bugfixes/ConditionalBranchFolderTest.class"); + /** * Tests the correct deletion of an if-statement with a constant condition. Transforms from * @@ -46,9 +56,17 @@ public class ConditionalBranchFolderTest { public void testUnconditionalBranching() { Body.BodyBuilder builder = createBodyBuilder(0); new ConditionalBranchFolder().interceptBody(builder, new JavaView(Collections.emptyList())); + Body body = builder.build(); assertEquals( - Arrays.asList("a = \"str\"", "b = \"str\"", "return a"), - Utils.bodyStmtsAsStrings(builder.build())); + Stream.of( + "java.lang.String a, b", + "a = \"str\"", + "b = \"str\"", + "goto label1", + "label1:", + "return b") + .collect(Collectors.toList()), + Utils.filterJimple(body.toString())); } /** @@ -65,8 +83,15 @@ public void testConditionalBranching() { Body processedBody = builder.build(); assertEquals( - Arrays.asList("a = \"str\"", "b = \"different string\"", "return b"), - Utils.bodyStmtsAsStrings(processedBody)); + Stream.of( + "java.lang.String a, b", + "a = \"str\"", + "b = \"different string\"", + "goto label1", + "label1:", + "return a") + .collect(Collectors.toList()), + Utils.filterJimple(processedBody.toString())); } @Test @@ -79,6 +104,85 @@ public void testConditionalBranchingWithNoConclusiveIfCondition() { assertEquals(Utils.bodyStmtsAsStrings(originalBody), Utils.bodyStmtsAsStrings(processedBody)); } + @Test + public void testConditionalBranchFolderWithMultipleBranches() { + AnalysisInputLocation inputLocation = + new PathBasedAnalysisInputLocation.ClassFileBasedAnalysisInputLocation + .ClassFileBasedAnalysisInputLocation( + classFilePath, + "", + SourceType.Application, + Arrays.asList(new CopyPropagator(), new ConditionalBranchFolder())); + JavaView view = new JavaView(Collections.singletonList(inputLocation)); + + final MethodSignature methodSignature = + view.getIdentifierFactory() + .getMethodSignature( + "ConditionalBranchFolderTest", "tc1", "void", Collections.emptyList()); + Body body = view.getMethod(methodSignature).get().getBody(); + assertFalse(body.getStmts().isEmpty()); + assertEquals( + Stream.of( + "ConditionalBranchFolderTest this", + "unknown $stack3, $stack4, l1", + "this := @this: ConditionalBranchFolderTest", + "l1 = 1", + "goto label1", + "label1:", + "goto label2", + "label2:", + "goto label3", + "label3:", + "$stack4 = ", + "virtualinvoke $stack4.(\"lets see\")", + "$stack3 = ", + "virtualinvoke $stack3.(\"mid\")", + "goto label4", + "label4:", + "return") + .collect(Collectors.toList()), + Utils.filterJimple(body.toString())); + } + + @Test + public void testConditionalBranchFolderWithTraps() { + AnalysisInputLocation inputLocation = + new PathBasedAnalysisInputLocation.ClassFileBasedAnalysisInputLocation + .ClassFileBasedAnalysisInputLocation( + classFilePath, + "", + SourceType.Application, + Arrays.asList(new CopyPropagator(), new ConditionalBranchFolder())); + JavaView view = new JavaView(Collections.singletonList(inputLocation)); + + final MethodSignature methodSignature = + view.getIdentifierFactory() + .getMethodSignature( + "ConditionalBranchFolderTest", "tc2", "void", Collections.emptyList()); + Body body = view.getMethod(methodSignature).get().getBody(); + assertEquals(5, body.getStmtGraph().getBlocks().size()); + + List actualStmts = Utils.bodyStmtsAsStrings(body); + assertEquals( + Stream.of( + "this := @this: ConditionalBranchFolderTest", + "l1 = 1", + "label1:", + "goto label2", + "label2:", + "$stack3 = new java.lang.Exception", + "specialinvoke $stack3.(java.lang.String)>(\"True\")", + "throw $stack3", + "label3:", + "$stack4 := @caughtexception", + "l2 = $stack4", + "virtualinvoke $stack4.()", + "return", + "catch java.lang.Exception from label1 to label3 with label3") + .collect(Collectors.toList()), + actualStmts); + } + /** * Generates the correct test {@link Body} for the corresponding test case. * diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/LabelledLoopBreakTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/LabelledLoopBreakTest.java index 6e9d946cbe..cbd72b18ab 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/LabelledLoopBreakTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/LabelledLoopBreakTest.java @@ -46,12 +46,15 @@ public List expectedBodyStmts() { "if l2 >= 5 goto label4", "if l1 != 1 goto label3", "goto label5", + "label3:", "l2 = l2 + 1", "goto label2", + "label4:", "l1 = l1 + 1", "goto label1", + "label5:", "return") .collect(Collectors.toList()); diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/SwitchCaseStatementTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/SwitchCaseStatementTest.java index 51bad2ddc0..f2273b7f31 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/SwitchCaseStatementTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/SwitchCaseStatementTest.java @@ -153,20 +153,25 @@ public void switchCaseStatementCaseIncludingIf() { "case 2: goto label3", "case 3: goto label4", "default: goto label5", + "label1:", "l2 = 1", "if l1 != 666 goto label2", "l2 = 11", "goto label6", + "label2:", "l2 = 12", "goto label6", + "label3:", "l2 = 2", "goto label6", + "label4:", "l2 = 3", "goto label6", + "label5:", "l2 = -1", "label6:", diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/TryCatchFinallyTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/TryCatchFinallyTest.java index 9d52baf8fb..691060be2d 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/TryCatchFinallyTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/minimaltestsuite/java6/TryCatchFinallyTest.java @@ -139,23 +139,25 @@ public List expectedBodyStmtsTryCatchFinally() { "label1:", "l1 = \"try\"", "$stack4 = ", - "virtualinvoke $stack4.(l1)", + "virtualinvoke $stack4.(l1)", // <-- endrange:handler:label3,label5 "label2:", "l1 = \"finally\"", "$stack5 = ", "virtualinvoke $stack5.(l1)", - "goto label6", + "goto label6", // <-- label6 + "label3:", "$stack8 := @caughtexception", "l2 = $stack8", "l1 = \"catch\"", "$stack9 = ", - "virtualinvoke $stack9.(l1)", + "virtualinvoke $stack9.(l1)", // <-- endrange:handler:label5 "label4:", "l1 = \"finally\"", "$stack10 = ", "virtualinvoke $stack10.(l1)", - "goto label6", + "goto label6", // <-- label6 + "label5:", "$stack6 := @caughtexception", "l3 = $stack6", @@ -163,8 +165,10 @@ public List expectedBodyStmtsTryCatchFinally() { "$stack7 = ", "virtualinvoke $stack7.(l1)", "throw l3", + "label6:", "return", + "catch java.lang.Exception from label1 to label2 with label3", "catch java.lang.Throwable from label1 to label2 with label5", "catch java.lang.Throwable from label3 to label4 with label5") diff --git a/sootup.java.core/src/main/java/sootup/java/core/interceptors/ConditionalBranchFolder.java b/sootup.java.core/src/main/java/sootup/java/core/interceptors/ConditionalBranchFolder.java index 9a7d4f03ed..921d5576e4 100644 --- a/sootup.java.core/src/main/java/sootup/java/core/interceptors/ConditionalBranchFolder.java +++ b/sootup.java.core/src/main/java/sootup/java/core/interceptors/ConditionalBranchFolder.java @@ -25,10 +25,9 @@ import java.util.*; import javax.annotation.Nonnull; import sootup.core.graph.MutableStmtGraph; -import sootup.core.graph.StmtGraph; +import sootup.core.jimple.Jimple; import sootup.core.jimple.common.constant.*; -import sootup.core.jimple.common.stmt.BranchingStmt; -import sootup.core.jimple.common.stmt.FallsThroughStmt; +import sootup.core.jimple.common.stmt.JGotoStmt; import sootup.core.jimple.common.stmt.JIfStmt; import sootup.core.jimple.common.stmt.Stmt; import sootup.core.model.Body; @@ -85,7 +84,11 @@ else if (evaluatedCondition instanceof DoubleConstant) { continue; } - final List ifSuccessors = stmtGraph.successors(ifStmt); + List ifSuccessors = stmtGraph.successors(ifStmt); + // The successors of IfStmt have true branch at index 0 & false branch at index 1. + // However, in other parts of code, TRUE_BRANCH_IDX is defined as 1 & FALSE_BRANCH_IDX as 0. + // To maintain consistency, we need to reverse the order of the successors. + ifSuccessors = Lists.reverse(ifSuccessors); final Stmt tautologicSuccessor; final Stmt neverReachedSucessor; @@ -102,102 +105,14 @@ else if (evaluatedCondition instanceof DoubleConstant) { neverReachedSucessor = ifSuccessors.get(JIfStmt.FALSE_BRANCH_IDX); } - // link previous stmt with always-reached successor of the if-Stmt - for (Stmt predecessor : stmtGraph.predecessors(ifStmt)) { - List successorIdxList = stmtGraph.removeEdge(predecessor, ifStmt); + // remove edge from ifStmt to neverReachedSucessor + stmtGraph.unLinkNodes(ifStmt, neverReachedSucessor); + // replace ifStmt block by gotoStmt + JGotoStmt gotoStmt = Jimple.newGotoStmt(ifStmt.getPositionInfo()); + stmtGraph.replaceStmt(ifStmt, gotoStmt); - if (predecessor instanceof FallsThroughStmt) { - FallsThroughStmt fallsThroughPred = (FallsThroughStmt) predecessor; - for (Integer successorIdx : successorIdxList) { - stmtGraph.putEdge(fallsThroughPred, tautologicSuccessor); - } - } else { - // should not be anything else than BranchingStmt.. just Stmt can have no successor - BranchingStmt branchingPred = (BranchingStmt) predecessor; - for (Integer successorIdx : successorIdxList) { - stmtGraph.putEdge(branchingPred, successorIdx, tautologicSuccessor); - } - } - } - - stmtGraph.removeNode(ifStmt, false); - - pruneExclusivelyReachableStmts(builder, neverReachedSucessor); - } - } - - private void pruneExclusivelyReachableStmts( - @Nonnull Body.BodyBuilder builder, @Nonnull Stmt fallsThroughStmt) { - - MutableStmtGraph stmtGraph = builder.getStmtGraph(); - Set reachedBranchingStmts = new HashSet<>(); - Deque q = new ArrayDeque<>(); - - q.addFirst(fallsThroughStmt); - // stmts we want to remove - // remove all now unreachable stmts from "true"-block - while (!q.isEmpty()) { - Stmt itStmt = q.pollFirst(); - if (itStmt.branches()) { - // reachable branching stmts that may or may not branch to another reachable stmt is all we - // are actually interested in - reachedBranchingStmts.add(itStmt); - } - if (stmtGraph.containsNode(itStmt)) { - final List predecessors = stmtGraph.predecessors(itStmt); - if (predecessors.size() <= 1) { - q.addAll(stmtGraph.successors(itStmt)); - } - } - } - // now iterate again and remove if possible: ie predecessor.size() < 1 - q.addFirst(fallsThroughStmt); - while (!q.isEmpty()) { - Stmt itStmt = q.pollFirst(); - if (stmtGraph.containsNode(itStmt)) { - // hint: predecessor could also be already removed - if (isExclusivelyReachable(stmtGraph, itStmt, reachedBranchingStmts)) { - q.addAll(stmtGraph.successors(itStmt)); - stmtGraph.removeNode(itStmt, false); - builder.removeDefLocalsOf(itStmt); - } - } - } - } - - /** reachedStmts contains all reached Stmts from entrypoint which ALSO do branch! */ - private boolean isExclusivelyReachable( - @Nonnull StmtGraph graph, @Nonnull Stmt stmt, @Nonnull Set reachedStmts) { - final List predecessors = graph.predecessors(stmt); - final int predecessorSize = predecessors.size(); - int amount = predecessorSize; - if (predecessorSize <= 1) { - // we already reached this stmt somehow via reachable stmts so at least one predecessor was - // reachable which makes it exclusively reachable if there are no other ingoing flows - // hint: <= because a predecessor could already be removed - return true; - } - for (Stmt predecessor : predecessors) { - if (predecessor.fallsThrough()) { - if (predecessor instanceof JIfStmt) { - final List predsSuccessors = graph.successors(predecessor); - if (predsSuccessors.size() > 0 && predsSuccessors.get(0) == stmt) { - // TODO: hint: possible problem occurs with partial removed targets as they change the - // idx positions.. - amount--; - continue; - } - } else { - // "usual" fallsthrough - amount--; - continue; - } - } - // was a branching predecessor reachable? - if (reachedStmts.contains(predecessor)) { - amount--; - } + // Call Unreachable Code Eliminator for pruning unreachable blocks + new UnreachableCodeEliminator().interceptBody(builder, view); } - return amount == 0; } } diff --git a/sootup.java.core/src/main/java/sootup/java/core/interceptors/UnreachableCodeEliminator.java b/sootup.java.core/src/main/java/sootup/java/core/interceptors/UnreachableCodeEliminator.java index 29ec9c6a68..161b78fdaa 100644 --- a/sootup.java.core/src/main/java/sootup/java/core/interceptors/UnreachableCodeEliminator.java +++ b/sootup.java.core/src/main/java/sootup/java/core/interceptors/UnreachableCodeEliminator.java @@ -21,9 +21,11 @@ * #L% */ import java.util.*; +import java.util.stream.Collectors; import javax.annotation.Nonnull; +import sootup.core.graph.BasicBlock; +import sootup.core.graph.MutableBasicBlock; import sootup.core.graph.MutableStmtGraph; -import sootup.core.jimple.common.stmt.Stmt; import sootup.core.model.Body; import sootup.core.transform.BodyInterceptor; import sootup.core.views.View; @@ -31,12 +33,10 @@ /** * A BodyInterceptor that removes all unreachable stmts from the given Body. * - * @author Zun Wang + * @author Zun Wang, Sahil Agichani */ public class UnreachableCodeEliminator implements BodyInterceptor { - // TODO: performance - quite expensive; maybe work on Block level to reduce hash calculations etc? - @Override public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) { @@ -47,32 +47,40 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) return; } - Deque queue = new ArrayDeque<>(); - queue.add(graph.getStartingStmt()); + Collection> allBlocks = graph.getBlocks(); + MutableBasicBlock startingStmtBlock = (MutableBasicBlock) graph.getStartingStmtBlock(); + Set reachableNodes = new HashSet<>(); + Deque stack = new ArrayDeque<>(); + stack.push(startingStmtBlock); - // calculate all reachable stmts - Set reachableStmts = new HashSet<>(); - while (!queue.isEmpty()) { - Stmt stmt = queue.removeFirst(); - reachableStmts.add(stmt); - for (Stmt succ : graph.getAllSuccessors(stmt)) { - if (!reachableStmts.contains(succ)) { - queue.add(succ); - } + // Traverse the call graph using DFS + while (!stack.isEmpty()) { + MutableBasicBlock currentBlock = stack.pop(); + // If the method has already been visited, skip it + if (!reachableNodes.add(currentBlock)) { + continue; } - } + // Get all the successors (i.e., called methods) of the current method + List currentBlockExceptionalSuccessors = + new ArrayList<>(currentBlock.getExceptionalSuccessors().values()); + List currentBlockSuccessors = currentBlock.getSuccessors(); + List currentBlockAllSuccessors = new ArrayList<>(currentBlockSuccessors); + currentBlockAllSuccessors.addAll(currentBlockExceptionalSuccessors); - // remove unreachable stmts from StmtGraph - Queue removeQ = new ArrayDeque<>(); - for (Stmt stmt : graph.getNodes()) { - if (!reachableStmts.contains(stmt)) { - removeQ.add(stmt); + // Push the successors into the stack + for (MutableBasicBlock successor : currentBlockAllSuccessors) { + if (!reachableNodes.contains(successor)) { + stack.push(successor); + } } } - for (Stmt stmt : removeQ) { - graph.removeNode(stmt, false); - builder.removeDefLocalsOf(stmt); + List> unreachableBlocks = + allBlocks.stream() + .filter(basicBlock -> !reachableNodes.contains(basicBlock)) + .collect(Collectors.toList()); + for (BasicBlock unreachableBlock : unreachableBlocks) { + graph.removeBlock(unreachableBlock); } } }