From 10074858947137f676fb349927218a4bb0550be4 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Fri, 3 Nov 2023 14:41:36 +0100 Subject: [PATCH 01/24] cleanup srcType.. --- .../JavaClassPathAnalysisInputLocation.java | 27 ++++--------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/inputlocation/JavaClassPathAnalysisInputLocation.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/inputlocation/JavaClassPathAnalysisInputLocation.java index 5370cd8e3d6..f84ca90d9e7 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/inputlocation/JavaClassPathAnalysisInputLocation.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/inputlocation/JavaClassPathAnalysisInputLocation.java @@ -30,7 +30,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sootup.core.frontend.AbstractClassSource; @@ -58,7 +57,7 @@ public class JavaClassPathAnalysisInputLocation implements AnalysisInputLocation @Nonnull private final Collection> cpEntries; /** Variable to track if user has specified the SourceType. By default, it will be set to null. */ - private SourceType srcType = null; + private final SourceType srcType; /** * Creates a {@link JavaClassPathAnalysisInputLocation} which locates classes in the given class @@ -67,15 +66,7 @@ public class JavaClassPathAnalysisInputLocation implements AnalysisInputLocation * @param classPath The class path to search in */ public JavaClassPathAnalysisInputLocation(@Nonnull String classPath) { - if (classPath.length() <= 0) { - throw new IllegalStateException("Empty class path given"); - } - - cpEntries = explodeClassPath(classPath); - - if (cpEntries.isEmpty()) { - throw new IllegalStateException("Empty class path is given."); - } + this(classPath, SourceType.Application); } /** @@ -86,11 +77,11 @@ public JavaClassPathAnalysisInputLocation(@Nonnull String classPath) { * @param srcType the source type for the path can be Library, Application, Phantom. */ public JavaClassPathAnalysisInputLocation( - @Nonnull String classPath, @Nullable SourceType srcType) { + @Nonnull String classPath, @Nonnull SourceType srcType) { + this.srcType = srcType; if (classPath.length() <= 0) { throw new IllegalStateException("Empty class path given"); } - setSpecifiedAsBuiltInByUser(srcType); cpEntries = explodeClassPath(classPath); if (cpEntries.isEmpty()) { @@ -98,16 +89,8 @@ public JavaClassPathAnalysisInputLocation( } } - /** - * The method sets the value of the variable srcType. - * - * @param srcType the source type for the path can be Library, Application, Phantom. - */ - public void setSpecifiedAsBuiltInByUser(@Nullable SourceType srcType) { - this.srcType = srcType; - } - @Override + @Nonnull public SourceType getSourceType() { return srcType; } From 3d7a38c310545dce0e3ee87f6f4ef6e2d27dfc93 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Fri, 3 Nov 2023 14:48:33 +0100 Subject: [PATCH 02/24] add test input and enable interceptors --- .../bugfixes/Issue739_Aggregator.java | 9 +++++++ .../bytecode/interceptors/AggregatorTest.java | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 shared-test-resources/bugfixes/Issue739_Aggregator.java diff --git a/shared-test-resources/bugfixes/Issue739_Aggregator.java b/shared-test-resources/bugfixes/Issue739_Aggregator.java new file mode 100644 index 00000000000..4da5eb5cbb8 --- /dev/null +++ b/shared-test-resources/bugfixes/Issue739_Aggregator.java @@ -0,0 +1,9 @@ +public class Issue739_Aggregator { + public static void main(String[] args) { + int a = Integer.valueOf(args[0]); + int b = a; + int c = b; + System.out.println(a + b + c); + + } +} \ No newline at end of file diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java index 82b3a36b963..0114e4b554a 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java @@ -2,9 +2,11 @@ import static org.junit.Assert.assertEquals; +import java.nio.file.Paths; import java.util.Collections; import java.util.List; import java.util.Set; +import org.junit.Assert; import org.junit.Test; import sootup.core.graph.MutableStmtGraph; import sootup.core.inputlocation.AnalysisInputLocation; @@ -18,9 +20,13 @@ import sootup.core.jimple.common.stmt.Stmt; import sootup.core.model.Body; import sootup.core.model.SootMethod; +import sootup.core.model.SourceType; +import sootup.core.types.ClassType; import sootup.core.types.PrimitiveType; import sootup.core.util.ImmutableUtils; +import sootup.java.bytecode.inputlocation.BytecodeClassLoadingOptions; import sootup.java.bytecode.inputlocation.JavaClassPathAnalysisInputLocation; +import sootup.java.bytecode.inputlocation.PathBasedAnalysisInputLocation; import sootup.java.core.JavaIdentifierFactory; import sootup.java.core.JavaProject; import sootup.java.core.JavaSootClass; @@ -177,4 +183,24 @@ public void testResource_Misuse() { System.out.println(sootMethod.getBody()); } } + + @Test + public void testIssue739() { + + PathBasedAnalysisInputLocation inputLocation = + PathBasedAnalysisInputLocation.create( + Paths.get("../shared-test-resources/bugfixes/Issue739_Aggregator.class"), + SourceType.Application); + + JavaProject project = + JavaProject.builder(new JavaLanguage(8)).addInputLocation(inputLocation).build(); + + JavaView view = project.createView(); + view.configBodyInterceptors(a -> BytecodeClassLoadingOptions.Default); + + final ClassType classType = view.getIdentifierFactory().getClassType("Issue739_Aggregator"); + Assert.assertTrue(view.getClass(classType).isPresent()); + + view.getClasses().stream().findFirst().get().getMethods().forEach(SootMethod::getBody); + } } From d0c977d39ae509c6b8ee77d2ef581640885c3f91 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Fri, 3 Nov 2023 15:59:51 +0100 Subject: [PATCH 03/24] handle only operands that we can handle --- .../java/sootup/java/bytecode/interceptors/Aggregator.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java index 528e4b7d9cb..09cc91c3cee 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java @@ -183,14 +183,15 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi continue; } - // cannot aggregate e.g. a JIdentityStmt + // can only aggregate JAssignStmts if (!(relevantDef instanceof JAssignStmt)) { continue; } Value aggregatee = ((AbstractDefinitionStmt) relevantDef).getRightOp(); JAssignStmt newStmt = null; - if (assignStmt.getRightOp() instanceof AbstractBinopExpr) { + if (assignStmt.getRightOp() instanceof AbstractBinopExpr + && aggregatee instanceof Immediate) { AbstractBinopExpr rightOp = (AbstractBinopExpr) assignStmt.getRightOp(); if (rightOp.getOp1() == val) { AbstractBinopExpr newBinopExpr = rightOp.withOp1((Immediate) aggregatee); From 1a1b66fe97399a57a992d770f4bca64208d5f8bd Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Fri, 3 Nov 2023 16:01:32 +0100 Subject: [PATCH 04/24] check if a Local is used *somewhere* not just on the rigth side of a simple Local assignment --- .../DeadAssignmentEliminator.java | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java index 4cd09e51b5a..3077084eed8 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java @@ -22,8 +22,7 @@ */ import java.util.*; import javax.annotation.Nonnull; -import sootup.core.graph.MutableBasicBlock; -import sootup.core.graph.StmtGraph; +import sootup.core.graph.MutableStmtGraph; import sootup.core.jimple.Jimple; import sootup.core.jimple.basic.LValue; import sootup.core.jimple.basic.Local; @@ -68,7 +67,7 @@ public DeadAssignmentEliminator(boolean eliminateOnlyStackLocals) { @Override public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) { - StmtGraph stmtGraph = builder.getStmtGraph(); + MutableStmtGraph stmtGraph = builder.getStmtGraph(); List stmts = builder.getStmts(); Deque deque = new ArrayDeque<>(stmts.size()); @@ -177,7 +176,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi if (containsInvoke || !allEssential) { // Add all the statements which are used to compute values for the essential statements, // recursively - allDefs = Body.collectDefs(builder.getStmtGraph().getNodes()); + allDefs = Body.collectDefs(stmtGraph.getNodes()); if (!allEssential) { Set essentialStmts = new HashSet<>(stmts.size()); @@ -199,13 +198,13 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi // Remove the dead statements for (Stmt stmt : stmts) { if (!essentialStmts.contains(stmt)) { - builder.removeStmt(stmt); + stmtGraph.removeNode(stmt); } } } if (containsInvoke) { - allUses = Body.collectUses(builder.getStmtGraph().getNodes()); + allUses = Body.collectUses(stmtGraph.getNodes()); // Eliminate dead assignments from invokes such as x = f(), where x is no longer used List postProcess = new ArrayList<>(); for (Stmt stmt : stmts) { @@ -214,10 +213,14 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi if (assignStmt.containsInvokeExpr()) { // Just find one use of Value which is essential boolean deadAssignment = true; - if (assignStmt.getRightOp() instanceof Local) { - Local value = (Local) assignStmt.getRightOp(); + + List values = assignStmt.getUses(); + for (Value value : values) { + if (!(value instanceof LValue)) { + continue; + } for (Stmt use : allUses.get(value)) { - if (builder.getStmtGraph().containsNode(use)) { + if (stmtGraph.containsNode(use)) { deadAssignment = false; break; } @@ -234,7 +237,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi // Transform it into a simple invoke Stmt newInvoke = Jimple.newInvokeStmt(assignStmt.getInvokeExpr(), assignStmt.getPositionInfo()); - builder.replaceStmt(assignStmt, newInvoke); + stmtGraph.replaceNode(assignStmt, newInvoke); } } } From 5489b095f42dfd24223d1f865cdff203c3fac2f3 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Mon, 6 Nov 2023 11:38:00 +0100 Subject: [PATCH 05/24] LocalNameStandardizer: dont rename Local if Type is not handled; TypeAssigner: change augmentation in cases of BottomType to unknownType --- .../BytecodeBodyInterceptors.java | 6 +- .../DeadAssignmentEliminator.java | 106 +++++++++--------- .../interceptors/LocalNameStandardizer.java | 14 ++- .../bytecode/interceptors/TypeAssigner.java | 19 +--- .../typeresolving/CastCounter.java | 29 +++-- .../typeresolving/TypeChecker.java | 25 +++-- .../typeresolving/TypeResolver.java | 26 ++--- .../typeresolving/types/BottomType.java | 2 +- .../bytecode/interceptors/AggregatorTest.java | 43 ++++++- 9 files changed, 150 insertions(+), 120 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/BytecodeBodyInterceptors.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/BytecodeBodyInterceptors.java index 19150d7ea6d..42f64e46ad6 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/BytecodeBodyInterceptors.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/BytecodeBodyInterceptors.java @@ -40,10 +40,8 @@ public enum BytecodeBodyInterceptors { new UnusedLocalEliminator(), new ConditionalBranchFolder(), new EmptySwitchEliminator(), - new TypeAssigner() - // ms: is already called from typeassigner? new LocalNameStandardizer() - // new LocalNameStandardizer(), - ); + new TypeAssigner(), + new LocalNameStandardizer()); @Nonnull private final List bodyInterceptors; diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java index 3077084eed8..b6176063b0f 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java @@ -173,72 +173,74 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi allEssential &= isEssential; } - if (containsInvoke || !allEssential) { - // Add all the statements which are used to compute values for the essential statements, - // recursively - allDefs = Body.collectDefs(stmtGraph.getNodes()); - - if (!allEssential) { - Set essentialStmts = new HashSet<>(stmts.size()); - while (!deque.isEmpty()) { - Stmt stmt = deque.removeFirst(); - if (essentialStmts.add(stmt)) { - for (Value value : stmt.getUses()) { - if (value instanceof Local) { - Local local = (Local) value; - Collection defs = allDefs.get(local); - if (defs != null) { - deque.addAll(defs); - } + if (!containsInvoke && allEssential) { + return; + } + + // Add all the statements which are used to compute values for the essential statements, + // recursively + allDefs = Body.collectDefs(stmtGraph.getNodes()); + + if (!allEssential) { + Set essentialStmts = new HashSet<>(stmts.size()); + while (!deque.isEmpty()) { + Stmt stmt = deque.removeFirst(); + if (essentialStmts.add(stmt)) { + for (Value value : stmt.getUses()) { + if (value instanceof Local) { + Local local = (Local) value; + Collection defs = allDefs.get(local); + if (defs != null) { + deque.addAll(defs); } } } } + } - // Remove the dead statements - for (Stmt stmt : stmts) { - if (!essentialStmts.contains(stmt)) { - stmtGraph.removeNode(stmt); - } + // Remove the dead statements + for (Stmt stmt : stmts) { + if (!essentialStmts.contains(stmt)) { + stmtGraph.removeNode(stmt); } } + } - if (containsInvoke) { - allUses = Body.collectUses(stmtGraph.getNodes()); - // Eliminate dead assignments from invokes such as x = f(), where x is no longer used - List postProcess = new ArrayList<>(); - for (Stmt stmt : stmts) { - if (stmt instanceof JAssignStmt) { - JAssignStmt assignStmt = (JAssignStmt) stmt; - if (assignStmt.containsInvokeExpr()) { - // Just find one use of Value which is essential - boolean deadAssignment = true; - - List values = assignStmt.getUses(); - for (Value value : values) { - if (!(value instanceof LValue)) { - continue; - } - for (Stmt use : allUses.get(value)) { - if (stmtGraph.containsNode(use)) { - deadAssignment = false; - break; - } - } + if (containsInvoke) { + allUses = Body.collectUses(stmtGraph.getNodes()); + // Eliminate dead assignments from invokes such as x = f(), where x is no longer used + List postProcess = new ArrayList<>(); + for (Stmt stmt : stmts) { + if (stmt instanceof JAssignStmt) { + JAssignStmt assignStmt = (JAssignStmt) stmt; + if (assignStmt.containsInvokeExpr()) { + // Just find one use of Value which is essential + boolean deadAssignment = true; + + List values = assignStmt.getUses(); + for (Value value : values) { + if (!(value instanceof LValue)) { + continue; } - if (deadAssignment) { - postProcess.add(assignStmt); + for (Stmt use : allUses.get(value)) { + if (stmtGraph.containsNode(use)) { + deadAssignment = false; + break; + } } } + if (deadAssignment) { + postProcess.add(assignStmt); + } } } + } - for (JAssignStmt assignStmt : postProcess) { - // Transform it into a simple invoke - Stmt newInvoke = - Jimple.newInvokeStmt(assignStmt.getInvokeExpr(), assignStmt.getPositionInfo()); - stmtGraph.replaceNode(assignStmt, newInvoke); - } + for (JAssignStmt assignStmt : postProcess) { + // Transform it into a simple invoke + Stmt newInvoke = + Jimple.newInvokeStmt(assignStmt.getInvokeExpr(), assignStmt.getPositionInfo()); + stmtGraph.replaceNode(assignStmt, newInvoke); } } } diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/LocalNameStandardizer.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/LocalNameStandardizer.java index 8f7251ae06e..008269aa52c 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/LocalNameStandardizer.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/LocalNameStandardizer.java @@ -30,7 +30,9 @@ import sootup.core.jimple.common.stmt.Stmt; import sootup.core.model.Body; import sootup.core.transform.BodyInterceptor; +import sootup.core.types.Type; import sootup.core.views.View; +import sootup.java.bytecode.interceptors.typeresolving.types.BottomType; // https://github.com/Sable/soot/blob/master/src/main/java/soot/jimple/toolkits/scalar/LocalNameStandardizer.java @@ -64,10 +66,18 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi while (iterator.hasNext()) { Local local = iterator.next(); Local newLocal; + Type type = local.getType(); + + if (type instanceof BottomType) { + // TODO: log that likely the jimple is not formed correctly + // TODO: handle module signatures + type = view.getIdentifierFactory().getClassType("java.lang.Object"); + } + if (local.isFieldLocal()) { - newLocal = lgen.generateFieldLocal(local.getType()); + newLocal = lgen.generateFieldLocal(type); } else { - newLocal = lgen.generateLocal(local.getType()); + newLocal = lgen.generateLocal(type); } builder.replaceLocal(local, newLocal); } diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/TypeAssigner.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/TypeAssigner.java index 8d48125369f..2978687dc03 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/TypeAssigner.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/TypeAssigner.java @@ -35,25 +35,10 @@ */ public class TypeAssigner implements BodyInterceptor { - private final boolean standardizeNames; - - public TypeAssigner() { - standardizeNames = true; - } - - /** - * @param autoStandardizeNames controls whether the LocalNameStandardizer should execute after the - * type assignment - */ - public TypeAssigner(boolean autoStandardizeNames) { - this.standardizeNames = autoStandardizeNames; - } + public TypeAssigner() {} @Override public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) { - if (new TypeResolver((JavaView) view).resolve(builder) && standardizeNames) { - LocalNameStandardizer standardizer = new LocalNameStandardizer(); - standardizer.interceptBody(builder, view); - } + new TypeResolver((JavaView) view).resolve(builder); } } diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/CastCounter.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/CastCounter.java index 658c07e1408..d82369a2440 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/CastCounter.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/CastCounter.java @@ -54,22 +54,22 @@ public CastCounter( } public int getCastCount(@Nonnull Typing typing) { - this.castCount = 0; - this.countOnly = true; + castCount = 0; + countOnly = true; setTyping(typing); for (Stmt stmt : builder.getStmts()) { stmt.accept(this); } - return this.castCount; + return castCount; } public int getCastCount() { - return this.castCount; + return castCount; } public void insertCastStmts(@Nonnull Typing typing) { - this.castCount = 0; - this.countOnly = false; + castCount = 0; + countOnly = false; setTyping(typing); for (Stmt stmt : Lists.newArrayList(builder.getStmts())) { stmt.accept(this); @@ -91,7 +91,7 @@ public void visit(@Nonnull Value value, @Nonnull Type stdType, @Nonnull Stmt stm if (hierarchy.isAncestor(stdType, evaType)) { return; } - this.castCount++; + castCount++; return; } @@ -112,8 +112,7 @@ public void visit(@Nonnull Value value, @Nonnull Type stdType, @Nonnull Stmt stm if (evaType == null || hierarchy.isAncestor(stdType, evaType)) { return; } - this.castCount++; - // TODO: modifiers later must be added + castCount++; final MutableStmtGraph stmtGraph = builder.getStmtGraph(); Local old_local; @@ -143,18 +142,18 @@ public void visit(@Nonnull Value value, @Nonnull Type stdType, @Nonnull Stmt stm newStmt = ((AbstractDefinitionStmt) stmt).withNewDef(new_local); } if (graph.containsNode(stmt)) { - builder.replaceStmt(stmt, newStmt); - this.stmt2NewStmt.put(oriStmt, newStmt); + graph.replaceNode(stmt, newStmt); + stmt2NewStmt.put(oriStmt, newStmt); } } private void addUpdatedValue(Value oldValue, Value newValue, Stmt stmt) { Map map; - if (!this.changedValues.containsKey(stmt)) { + if (!changedValues.containsKey(stmt)) { map = new HashMap<>(); - this.changedValues.put(stmt, map); + changedValues.put(stmt, map); } else { - map = this.changedValues.get(stmt); + map = changedValues.get(stmt); } map.put(oldValue, newValue); if (stmt instanceof JAssignStmt && stmt.containsArrayRef()) { @@ -183,7 +182,7 @@ private void addUpdatedValue(Value oldValue, Value newValue, Stmt stmt) { } private Local generateTempLocal(@Nonnull Type type) { - String name = "#l" + newLocalsCount++; + String name = "$#l" + newLocalsCount++; return Jimple.newLocal(name, type); } } diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/TypeChecker.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/TypeChecker.java index 1e895254001..b52a40534c0 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/TypeChecker.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/TypeChecker.java @@ -27,7 +27,7 @@ import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sootup.core.graph.StmtGraph; +import sootup.core.graph.MutableStmtGraph; import sootup.core.jimple.basic.LValue; import sootup.core.jimple.basic.Local; import sootup.core.jimple.basic.Value; @@ -56,7 +56,7 @@ public abstract class TypeChecker extends AbstractStmtVisitor { private Typing typing; protected final Body.BodyBuilder builder; - protected final StmtGraph graph; + protected final MutableStmtGraph graph; private static final Logger logger = LoggerFactory.getLogger(TypeChecker.class); @@ -79,7 +79,7 @@ public void caseInvokeStmt(@Nonnull JInvokeStmt stmt) { @Override public void caseAssignStmt(@Nonnull JAssignStmt stmt) { - Value lhs = stmt.getLeftOp(); + LValue lhs = stmt.getLeftOp(); Value rhs = stmt.getRightOp(); Type type_lhs = null; if (lhs instanceof Local) { @@ -96,10 +96,10 @@ public void caseAssignStmt(@Nonnull JAssignStmt stmt) { Type type_rhs = typing.getType((Local) rhs); // if base type of lhs is an object-like-type, retrieve its base type from array // allocation site. - if (Type.isObjectLikeType(type_base) - || (Type.isObject(type_base) && type_rhs instanceof PrimitiveType)) { - Map> defs = - Body.collectDefs(builder.getStmtGraph().getNodes()); + if (type_base != null + && (Type.isObjectLikeType(type_base) + || (Type.isObject(type_base) && type_rhs instanceof PrimitiveType))) { + Map> defs = Body.collectDefs(graph.getNodes()); Collection defStmts = defs.get(base); boolean findDef = false; if (defStmts != null) { @@ -118,18 +118,23 @@ public void caseAssignStmt(@Nonnull JAssignStmt stmt) { } } } - if (!findDef) { + if (!findDef && type_rhs != null) { arrayType = Type.createArrayType(type_rhs, 1); } } } - if (arrayType == null) { + if (arrayType == null && type_base != null) { arrayType = Type.createArrayType(type_base, 1); } } + + if (arrayType == null) { + return; + } type_lhs = arrayType.getElementType(); visit(base, arrayType, stmt); visit(lhs, type_lhs, stmt); + } else if (lhs instanceof JFieldRef) { if (lhs instanceof JInstanceFieldRef) { visit( @@ -151,7 +156,7 @@ public void caseAssignStmt(@Nonnull JAssignStmt stmt) { arrayType = (ArrayType) type_base; } else { if (type_base instanceof NullType || Type.isObjectLikeType(type_base)) { - Map> defs = Body.collectDefs(builder.getStmtGraph().getNodes()); + Map> defs = Body.collectDefs(graph.getNodes()); Deque worklist = new ArrayDeque<>(); Set visited = new HashSet<>(); worklist.add(new StmtLocalPair(stmt, base)); diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/TypeResolver.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/TypeResolver.java index be3358008ae..e80246b13fd 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/TypeResolver.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/TypeResolver.java @@ -38,10 +38,10 @@ import sootup.core.jimple.common.stmt.Stmt; import sootup.core.model.Body; import sootup.core.types.ArrayType; -import sootup.core.types.ClassType; import sootup.core.types.PrimitiveType; import sootup.core.types.Type; import sootup.java.bytecode.interceptors.typeresolving.types.AugmentIntegerTypes; +import sootup.java.bytecode.interceptors.typeresolving.types.BottomType; import sootup.java.core.views.JavaView; /** @author Zun Wang Algorithm: see 'Efficient Local Type Inference' at OOPSLA 08 */ @@ -95,8 +95,10 @@ public boolean resolve(@Nonnull Body.BodyBuilder builder) { if (newType == null || oldType.equals(newType)) { continue; } - Local newLocal = local.withType(newType); - builder.replaceLocal(local, newLocal); + if (!(newType instanceof BottomType)) { + Local newLocal = local.withType(newType); + builder.replaceLocal(local, newLocal); + } } return true; } @@ -238,20 +240,18 @@ private Collection applyAssignmentConstraint( /** This method is used to remove the more general typings. */ private void minimize(@Nonnull List typings, @Nonnull BytecodeHierarchy hierarchy) { Set objectLikeTypes = new HashSet<>(); - IdentifierFactory factory = view.getIdentifierFactory(); - ClassType obj = factory.getClassType("java.lang.Object"); - ClassType ser = factory.getClassType("java.io.Serializable"); - ClassType clo = factory.getClassType("java.lang.Cloneable"); - objectLikeTypes.add(obj); - objectLikeTypes.add(ser); - objectLikeTypes.add(clo); + // FIXME: [ms] handle java modules as well! + IdentifierFactory identifierFactory = view.getIdentifierFactory(); + objectLikeTypes.add(identifierFactory.getClassType("java.lang.Object")); + objectLikeTypes.add(identifierFactory.getClassType("java.io.Serializable")); + objectLikeTypes.add(identifierFactory.getClassType("java.lang.Cloneable")); // collect all locals whose types are object, serializable, cloneable Set objectLikeLocals = new HashSet<>(); Map> local2Types = getLocal2Types(typings); - for (Local local : local2Types.keySet()) { - if (local2Types.get(local).equals(objectLikeTypes)) { - objectLikeLocals.add(local); + for (Map.Entry> local : local2Types.entrySet()) { + if (local.getValue().equals(objectLikeTypes)) { + objectLikeLocals.add(local.getKey()); } } // if one typing is more general als another typing, it should be removed. diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/types/BottomType.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/types/BottomType.java index aa71c942a9a..3c55181c9d9 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/types/BottomType.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/types/BottomType.java @@ -43,7 +43,7 @@ private BottomType() {} @Override public void accept(@Nonnull TypeVisitor v) { - // todo: add bottom type case + v.defaultCaseType(); } @Override diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java index 0114e4b554a..0763bd9add9 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java @@ -3,13 +3,13 @@ import static org.junit.Assert.assertEquals; import java.nio.file.Paths; -import java.util.Collections; -import java.util.List; -import java.util.Set; +import java.util.*; +import javax.annotation.Nonnull; import org.junit.Assert; import org.junit.Test; import sootup.core.graph.MutableStmtGraph; import sootup.core.inputlocation.AnalysisInputLocation; +import sootup.core.inputlocation.ClassLoadingOptions; import sootup.core.jimple.Jimple; import sootup.core.jimple.basic.Local; import sootup.core.jimple.basic.NoPositionInformation; @@ -21,15 +21,16 @@ import sootup.core.model.Body; import sootup.core.model.SootMethod; import sootup.core.model.SourceType; +import sootup.core.transform.BodyInterceptor; import sootup.core.types.ClassType; import sootup.core.types.PrimitiveType; import sootup.core.util.ImmutableUtils; -import sootup.java.bytecode.inputlocation.BytecodeClassLoadingOptions; import sootup.java.bytecode.inputlocation.JavaClassPathAnalysisInputLocation; import sootup.java.bytecode.inputlocation.PathBasedAnalysisInputLocation; import sootup.java.core.JavaIdentifierFactory; import sootup.java.core.JavaProject; import sootup.java.core.JavaSootClass; +import sootup.java.core.JavaSootMethod; import sootup.java.core.language.JavaJimple; import sootup.java.core.language.JavaLanguage; import sootup.java.core.types.JavaClassType; @@ -195,12 +196,42 @@ public void testIssue739() { JavaProject project = JavaProject.builder(new JavaLanguage(8)).addInputLocation(inputLocation).build(); + final ClassLoadingOptions classLoadingOptions = + new ClassLoadingOptions() { + + @Nonnull + @Override + public List getBodyInterceptors() { + return Arrays.asList( + new NopEliminator(), + new CastAndReturnInliner(), + new UnreachableCodeEliminator(), + new LocalSplitter(), + new Aggregator(), + new CopyPropagator(), + new DeadAssignmentEliminator(), + new UnusedLocalEliminator(), + new ConditionalBranchFolder(), + new EmptySwitchEliminator(), + new TypeAssigner(), + new LocalNameStandardizer()); + } + }; + JavaView view = project.createView(); - view.configBodyInterceptors(a -> BytecodeClassLoadingOptions.Default); + view.configBodyInterceptors(a -> classLoadingOptions); final ClassType classType = view.getIdentifierFactory().getClassType("Issue739_Aggregator"); Assert.assertTrue(view.getClass(classType).isPresent()); - view.getClasses().stream().findFirst().get().getMethods().forEach(SootMethod::getBody); + // FIXME: [ms] TypeAssigner returned BottomType in d0c977d3 when the input was no + // correct/complete jimple! i.e. l1 had no assignment + // FIXME: [ms] Aggregator removes one "addition of the value of $l1" its currently just l1+l1 + // instead of l1+l1+l1 that gets printed + for (JavaSootMethod javaSootMethod : + view.getClasses().stream().findFirst().get().getMethods()) { + final Body body = javaSootMethod.getBody(); + System.out.println(body); + } } } From 6a946853c4f8d5535df0c04628d4612d99028d9a Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Mon, 6 Nov 2023 13:48:54 +0100 Subject: [PATCH 06/24] fix: dont replace the rhs of a non-invoke completely by the aggregatee --- .../bytecode/interceptors/Aggregator.java | 37 +++++++------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java index 09cc91c3cee..013f19db8c2 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java @@ -28,18 +28,18 @@ import java.util.Map; import java.util.Set; import javax.annotation.Nonnull; -import sootup.core.graph.StmtGraph; +import sootup.core.graph.MutableStmtGraph; import sootup.core.jimple.basic.Immediate; import sootup.core.jimple.basic.LValue; import sootup.core.jimple.basic.Local; import sootup.core.jimple.basic.Value; -import sootup.core.jimple.common.expr.AbstractBinopExpr; import sootup.core.jimple.common.expr.AbstractInstanceInvokeExpr; import sootup.core.jimple.common.ref.JArrayRef; import sootup.core.jimple.common.ref.JFieldRef; import sootup.core.jimple.common.stmt.AbstractDefinitionStmt; import sootup.core.jimple.common.stmt.JAssignStmt; import sootup.core.jimple.common.stmt.Stmt; +import sootup.core.jimple.visitor.ReplaceUseStmtVisitor; import sootup.core.model.Body; import sootup.core.transform.BodyInterceptor; import sootup.core.views.View; @@ -71,7 +71,7 @@ public Aggregator(boolean dontAggregateFieldLocals) { */ @Override public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) { - StmtGraph graph = builder.getStmtGraph(); + MutableStmtGraph graph = builder.getStmtGraph(); List stmts = builder.getStmts(); Map> usesMap = Body.collectUses(stmts); @@ -92,7 +92,8 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi if (!(val instanceof Local)) { continue; } - if (usesMap.get(val).size() > 1) { + final Collection usesOfVal = usesMap.get(val); + if (usesOfVal.size() > 1) { // there are other uses, so it can't be aggregated continue; } @@ -189,30 +190,18 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi } Value aggregatee = ((AbstractDefinitionStmt) relevantDef).getRightOp(); - JAssignStmt newStmt = null; - if (assignStmt.getRightOp() instanceof AbstractBinopExpr - && aggregatee instanceof Immediate) { - AbstractBinopExpr rightOp = (AbstractBinopExpr) assignStmt.getRightOp(); - if (rightOp.getOp1() == val) { - AbstractBinopExpr newBinopExpr = rightOp.withOp1((Immediate) aggregatee); - newStmt = - new JAssignStmt(assignStmt.getLeftOp(), newBinopExpr, assignStmt.getPositionInfo()); - } else if (rightOp.getOp2() == val) { - AbstractBinopExpr newBinopExpr = rightOp.withOp2((Immediate) aggregatee); - newStmt = - new JAssignStmt(assignStmt.getLeftOp(), newBinopExpr, assignStmt.getPositionInfo()); - } - } else { - newStmt = assignStmt.withRValue(aggregatee); - } + Stmt newStmt = null; + if (aggregatee instanceof Immediate) { + final ReplaceUseStmtVisitor replaceVisitor = new ReplaceUseStmtVisitor(val, aggregatee); + replaceVisitor.caseAssignStmt(assignStmt); + newStmt = replaceVisitor.getResult(); - if (newStmt != null) { - builder.replaceStmt(stmt, newStmt); + graph.replaceNode(stmt, newStmt); if (graph.getStartingStmt() == relevantDef) { Stmt newStartingStmt = builder.getStmtGraph().successors(relevantDef).get(0); - builder.setStartingStmt(newStartingStmt); + graph.setStartingStmt(newStartingStmt); } - builder.removeStmt(relevantDef); + graph.removeNode(relevantDef); } } } From 7b5dc03782e6ee1daad80e7e84c7ef05fa3fbb73 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Mon, 6 Nov 2023 14:01:13 +0100 Subject: [PATCH 07/24] fix value replacement in aggregation via a hacky "ValueBox.canContainValue" --- .../java/bytecode/interceptors/Aggregator.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java index 013f19db8c2..7c3a7eb9808 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java @@ -29,7 +29,6 @@ import java.util.Set; import javax.annotation.Nonnull; import sootup.core.graph.MutableStmtGraph; -import sootup.core.jimple.basic.Immediate; import sootup.core.jimple.basic.LValue; import sootup.core.jimple.basic.Local; import sootup.core.jimple.basic.Value; @@ -190,12 +189,19 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi } Value aggregatee = ((AbstractDefinitionStmt) relevantDef).getRightOp(); - Stmt newStmt = null; - if (aggregatee instanceof Immediate) { - final ReplaceUseStmtVisitor replaceVisitor = new ReplaceUseStmtVisitor(val, aggregatee); + Stmt newStmt; + + final ReplaceUseStmtVisitor replaceVisitor = new ReplaceUseStmtVisitor(val, aggregatee); + // FIXME: this try-catch is an awful hack for "ValueBox.canContainValue" -> try to determine + // a replaceability earlier! + try { replaceVisitor.caseAssignStmt(assignStmt); newStmt = replaceVisitor.getResult(); + } catch (ClassCastException iae) { + newStmt = null; + } + if (newStmt != null) { graph.replaceNode(stmt, newStmt); if (graph.getStartingStmt() == relevantDef) { Stmt newStartingStmt = builder.getStmtGraph().successors(relevantDef).get(0); From 12dcf426f43d7fd46180456e3444f378cc0bb315 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Mon, 6 Nov 2023 14:04:02 +0100 Subject: [PATCH 08/24] cleanup --- .../bytecode/interceptors/AggregatorTest.java | 32 ++----------------- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java index 0763bd9add9..cd4ff2f0e49 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java @@ -4,12 +4,10 @@ import java.nio.file.Paths; import java.util.*; -import javax.annotation.Nonnull; import org.junit.Assert; import org.junit.Test; import sootup.core.graph.MutableStmtGraph; import sootup.core.inputlocation.AnalysisInputLocation; -import sootup.core.inputlocation.ClassLoadingOptions; import sootup.core.jimple.Jimple; import sootup.core.jimple.basic.Local; import sootup.core.jimple.basic.NoPositionInformation; @@ -21,10 +19,10 @@ import sootup.core.model.Body; import sootup.core.model.SootMethod; import sootup.core.model.SourceType; -import sootup.core.transform.BodyInterceptor; import sootup.core.types.ClassType; import sootup.core.types.PrimitiveType; import sootup.core.util.ImmutableUtils; +import sootup.java.bytecode.inputlocation.BytecodeClassLoadingOptions; import sootup.java.bytecode.inputlocation.JavaClassPathAnalysisInputLocation; import sootup.java.bytecode.inputlocation.PathBasedAnalysisInputLocation; import sootup.java.core.JavaIdentifierFactory; @@ -196,38 +194,12 @@ public void testIssue739() { JavaProject project = JavaProject.builder(new JavaLanguage(8)).addInputLocation(inputLocation).build(); - final ClassLoadingOptions classLoadingOptions = - new ClassLoadingOptions() { - - @Nonnull - @Override - public List getBodyInterceptors() { - return Arrays.asList( - new NopEliminator(), - new CastAndReturnInliner(), - new UnreachableCodeEliminator(), - new LocalSplitter(), - new Aggregator(), - new CopyPropagator(), - new DeadAssignmentEliminator(), - new UnusedLocalEliminator(), - new ConditionalBranchFolder(), - new EmptySwitchEliminator(), - new TypeAssigner(), - new LocalNameStandardizer()); - } - }; - JavaView view = project.createView(); - view.configBodyInterceptors(a -> classLoadingOptions); + view.configBodyInterceptors(a -> BytecodeClassLoadingOptions.Default); final ClassType classType = view.getIdentifierFactory().getClassType("Issue739_Aggregator"); Assert.assertTrue(view.getClass(classType).isPresent()); - // FIXME: [ms] TypeAssigner returned BottomType in d0c977d3 when the input was no - // correct/complete jimple! i.e. l1 had no assignment - // FIXME: [ms] Aggregator removes one "addition of the value of $l1" its currently just l1+l1 - // instead of l1+l1+l1 that gets printed for (JavaSootMethod javaSootMethod : view.getClasses().stream().findFirst().get().getMethods()) { final Body body = javaSootMethod.getBody(); From 894fbf9476ae1c86968512a596b2d56ac95eed0c Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Mon, 6 Nov 2023 14:15:17 +0100 Subject: [PATCH 09/24] adapting test to casts with non-field local names /add $ prefix --- .../typeresolving/CastCounterTest.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/typeresolving/CastCounterTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/typeresolving/CastCounterTest.java index 0ade1555c77..f6f4e1482aa 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/typeresolving/CastCounterTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/typeresolving/CastCounterTest.java @@ -106,24 +106,24 @@ public void testInvokeStmtWithNewCasts() { Assert.assertEquals( Stream.of( "CastCounterDemos $l0", - "Sub2 #l3", - "int #l2", - "integer1 #l0", - "long #l1", + "Sub2 $#l3", + "int $#l2", + "integer1 $#l0", + "long $#l1", "unknown $l1, $l2, $l3, $stack4, $stack5", "$l0 := @this: CastCounterDemos", "$stack4 = new Sub1", "specialinvoke $stack4.()>()", "$l1 = $stack4", - "#l0 = 1", - "#l1 = (long) #l0", - "$l2 = #l1", + "$#l0 = 1", + "$#l1 = (long) $#l0", + "$l2 = $#l1", "$stack5 = new Sub2", "specialinvoke $stack5.()>()", "$l3 = $stack5", - "#l2 = (int) $l2", - "#l3 = (Sub2) $l3", - "virtualinvoke $l1.(#l2, #l3)", + "$#l2 = (int) $l2", + "$#l3 = (Sub2) $l3", + "virtualinvoke $l1.($#l2, $#l3)", "return") .collect(Collectors.toList()), actualStmts); @@ -149,16 +149,16 @@ public void testAssignStmtWithNewCasts() { Assert.assertEquals( Stream.of( "CastCounterDemos $l0", - "Super1[] #l0, #l1", + "Super1[] $#l0, $#l1", "unknown $l1, $l2, $stack3", "$l0 := @this: CastCounterDemos", "$l1 = newarray (Super1)[10]", "$stack3 = new Sub1", "specialinvoke $stack3.()>()", - "#l0 = (Super1[]) $l1", - "#l0[0] = $stack3", - "#l1 = (Super1[]) $l1", - "$l2 = #l1[2]", + "$#l0 = (Super1[]) $l1", + "$#l0[0] = $stack3", + "$#l1 = (Super1[]) $l1", + "$l2 = $#l1[2]", "return") .collect(Collectors.toList()), actualStmts); From 3e5e94be017fc1e1aeb2dfe354b7803822c972ef Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Mon, 6 Nov 2023 14:47:18 +0100 Subject: [PATCH 10/24] fix test: adding name standardizer --- .../bytecode/interceptors/typeresolving/TypeAssignerTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/typeresolving/TypeAssignerTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/typeresolving/TypeAssignerTest.java index 606633a7811..3461d14c1f0 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/typeresolving/TypeAssignerTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/typeresolving/TypeAssignerTest.java @@ -10,6 +10,7 @@ import org.junit.experimental.categories.Category; import sootup.core.model.Body; import sootup.core.util.Utils; +import sootup.java.bytecode.interceptors.LocalNameStandardizer; import sootup.java.bytecode.interceptors.TypeAssigner; @Category(Java8Test.class) @@ -26,6 +27,8 @@ public void setup() { public void testInvokeStmt() { final Body.BodyBuilder builder = createMethodsBuilder("invokeStmt", "void"); new TypeAssigner().interceptBody(builder, view); + new LocalNameStandardizer().interceptBody(builder, view); + List actualStmts = Utils.filterJimple(builder.build().toString()); Assert.assertEquals( @@ -52,6 +55,7 @@ public void testInvokeStmt() { public void testAssignStmt() { final Body.BodyBuilder builder = createMethodsBuilder("assignStmt", "void"); new TypeAssigner().interceptBody(builder, view); + new LocalNameStandardizer().interceptBody(builder, view); List actualStmts = Utils.filterJimple(builder.build().toString()); From 316e53751d954c0524b388750fcaeee6b6ddb1ca Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Mon, 6 Nov 2023 17:04:02 +0100 Subject: [PATCH 11/24] SourceType.Application vs SourceType.Libarary works already with mainmethod finding ;) --- .../src/test/java/sootup/callgraph/CallGraphTestBase.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sootup.callgraph/src/test/java/sootup/callgraph/CallGraphTestBase.java b/sootup.callgraph/src/test/java/sootup/callgraph/CallGraphTestBase.java index cefcac8de44..f504b20b337 100644 --- a/sootup.callgraph/src/test/java/sootup/callgraph/CallGraphTestBase.java +++ b/sootup.callgraph/src/test/java/sootup/callgraph/CallGraphTestBase.java @@ -7,6 +7,7 @@ import org.junit.Test; import sootup.core.model.SootClass; import sootup.core.model.SootMethod; +import sootup.core.model.SourceType; import sootup.core.signatures.MethodSignature; import sootup.java.bytecode.inputlocation.JavaClassPathAnalysisInputLocation; import sootup.java.core.JavaIdentifierFactory; @@ -36,7 +37,7 @@ private JavaView createViewForClassPath(String classPath, boolean useSourceCodeF JavaProject.builder(new JavaLanguage(8)) .addInputLocation( new JavaClassPathAnalysisInputLocation( - System.getProperty("java.home") + "/lib/rt.jar")); + System.getProperty("java.home") + "/lib/rt.jar", SourceType.Library)); if (useSourceCodeFrontend) { javaProjectBuilder.addInputLocation(new JavaSourcePathAnalysisInputLocation(classPath)); } else { From b270b08721be0f854d9facd878ece810aa9dda51 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Tue, 7 Nov 2023 17:00:09 +0100 Subject: [PATCH 12/24] tame another class not in the view exception throwing; it seems typehierarchy.contains() view.getClass().isPresent() as signatures could be referenced from other classes which then end up in the underlying graph of the typehierarchy --- .../sootup/core/typehierarchy/ViewTypeHierarchy.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java b/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java index 1a5b91d48ac..0af4ec45d56 100644 --- a/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java +++ b/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java @@ -299,7 +299,12 @@ private Stream selfAndImplementedInterfaces(Vertex vertex) { @Nullable @Override public ClassType superClassOf(@Nonnull ClassType classType) { - return sootClassFor(classType).getSuperclass().orElse(null); + final Optional> classOpt = view.getClass(classType); + if (!classOpt.isPresent()) { + logger.info("cant get superclass of " + classType + " is not in the view."); + return null; + } + return classOpt.get().getSuperclass().orElse(null); } public boolean isInterface(@Nonnull ClassType type) { @@ -416,11 +421,6 @@ private static Vertex createAndAddInterfaceVertex(Graph graph, Cla return interfaceVertex; } - @Nonnull - private SootClass sootClassFor(@Nonnull ClassType classType) { - return view.getClassOrThrow(classType); - } - @Override public void addType(@Nonnull SootClass sootClass) { ScanResult scanResult = lazyScanResult.get(); From fadcf4310ec7ee65419a07112f4b53462be04a5d Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Tue, 7 Nov 2023 18:05:28 +0100 Subject: [PATCH 13/24] try if test is not executed because of naming convention --- .../sootup/java/bytecode/{Soot1577.java => Soot1577Test.java} | 2 +- .../sootup/java/bytecode/{Soot1580.java => Soot1580Test.java} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename sootup.java.bytecode/src/test/java/sootup/java/bytecode/{Soot1577.java => Soot1577Test.java} (97%) rename sootup.java.bytecode/src/test/java/sootup/java/bytecode/{Soot1580.java => Soot1580Test.java} (98%) diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1577.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1577Test.java similarity index 97% rename from sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1577.java rename to sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1577Test.java index 6e391d16907..daeeaa47576 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1577.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1577Test.java @@ -15,7 +15,7 @@ import sootup.java.core.views.JavaView; @Category(Java8Test.class) -public class Soot1577 { +public class Soot1577Test { final String directory = "../shared-test-resources/soot-1577/"; @Test diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580Test.java similarity index 98% rename from sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580.java rename to sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580Test.java index 0d5555d72ec..a88893170eb 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580Test.java @@ -18,7 +18,7 @@ import sootup.java.core.views.JavaView; @Category(Java8Test.class) -public class Soot1580 { +public class Soot1580Test { final Path jar = Paths.get("../shared-test-resources/soot-1580/jpush-android_v3.0.5.jar"); @Test From c4da4d4d22bb71b6195102ce4243e662ef39a1b8 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Tue, 7 Nov 2023 19:10:57 +0100 Subject: [PATCH 14/24] ignore - previously ignored testcase --- .../src/test/java/sootup/java/bytecode/Soot1580Test.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580Test.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580Test.java index a88893170eb..559632fcda7 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580Test.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/Soot1580Test.java @@ -4,6 +4,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; import sootup.core.inputlocation.AnalysisInputLocation; @@ -22,6 +23,7 @@ public class Soot1580Test { final Path jar = Paths.get("../shared-test-resources/soot-1580/jpush-android_v3.0.5.jar"); @Test + @Ignore("Localsplitter fails; bytecode itself is somehow strange") public void test() { AnalysisInputLocation inputLocation = PathBasedAnalysisInputLocation.create(jar, null); From 05afa2886bb4eb99c322c50c74120a83348aedd4 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Wed, 8 Nov 2023 11:38:50 +0100 Subject: [PATCH 15/24] fix Body.collectUses() signature to use value instead of LValue as e.g. an Expr is a use as well --- .../expr/AbstractInstanceInvokeExpr.java | 10 +++----- .../common/stmt/AbstractDefinitionStmt.java | 6 ++--- .../src/main/java/sootup/core/model/Body.java | 23 ++++++++++--------- .../bytecode/interceptors/Aggregator.java | 3 +-- .../DeadAssignmentEliminator.java | 6 ++--- 5 files changed, 22 insertions(+), 26 deletions(-) diff --git a/sootup.core/src/main/java/sootup/core/jimple/common/expr/AbstractInstanceInvokeExpr.java b/sootup.core/src/main/java/sootup/core/jimple/common/expr/AbstractInstanceInvokeExpr.java index 1636d05aecb..2f0bc0e8b4f 100644 --- a/sootup.core/src/main/java/sootup/core/jimple/common/expr/AbstractInstanceInvokeExpr.java +++ b/sootup.core/src/main/java/sootup/core/jimple/common/expr/AbstractInstanceInvokeExpr.java @@ -48,14 +48,10 @@ public Local getBase() { @Override @Nonnull public List getUses() { - List list = new ArrayList<>(); - List args = getArgs(); - if (args != null) { - list.addAll(args); - for (Value arg : args) { - list.addAll(arg.getUses()); - } + List list = new ArrayList<>(args); + for (Value arg : args) { + list.addAll(arg.getUses()); } list.addAll(base.getUses()); list.add(base); diff --git a/sootup.core/src/main/java/sootup/core/jimple/common/stmt/AbstractDefinitionStmt.java b/sootup.core/src/main/java/sootup/core/jimple/common/stmt/AbstractDefinitionStmt.java index e900acb6d51..4240bb1136e 100644 --- a/sootup.core/src/main/java/sootup/core/jimple/common/stmt/AbstractDefinitionStmt.java +++ b/sootup.core/src/main/java/sootup/core/jimple/common/stmt/AbstractDefinitionStmt.java @@ -61,11 +61,11 @@ public List getDefs() { public final List getUses() { final List defsuses = getLeftOp().getUses(); final Value rightOp = getRightOp(); - final List uses = rightOp.getUses(); - List list = new ArrayList<>(defsuses.size() + uses.size() + 1); + final List rightOpUses = rightOp.getUses(); + List list = new ArrayList<>(defsuses.size() + rightOpUses.size() + 1); list.addAll(defsuses); list.add(rightOp); - list.addAll(uses); + list.addAll(rightOpUses); return list; } diff --git a/sootup.core/src/main/java/sootup/core/model/Body.java b/sootup.core/src/main/java/sootup/core/model/Body.java index 706d8e7e8d8..ea9158caa03 100644 --- a/sootup.core/src/main/java/sootup/core/model/Body.java +++ b/sootup.core/src/main/java/sootup/core/model/Body.java @@ -28,9 +28,12 @@ import java.util.*; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import sootup.core.graph.*; +import sootup.core.graph.MutableBlockStmtGraph; +import sootup.core.graph.MutableStmtGraph; +import sootup.core.graph.StmtGraph; import sootup.core.jimple.basic.*; -import sootup.core.jimple.common.ref.*; +import sootup.core.jimple.common.ref.JParameterRef; +import sootup.core.jimple.common.ref.JThisRef; import sootup.core.jimple.common.stmt.*; import sootup.core.signatures.MethodSignature; import sootup.core.util.Copyable; @@ -597,19 +600,17 @@ public static Map> collectDefs(Collection stmts) * @param stmts The searched list of statements * @return A map of Locals and their using statements */ - public static Map> collectUses(Collection stmts) { - Map> allUses = new HashMap<>(); + public static Map> collectUses(Collection stmts) { + Map> allUses = new HashMap<>(); for (Stmt stmt : stmts) { Collection uses = stmt.getUses(); for (Value value : uses) { - if (value instanceof LValue) { - Collection localUses = allUses.get(value); - if (localUses == null) { - localUses = new ArrayList<>(); - } - localUses.add(stmt); - allUses.put((LValue) value, localUses); + Collection localUses = allUses.get(value); + if (localUses == null) { + localUses = new ArrayList<>(); } + localUses.add(stmt); + allUses.put((LValue) value, localUses); } } return allUses; diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java index 7c3a7eb9808..7e024ee9e77 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java @@ -29,7 +29,6 @@ import java.util.Set; import javax.annotation.Nonnull; import sootup.core.graph.MutableStmtGraph; -import sootup.core.jimple.basic.LValue; import sootup.core.jimple.basic.Local; import sootup.core.jimple.basic.Value; import sootup.core.jimple.common.expr.AbstractInstanceInvokeExpr; @@ -72,7 +71,7 @@ public Aggregator(boolean dontAggregateFieldLocals) { public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) { MutableStmtGraph graph = builder.getStmtGraph(); List stmts = builder.getStmts(); - Map> usesMap = Body.collectUses(stmts); + Map> usesMap = Body.collectUses(stmts); for (Stmt stmt : stmts) { if (!(stmt instanceof JAssignStmt)) { diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java index b6176063b0f..86340fda849 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java @@ -63,7 +63,7 @@ public DeadAssignmentEliminator(boolean eliminateOnlyStackLocals) { } Map> allDefs = new HashMap<>(); - Map> allUses = new HashMap<>(); + Map> allUses = new HashMap<>(); @Override public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) { @@ -222,8 +222,8 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi if (!(value instanceof LValue)) { continue; } - for (Stmt use : allUses.get(value)) { - if (stmtGraph.containsNode(use)) { + for (Stmt stmtOfUse : allUses.get(value)) { + if (stmtGraph.containsNode(stmtOfUse)) { deadAssignment = false; break; } From f557de29a24ff33c41abc25308ea647e7ce3ffec Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Thu, 9 Nov 2023 11:06:38 +0100 Subject: [PATCH 16/24] fix porting bug in DeadAssignmentEliminator: checks now if a def/use is used in *another* essential stmt --- .../src/main/java/sootup/core/model/Body.java | 2 +- .../DeadAssignmentEliminator.java | 61 ++++++++++--------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/sootup.core/src/main/java/sootup/core/model/Body.java b/sootup.core/src/main/java/sootup/core/model/Body.java index ea9158caa03..927dc4d6afb 100644 --- a/sootup.core/src/main/java/sootup/core/model/Body.java +++ b/sootup.core/src/main/java/sootup/core/model/Body.java @@ -610,7 +610,7 @@ public static Map> collectUses(Collection stmts) { localUses = new ArrayList<>(); } localUses.add(stmt); - allUses.put((LValue) value, localUses); + allUses.put(value, localUses); } } return allUses; diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java index 86340fda849..41791b66d26 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java @@ -62,9 +62,6 @@ public DeadAssignmentEliminator(boolean eliminateOnlyStackLocals) { this.eliminateOnlyStackLocals = eliminateOnlyStackLocals; } - Map> allDefs = new HashMap<>(); - Map> allUses = new HashMap<>(); - @Override public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) { MutableStmtGraph stmtGraph = builder.getStmtGraph(); @@ -72,7 +69,6 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi Deque deque = new ArrayDeque<>(stmts.size()); // Make a first pass through the statements, noting the statements we must absolutely keep - boolean isStatic = MethodModifier.isStatic(builder.getModifiers()); boolean allEssential = true; boolean containsInvoke = false; @@ -177,55 +173,60 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi return; } + if (allEssential) { + return; + } + // Add all the statements which are used to compute values for the essential statements, // recursively - allDefs = Body.collectDefs(stmtGraph.getNodes()); - - if (!allEssential) { - Set essentialStmts = new HashSet<>(stmts.size()); - while (!deque.isEmpty()) { - Stmt stmt = deque.removeFirst(); - if (essentialStmts.add(stmt)) { - for (Value value : stmt.getUses()) { - if (value instanceof Local) { - Local local = (Local) value; - Collection defs = allDefs.get(local); - if (defs != null) { - deque.addAll(defs); - } + Map> allDefs = Body.collectDefs(stmtGraph.getNodes()); + + Set essentialStmts = new HashSet<>(stmts.size()); + while (!deque.isEmpty()) { + Stmt stmt = deque.removeFirst(); + if (essentialStmts.add(stmt)) { + for (Value value : stmt.getUses()) { + if (value instanceof Local) { + Local local = (Local) value; + Collection defs = allDefs.get(local); + if (defs != null) { + deque.addAll(defs); } } } } + } - // Remove the dead statements - for (Stmt stmt : stmts) { - if (!essentialStmts.contains(stmt)) { - stmtGraph.removeNode(stmt); - } + // Remove the dead statements + for (Stmt stmt : stmts) { + if (!essentialStmts.contains(stmt)) { + stmtGraph.removeNode(stmt); } } if (containsInvoke) { - allUses = Body.collectUses(stmtGraph.getNodes()); + Map> allUses = Body.collectUses(stmtGraph.getNodes()); // Eliminate dead assignments from invokes such as x = f(), where x is no longer used List postProcess = new ArrayList<>(); for (Stmt stmt : stmts) { if (stmt instanceof JAssignStmt) { JAssignStmt assignStmt = (JAssignStmt) stmt; if (assignStmt.containsInvokeExpr()) { - // Just find one use of Value which is essential + // find at least one use of Value which is in an essential stmt boolean deadAssignment = true; - List values = assignStmt.getUses(); + List values = assignStmt.getUsesAndDefs(); for (Value value : values) { if (!(value instanceof LValue)) { continue; } - for (Stmt stmtOfUse : allUses.get(value)) { - if (stmtGraph.containsNode(stmtOfUse)) { - deadAssignment = false; - break; + final Collection stmtsWithValuesUse = allUses.get(value); + if (stmtsWithValuesUse != null) { + for (Stmt stmtOfUse : stmtsWithValuesUse) { + if (essentialStmts.contains(stmtOfUse)) { + deadAssignment = false; + break; + } } } } From 080bfd765454326d5c1f9c7a71d64a2ba832c489 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Thu, 9 Nov 2023 11:19:22 +0100 Subject: [PATCH 17/24] cleanup --- .../bytecode/interceptors/DeadAssignmentEliminator.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java index 41791b66d26..07cebb57267 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java @@ -169,10 +169,6 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi allEssential &= isEssential; } - if (!containsInvoke && allEssential) { - return; - } - if (allEssential) { return; } @@ -197,7 +193,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi } } - // Remove the dead statements + // Remove the dead statements from the stmtGraph for (Stmt stmt : stmts) { if (!essentialStmts.contains(stmt)) { stmtGraph.removeNode(stmt); @@ -237,6 +233,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi } } + // change JAssignStmt+InvokeExpr where the lhs is not used/essential to an JInvokeStmt for (JAssignStmt assignStmt : postProcess) { // Transform it into a simple invoke Stmt newInvoke = From d462a6642d93448b44e29e06b764da1408ffad40 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Thu, 9 Nov 2023 11:33:58 +0100 Subject: [PATCH 18/24] fix early execution end conditions --- .../DeadAssignmentEliminator.java | 71 ++++++++++--------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java index 07cebb57267..a0f24af8efd 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java @@ -20,6 +20,7 @@ * . * #L% */ + import java.util.*; import javax.annotation.Nonnull; import sootup.core.graph.MutableStmtGraph; @@ -169,7 +170,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi allEssential &= isEssential; } - if (allEssential) { + if (!containsInvoke && allEssential) { return; } @@ -200,46 +201,48 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi } } - if (containsInvoke) { - Map> allUses = Body.collectUses(stmtGraph.getNodes()); - // Eliminate dead assignments from invokes such as x = f(), where x is no longer used - List postProcess = new ArrayList<>(); - for (Stmt stmt : stmts) { - if (stmt instanceof JAssignStmt) { - JAssignStmt assignStmt = (JAssignStmt) stmt; - if (assignStmt.containsInvokeExpr()) { - // find at least one use of Value which is in an essential stmt - boolean deadAssignment = true; - - List values = assignStmt.getUsesAndDefs(); - for (Value value : values) { - if (!(value instanceof LValue)) { - continue; - } - final Collection stmtsWithValuesUse = allUses.get(value); - if (stmtsWithValuesUse != null) { - for (Stmt stmtOfUse : stmtsWithValuesUse) { - if (essentialStmts.contains(stmtOfUse)) { - deadAssignment = false; - break; - } + if (!containsInvoke) { + return; + } + + Map> allUses = Body.collectUses(stmtGraph.getNodes()); + // Eliminate dead assignments from invokes such as x = f(), where x is no longer used + List postProcess = new ArrayList<>(); + for (Stmt stmt : stmts) { + if (stmt instanceof JAssignStmt) { + JAssignStmt assignStmt = (JAssignStmt) stmt; + if (assignStmt.containsInvokeExpr()) { + // find at least one use of Value which is in an essential stmt + boolean deadAssignment = true; + + List values = assignStmt.getUsesAndDefs(); + for (Value value : values) { + if (!(value instanceof LValue)) { + continue; + } + final Collection stmtsWithValuesUse = allUses.get(value); + if (stmtsWithValuesUse != null) { + for (Stmt stmtOfUse : stmtsWithValuesUse) { + if (essentialStmts.contains(stmtOfUse)) { + deadAssignment = false; + break; } } } - if (deadAssignment) { - postProcess.add(assignStmt); - } + } + if (deadAssignment) { + postProcess.add(assignStmt); } } } + } - // change JAssignStmt+InvokeExpr where the lhs is not used/essential to an JInvokeStmt - for (JAssignStmt assignStmt : postProcess) { - // Transform it into a simple invoke - Stmt newInvoke = - Jimple.newInvokeStmt(assignStmt.getInvokeExpr(), assignStmt.getPositionInfo()); - stmtGraph.replaceNode(assignStmt, newInvoke); - } + // change JAssignStmt+InvokeExpr where the lhs is not used/essential to an JInvokeStmt + for (JAssignStmt assignStmt : postProcess) { + // Transform it into a simple invoke + Stmt newInvoke = + Jimple.newInvokeStmt(assignStmt.getInvokeExpr(), assignStmt.getPositionInfo()); + stmtGraph.replaceNode(assignStmt, newInvoke); } } } From 0a10b59522642e25d42fa9124f5851a4a7b3e98a Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Thu, 9 Nov 2023 11:44:32 +0100 Subject: [PATCH 19/24] work directly on essentialStmts while collecting uses - removes the need to filter for essential stmts again in the second step --- .../interceptors/DeadAssignmentEliminator.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java index a0f24af8efd..1ecd7eeb4ae 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java @@ -205,7 +205,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi return; } - Map> allUses = Body.collectUses(stmtGraph.getNodes()); + Map> essentialUses = Body.collectUses(essentialStmts); // Eliminate dead assignments from invokes such as x = f(), where x is no longer used List postProcess = new ArrayList<>(); for (Stmt stmt : stmts) { @@ -220,14 +220,9 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi if (!(value instanceof LValue)) { continue; } - final Collection stmtsWithValuesUse = allUses.get(value); + final Collection stmtsWithValuesUse = essentialUses.get(value); if (stmtsWithValuesUse != null) { - for (Stmt stmtOfUse : stmtsWithValuesUse) { - if (essentialStmts.contains(stmtOfUse)) { - deadAssignment = false; - break; - } - } + deadAssignment = false; } } if (deadAssignment) { From edbcc4b2f8386953bb527f00b4ad4630135ccc08 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Thu, 9 Nov 2023 11:47:40 +0100 Subject: [PATCH 20/24] break early --- .../java/bytecode/interceptors/DeadAssignmentEliminator.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java index 1ecd7eeb4ae..62cff5d0910 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/DeadAssignmentEliminator.java @@ -223,6 +223,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi final Collection stmtsWithValuesUse = essentialUses.get(value); if (stmtsWithValuesUse != null) { deadAssignment = false; + break; } } if (deadAssignment) { From 59fef4d30222c4cf688b9f59a2edcd790e5bf409 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Thu, 9 Nov 2023 11:54:17 +0100 Subject: [PATCH 21/24] cleanup --- .../sootup/java/bytecode/interceptors/TrapTightener.java | 2 +- .../bytecode/interceptors/UnreachableCodeEliminator.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/TrapTightener.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/TrapTightener.java index 0db1ae11ea9..80bd6567a08 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/TrapTightener.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/TrapTightener.java @@ -56,7 +56,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi // FIXME: [ms] ThrowAnalysis is missing and in result mightThrow (...) makes no sense. Issue // #486 if (true) { - return; + throw new UnsupportedOperationException("TrapTightener is not yet implemented."); } MutableStmtGraph graph = builder.getStmtGraph(); diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/UnreachableCodeEliminator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/UnreachableCodeEliminator.java index f9b74d5c658..8770af69cac 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/UnreachableCodeEliminator.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/UnreachableCodeEliminator.java @@ -22,7 +22,7 @@ */ import java.util.*; import javax.annotation.Nonnull; -import sootup.core.graph.StmtGraph; +import sootup.core.graph.MutableStmtGraph; import sootup.core.jimple.common.stmt.Stmt; import sootup.core.model.Body; import sootup.core.transform.BodyInterceptor; @@ -38,7 +38,7 @@ public class UnreachableCodeEliminator implements BodyInterceptor { @Override public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) { - StmtGraph graph = builder.getStmtGraph(); + MutableStmtGraph graph = builder.getStmtGraph(); Deque queue = new ArrayDeque<>(); queue.add(graph.getStartingStmt()); @@ -64,7 +64,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View vi } for (Stmt stmt : removeQ) { - builder.removeStmt(stmt); + graph.removeNode(stmt); } } } From 94feb007705521536961d747f5509b4597cc04c9 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Thu, 9 Nov 2023 13:16:40 +0100 Subject: [PATCH 22/24] correct visibility --- .../core/typehierarchy/ViewTypeHierarchy.java | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java b/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java index 0af4ec45d56..80a4658e184 100644 --- a/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java +++ b/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java @@ -36,7 +36,6 @@ import javax.annotation.Nullable; import org.jgrapht.Graph; import org.jgrapht.graph.SimpleDirectedGraph; -import sootup.core.frontend.ResolveException; import sootup.core.model.SootClass; import sootup.core.typehierarchy.ViewTypeHierarchy.ScanResult.Edge; import sootup.core.typehierarchy.ViewTypeHierarchy.ScanResult.EdgeType; @@ -67,10 +66,10 @@ public ViewTypeHierarchy(@Nonnull View> view) { public Set implementersOf(@Nonnull ClassType interfaceType) { Vertex vertex = lazyScanResult.get().typeToVertex.get(interfaceType); if (vertex == null) { - throw new ResolveException("Could not find " + interfaceType + " in hierarchy."); + throw new IllegalArgumentException("Could not find '" + interfaceType + "' in hierarchy."); } if (vertex.type != VertexType.Interface) { - throw new IllegalArgumentException(interfaceType + " is not an interface."); + throw new IllegalArgumentException("'" + interfaceType + "' is not an interface."); } return subtypesOf(interfaceType); } @@ -80,10 +79,10 @@ public Set implementersOf(@Nonnull ClassType interfaceType) { public Set subclassesOf(@Nonnull ClassType classType) { Vertex vertex = lazyScanResult.get().typeToVertex.get(classType); if (vertex == null) { - throw new ResolveException("Could not find " + classType + " in hierarchy."); + throw new IllegalArgumentException("Could not find '" + classType + "' in hierarchy."); } if (vertex.type != VertexType.Class) { - throw new IllegalArgumentException(classType + " is not a class."); + throw new IllegalArgumentException("'" + classType + "' is not a class."); } return subtypesOf(classType); } @@ -94,7 +93,7 @@ public Set subtypesOf(@Nonnull ClassType type) { ScanResult scanResult = lazyScanResult.get(); Vertex vertex = scanResult.typeToVertex.get(type); if (vertex == null) { - throw new ResolveException("Could not find " + type + " in hierarchy."); + throw new IllegalArgumentException("Could not find '" + type + "' in hierarchy."); } Set subclasses = new HashSet<>(); @@ -110,7 +109,7 @@ public Set directSubtypesOf(@Nonnull ClassType type) { ScanResult scanResult = lazyScanResult.get(); Vertex vertex = scanResult.typeToVertex.get(type); if (vertex == null) { - throw new ResolveException("Could not find " + type + " in hierarchy."); + throw new IllegalArgumentException("Could not find '" + type + "' in hierarchy."); } Set subclasses = new HashSet<>(); @@ -141,7 +140,7 @@ public Set directSubtypesOf(@Nonnull ClassType type) { } @Nonnull - public List superClassesOf(@Nonnull Vertex classVertex, boolean includingSelf) { + protected List superClassesOf(@Nonnull Vertex classVertex, boolean includingSelf) { ScanResult scanResult = lazyScanResult.get(); Graph graph = scanResult.graph; @@ -167,21 +166,21 @@ public List superClassesOf(@Nonnull Vertex classVertex, boolean includin return superClasses; } - public Stream directlyImplementedInterfacesOf(@Nonnull Vertex classVertex) { + protected Stream directlyImplementedInterfacesOf(@Nonnull Vertex classVertex) { Graph graph = lazyScanResult.get().graph; return graph.outgoingEdgesOf(classVertex).stream() .filter(edge -> edge.type == EdgeType.ClassDirectlyImplements) .map(graph::getEdgeTarget); } - public Stream directlyExtendedInterfacesOf(@Nonnull Vertex interfaceVertex) { + protected Stream directlyExtendedInterfacesOf(@Nonnull Vertex interfaceVertex) { Graph graph = lazyScanResult.get().graph; return graph.outgoingEdgesOf(interfaceVertex).stream() .filter(edge -> edge.type == EdgeType.InterfaceDirectlyExtends) .map(graph::getEdgeTarget); } - public Stream directSuperClassOf(@Nonnull Vertex classVertex) { + protected Stream directSuperClassOf(@Nonnull Vertex classVertex) { Graph graph = lazyScanResult.get().graph; return graph.outgoingEdgesOf(classVertex).stream() .filter(edge -> edge.type == EdgeType.ClassDirectlyExtends) @@ -301,7 +300,7 @@ private Stream selfAndImplementedInterfaces(Vertex vertex) { public ClassType superClassOf(@Nonnull ClassType classType) { final Optional> classOpt = view.getClass(classType); if (!classOpt.isPresent()) { - logger.info("cant get superclass of " + classType + " is not in the view."); + logger.info("can't get superclass of '" + classType + "' is not in the view."); return null; } return classOpt.get().getSuperclass().orElse(null); @@ -310,7 +309,7 @@ public ClassType superClassOf(@Nonnull ClassType classType) { public boolean isInterface(@Nonnull ClassType type) { Vertex vertex = lazyScanResult.get().typeToVertex.get(type); if (vertex == null) { - throw new RuntimeException("Could not find " + type + " in hierarchy."); + throw new RuntimeException("Could not find '" + type + "' in hierarchy."); } return vertex.type == VertexType.Interface; } @@ -318,7 +317,7 @@ public boolean isInterface(@Nonnull ClassType type) { public boolean isClass(@Nonnull ClassType type) { Vertex vertex = lazyScanResult.get().typeToVertex.get(type); if (vertex == null) { - throw new RuntimeException("Could not find " + type + " in hierarchy."); + throw new RuntimeException("Could not find '" + type + "' in hierarchy."); } return vertex.type == VertexType.Class; } @@ -428,7 +427,7 @@ public void addType(@Nonnull SootClass sootClass) { } /** Holds a vertex for each {@link ClassType} encountered during the scan. */ - static class ScanResult { + protected static class ScanResult { enum VertexType { Class, @@ -439,7 +438,7 @@ enum VertexType { * @see #javaClassType * @see #type */ - static class Vertex { + protected static class Vertex { @Nonnull final ClassType javaClassType; @Nonnull final VertexType type; @@ -449,7 +448,7 @@ static class Vertex { } } - enum EdgeType { + protected enum EdgeType { /** Edge to an interface vertex this interface extends directly, non-transitively. */ InterfaceDirectlyExtends, /** Edge to an interface extending this interface directly, non-transitively. */ @@ -459,7 +458,7 @@ enum EdgeType { } /** @see #type */ - static class Edge { + protected static class Edge { @Nonnull final EdgeType type; Edge(@Nonnull EdgeType type) { From f684a981862d442eaf36b900736b26f9e5678a5c Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Thu, 9 Nov 2023 13:18:20 +0100 Subject: [PATCH 23/24] dont print in testcase --- .../java/sootup/java/bytecode/interceptors/AggregatorTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java index cd4ff2f0e49..b1fd34054dc 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java @@ -203,7 +203,6 @@ public void testIssue739() { for (JavaSootMethod javaSootMethod : view.getClasses().stream().findFirst().get().getMethods()) { final Body body = javaSootMethod.getBody(); - System.out.println(body); } } } From cc8a35aa84ddd2175bb4977f69bade4d30919a45 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Thu, 9 Nov 2023 14:02:52 +0100 Subject: [PATCH 24/24] don't distinguish explicitly between incomplete/complete superclassesof --- .../callgraph/AbstractCallGraphAlgorithm.java | 6 +++--- .../typehierarchy/MethodDispatchResolver.java | 3 +-- .../core/typehierarchy/TypeHierarchy.java | 19 ++----------------- .../core/typehierarchy/ViewTypeHierarchy.java | 18 +++++++++--------- .../typeresolving/BytecodeHierarchy.java | 14 ++++++++++++-- .../IncompleteSuperclassTest.java | 3 +-- 6 files changed, 28 insertions(+), 35 deletions(-) diff --git a/sootup.callgraph/src/main/java/sootup/callgraph/AbstractCallGraphAlgorithm.java b/sootup.callgraph/src/main/java/sootup/callgraph/AbstractCallGraphAlgorithm.java index 4c895c238e2..cf8877e083f 100644 --- a/sootup.callgraph/src/main/java/sootup/callgraph/AbstractCallGraphAlgorithm.java +++ b/sootup.callgraph/src/main/java/sootup/callgraph/AbstractCallGraphAlgorithm.java @@ -268,7 +268,7 @@ protected Stream resolveAllStaticInitializerCallsFromSourceMeth classType -> Stream.concat( Stream.of(classType), - view.getTypeHierarchy().incompleteSuperClassesOf(classType).stream())) + view.getTypeHierarchy().superClassesOf(classType).stream())) .filter(Objects::nonNull) .map(classType -> view.getMethod(classType.getStaticInitializer())) .filter(Optional::isPresent) @@ -291,7 +291,7 @@ protected final T findMethodInHierarchy( if (optSc.isPresent()) { SootClass sc = optSc.get(); - List superClasses = view.getTypeHierarchy().incompleteSuperClassesOf(sc.getType()); + List superClasses = view.getTypeHierarchy().superClassesOf(sc.getType()); Set interfaces = view.getTypeHierarchy().implementedInterfacesOf(sc.getType()); superClasses.addAll(interfaces); @@ -364,7 +364,7 @@ public CallGraph addClass(@Nonnull CallGraph oldCallGraph, @Nonnull JavaClassTyp processWorkList(view, workList, processed, updated); // Step 2: Add edges from old methods to methods overridden in the new class - List superClasses = view.getTypeHierarchy().incompleteSuperClassesOf(classType); + List superClasses = view.getTypeHierarchy().superClassesOf(classType); Set implementedInterfaces = view.getTypeHierarchy().implementedInterfacesOf(classType); Stream superTypes = diff --git a/sootup.core/src/main/java/sootup/core/typehierarchy/MethodDispatchResolver.java b/sootup.core/src/main/java/sootup/core/typehierarchy/MethodDispatchResolver.java index ff6f94fb36d..3a642172219 100644 --- a/sootup.core/src/main/java/sootup/core/typehierarchy/MethodDispatchResolver.java +++ b/sootup.core/src/main/java/sootup/core/typehierarchy/MethodDispatchResolver.java @@ -181,8 +181,7 @@ public static boolean canDispatch( private static List> findSuperClassesInclusive( View> view, ClassType classType) { return Stream.concat( - Stream.of(classType), - view.getTypeHierarchy().incompleteSuperClassesOf(classType).stream()) + Stream.of(classType), view.getTypeHierarchy().superClassesOf(classType).stream()) .flatMap(t -> view.getClass(t).map(Stream::of).orElseGet(Stream::empty)) .collect(Collectors.toList()); } diff --git a/sootup.core/src/main/java/sootup/core/typehierarchy/TypeHierarchy.java b/sootup.core/src/main/java/sootup/core/typehierarchy/TypeHierarchy.java index 48e1d20e167..096193ceb28 100644 --- a/sootup.core/src/main/java/sootup/core/typehierarchy/TypeHierarchy.java +++ b/sootup.core/src/main/java/sootup/core/typehierarchy/TypeHierarchy.java @@ -154,7 +154,7 @@ default boolean isSubtype(@Nonnull Type supertype, @Nonnull Type potentialSubtyp return (supertypeName.equals("java.lang.Object") && !potentialSubtypeName.equals("java.lang.Object")) || supertype.equals(superClassOf((ClassType) potentialSubtype)) - || incompleteSuperClassesOf((ClassType) potentialSubtype).contains(supertype) + || superClassesOf((ClassType) potentialSubtype).contains(supertype) || implementedInterfacesOf((ClassType) potentialSubtype).contains(supertype); } else if (potentialSubtype instanceof ArrayType) { // Arrays are subtypes of java.lang.Object, java.io.Serializable and java.lang.Cloneable @@ -169,27 +169,12 @@ default boolean isSubtype(@Nonnull Type supertype, @Nonnull Type potentialSubtyp } } - /** - * Returns all superclasses of classType up to java.lang.Object, which - * will be the last entry in the list. i.e. its ordered from bottom level to top level. - */ - @Nonnull - default List superClassesOf(@Nonnull ClassType classType) { - List superClasses = new ArrayList<>(); - ClassType currentSuperClass = superClassOf(classType); - while (currentSuperClass != null) { - superClasses.add(currentSuperClass); - currentSuperClass = superClassOf(currentSuperClass); - } - return superClasses; - } - /** * Returns all superclasses of classType up to java.lang.Object, which * will be the last entry in the list, or till one of the superclasses is not contained in view. */ @Nonnull - default List incompleteSuperClassesOf(@Nonnull ClassType classType) { + default List superClassesOf(@Nonnull ClassType classType) { List superClasses = new ArrayList<>(); ClassType currentSuperClass = null; try { diff --git a/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java b/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java index 80a4658e184..20b2ddde0b4 100644 --- a/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java +++ b/sootup.core/src/main/java/sootup/core/typehierarchy/ViewTypeHierarchy.java @@ -190,7 +190,7 @@ protected Stream directSuperClassOf(@Nonnull Vertex classVertex) { public Set directlyImplementedInterfacesOf(@Nonnull ClassType classType) { Vertex vertex = lazyScanResult.get().typeToVertex.get(classType); if (vertex == null) { - throw new IllegalStateException("Could not find '" + classType + "' in hierarchy."); + throw new IllegalArgumentException("Could not find '" + classType + "' in hierarchy."); } if (vertex.type != VertexType.Class) { throw new IllegalArgumentException(classType + " is not a class."); @@ -204,7 +204,7 @@ public Set directlyImplementedInterfacesOf(@Nonnull ClassType classTy public Set directlyExtendedInterfacesOf(@Nonnull ClassType interfaceType) { Vertex vertex = lazyScanResult.get().typeToVertex.get(interfaceType); if (vertex == null) { - throw new IllegalStateException("Could not find " + interfaceType + " in hierarchy."); + throw new IllegalArgumentException("Could not find " + interfaceType + " in hierarchy."); } if (vertex.type != VertexType.Interface) { throw new IllegalArgumentException(interfaceType + " is not a class."); @@ -228,7 +228,7 @@ public boolean contains(ClassType type) { public ClassType directSuperClassOf(@Nonnull ClassType classType) { Vertex vertex = lazyScanResult.get().typeToVertex.get(classType); if (vertex == null) { - throw new IllegalStateException("Could not find " + classType + " in hierarchy."); + throw new IllegalArgumentException("Could not find " + classType + " in hierarchy."); } Graph graph = lazyScanResult.get().graph; List list = @@ -241,7 +241,7 @@ public ClassType directSuperClassOf(@Nonnull ClassType classType) { /* is java.lang.Object */ return null; } else if (list.size() > 1) { - throw new RuntimeException(classType + "cannot have multiple superclasses"); + throw new IllegalArgumentException(classType + "cannot have multiple superclasses"); } else { return list.get(0).javaClassType; } @@ -254,7 +254,8 @@ public Set implementedInterfacesOf(@Nonnull ClassType type) { Vertex vertex = scanResult.typeToVertex.get(type); if (vertex == null) { - throw new IllegalStateException("Could not find " + type + " in hierarchy for view " + view); + throw new IllegalArgumentException( + "Could not find " + type + " in hierarchy for view " + view); } switch (vertex.type) { @@ -300,8 +301,7 @@ private Stream selfAndImplementedInterfaces(Vertex vertex) { public ClassType superClassOf(@Nonnull ClassType classType) { final Optional> classOpt = view.getClass(classType); if (!classOpt.isPresent()) { - logger.info("can't get superclass of '" + classType + "' is not in the view."); - return null; + throw new IllegalArgumentException("Could not find '" + classType + "' in the view."); } return classOpt.get().getSuperclass().orElse(null); } @@ -309,7 +309,7 @@ public ClassType superClassOf(@Nonnull ClassType classType) { public boolean isInterface(@Nonnull ClassType type) { Vertex vertex = lazyScanResult.get().typeToVertex.get(type); if (vertex == null) { - throw new RuntimeException("Could not find '" + type + "' in hierarchy."); + throw new IllegalArgumentException("Could not find '" + type + "' in hierarchy."); } return vertex.type == VertexType.Interface; } @@ -317,7 +317,7 @@ public boolean isInterface(@Nonnull ClassType type) { public boolean isClass(@Nonnull ClassType type) { Vertex vertex = lazyScanResult.get().typeToVertex.get(type); if (vertex == null) { - throw new RuntimeException("Could not find '" + type + "' in hierarchy."); + throw new IllegalArgumentException("Could not find '" + type + "' in hierarchy."); } return vertex.type == VertexType.Class; } diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/BytecodeHierarchy.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/BytecodeHierarchy.java index 49f741b086d..4009dd5c530 100644 --- a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/BytecodeHierarchy.java +++ b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/typeresolving/BytecodeHierarchy.java @@ -228,12 +228,22 @@ private Set buildAncestryPaths(ClassType type) { } } } else { - Set superInterfaces = typeHierarchy.directlyImplementedInterfacesOf(node.type); + + Set superInterfaces; + ClassType superClass; + try { + superInterfaces = typeHierarchy.directlyImplementedInterfacesOf(node.type); + superClass = typeHierarchy.superClassOf(node.type); + } catch (IllegalArgumentException iae) { + // node.type does not exist in + continue; + } + for (ClassType superInterface : superInterfaces) { AncestryPath superNode = new AncestryPath(superInterface, node); pathNodes.add(superNode); } - ClassType superClass = typeHierarchy.superClassOf(node.type); + // only java.lang.Object can have no SuperClass i.e. is null if (superClass != null) { AncestryPath superNode = new AncestryPath(superClass, node); diff --git a/sootup.tests/src/test/java/sootup/tests/typehierarchy/viewtypehierarchytestcase/IncompleteSuperclassTest.java b/sootup.tests/src/test/java/sootup/tests/typehierarchy/viewtypehierarchytestcase/IncompleteSuperclassTest.java index c51a52b0a8f..a3895b6ce2e 100644 --- a/sootup.tests/src/test/java/sootup/tests/typehierarchy/viewtypehierarchytestcase/IncompleteSuperclassTest.java +++ b/sootup.tests/src/test/java/sootup/tests/typehierarchy/viewtypehierarchytestcase/IncompleteSuperclassTest.java @@ -20,8 +20,7 @@ public class IncompleteSuperclassTest extends JavaTypeHierarchyTestBase { public void method() { ViewTypeHierarchy typeHierarchy = (ViewTypeHierarchy) customTestWatcher.getView().getTypeHierarchy(); - List superclasses = - typeHierarchy.incompleteSuperClassesOf(getClassType("SubClassB")); + List superclasses = typeHierarchy.superClassesOf(getClassType("SubClassB")); ClassType object = getClassType("java.lang.Object"); ImmutableList expectedSuperClasses = immutableList(getClassType("SubClassA"), object);