diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java index 183255afa..508705013 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java @@ -72,6 +72,7 @@ public enum AFlag { INCONSISTENT_CODE, // warning about incorrect decompilation REQUEST_IF_REGION_OPTIMIZE, // run if region visitor again + RERUN_SSA_TRANSFORM, // Class processing flags RESTART_CODEGEN, // codegen must be executed again diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java index 1286e323f..263417282 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java @@ -83,6 +83,10 @@ public class RegisterArg extends InsnArg implements Named { this.sVar = sVar; } + public void resetSSAVar() { + this.sVar = null; + } + @Override public String getName() { if (isSuper()) { diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/mods/ConstructorInsn.java b/jadx-core/src/main/java/jadx/core/dex/instructions/mods/ConstructorInsn.java index 602bac75e..dcb29ab49 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/mods/ConstructorInsn.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/mods/ConstructorInsn.java @@ -35,17 +35,17 @@ public final class ConstructorInsn extends BaseInvokeNode { } private CallType getCallType(MethodNode mth, ClassInfo classType, InsnArg instanceArg) { - if (instanceArg.isThis()) { - if (classType.equals(mth.getParentClass().getClassInfo())) { - if (callMth.getShortId().equals(mth.getMethodInfo().getShortId())) { - // self constructor - return CallType.SELF; - } - return CallType.THIS; - } + if (!instanceArg.isThis()) { + return CallType.CONSTRUCTOR; + } + if (!classType.equals(mth.getParentClass().getClassInfo())) { return CallType.SUPER; } - return CallType.CONSTRUCTOR; + if (callMth.getShortId().equals(mth.getMethodInfo().getShortId())) { + // self constructor + return CallType.SELF; + } + return CallType.THIS; } public ConstructorInsn(MethodInfo callMth, CallType callType) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstructorVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstructorVisitor.java index 0c0fa6ca0..b20a1645f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstructorVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstructorVisitor.java @@ -5,7 +5,7 @@ import java.util.ArrayList; import org.jetbrains.annotations.Nullable; import jadx.core.codegen.TypeGen; -import jadx.core.dex.info.MethodInfo; +import jadx.core.dex.attributes.AFlag; import jadx.core.dex.instructions.InsnType; import jadx.core.dex.instructions.InvokeNode; import jadx.core.dex.instructions.args.InsnArg; @@ -33,8 +33,10 @@ public class ConstructorVisitor extends AbstractVisitor { if (mth.isNoCode()) { return; } - replaceInvoke(mth); + if (mth.contains(AFlag.RERUN_SSA_TRANSFORM)) { + SSATransform.rerun(mth); + } } private static void replaceInvoke(MethodNode mth) { @@ -53,55 +55,46 @@ public class ConstructorVisitor extends AbstractVisitor { } private static void processInvoke(MethodNode mth, BlockNode block, int indexInBlock, InsnRemover remover) { - ClassNode parentClass = mth.getParentClass(); - InsnNode insn = block.getInstructions().get(indexInBlock); - InvokeNode inv = (InvokeNode) insn; - MethodInfo callMth = inv.getCallMth(); - if (!callMth.isConstructor()) { + InvokeNode inv = (InvokeNode) block.getInstructions().get(indexInBlock); + if (!inv.getCallMth().isConstructor()) { + return; + } + ConstructorInsn co = new ConstructorInsn(mth, inv); + if (canRemoveConstructor(mth, co)) { + remover.addAndUnbind(inv); return; } RegisterArg instanceArg = ((RegisterArg) inv.getArg(0)); - InsnNode instArgAssignInsn = instanceArg.getAssignInsn(); - ConstructorInsn co = new ConstructorInsn(mth, inv); + InsnNode newInstInsn = null; if (co.isNewInstance()) { - co.setResult(instanceArg); - // convert from 'use' to 'assign' - instanceArg.getSVar().setAssign(instanceArg); + InsnNode assignInsn = instanceArg.getAssignInsn(); + if (assignInsn != null) { + if (assignInsn.getType() == InsnType.CONSTRUCTOR) { + // arg already used in another constructor instruction + mth.add(AFlag.RERUN_SSA_TRANSFORM); + } else { + newInstInsn = removeAssignChain(mth, assignInsn, remover, InsnType.NEW_INSTANCE); + if (newInstInsn != null) { + newInstInsn.add(AFlag.REMOVE); + remover.addWithoutUnbind(newInstInsn); + } + } + } + // convert instance arg from 'use' to 'assign' + co.setResult(instanceArg.duplicate()); } instanceArg.getSVar().removeUse(instanceArg); co.rebindArgs(); - boolean remove = false; - if (co.isSuper() && (co.getArgsCount() == 0 || parentClass.isEnum())) { - remove = true; - } else if (co.isThis() && co.getArgsCount() == 0) { - MethodNode defCo = parentClass.searchMethodByShortId(callMth.getShortId()); - if (defCo == null || defCo.isNoCode()) { - // default constructor not implemented - remove = true; - } - } - // remove super() call in instance initializer - if (parentClass.isAnonymous() && mth.isDefaultConstructor() && co.isSuper()) { - remove = true; - } - if (remove) { - remover.addAndUnbind(insn); - return; - } - if (co.isNewInstance()) { - InsnNode newInstInsn = removeAssignChain(mth, instArgAssignInsn, remover, InsnType.NEW_INSTANCE); - if (newInstInsn != null) { - remover.addWithoutUnbind(newInstInsn); - RegisterArg instArg = newInstInsn.getResult(); - RegisterArg resultArg = co.getResult(); - if (!resultArg.equals(instArg)) { - // replace all usages of 'instArg' with result of this constructor instruction - for (RegisterArg useArg : new ArrayList<>(instArg.getSVar().getUseList())) { - InsnNode parentInsn = useArg.getParentInsn(); - if (parentInsn != null) { - parentInsn.replaceArg(useArg, resultArg.duplicate()); - } + if (co.isNewInstance() && newInstInsn != null) { + RegisterArg instArg = newInstInsn.getResult(); + RegisterArg resultArg = co.getResult(); + if (!resultArg.equals(instArg)) { + // replace all usages of 'instArg' with result of this constructor instruction + for (RegisterArg useArg : new ArrayList<>(instArg.getSVar().getUseList())) { + InsnNode parentInsn = useArg.getParentInsn(); + if (parentInsn != null) { + parentInsn.replaceArg(useArg, resultArg.duplicate()); } } } @@ -109,9 +102,26 @@ public class ConstructorVisitor extends AbstractVisitor { ConstructorInsn replace = processConstructor(mth, co); if (replace != null) { remover.addAndUnbind(co); - co = replace; + BlockUtils.replaceInsn(mth, block, indexInBlock, replace); + } else { + BlockUtils.replaceInsn(mth, block, indexInBlock, co); } - BlockUtils.replaceInsn(mth, block, indexInBlock, co); + } + + private static boolean canRemoveConstructor(MethodNode mth, ConstructorInsn co) { + ClassNode parentClass = mth.getParentClass(); + if (co.isSuper() && (co.getArgsCount() == 0 || parentClass.isEnum())) { + return true; + } + if (co.isThis() && co.getArgsCount() == 0) { + MethodNode defCo = parentClass.searchMethodByShortId(co.getCallMth().getShortId()); + if (defCo == null || defCo.isNoCode()) { + // default constructor not implemented + return true; + } + } + // remove super() call in instance initializer + return parentClass.isAnonymous() && mth.isDefaultConstructor() && co.isSuper(); } /** diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java index c50ea9b81..6cc68894b 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java @@ -41,11 +41,16 @@ public class SSATransform extends AbstractVisitor { process(mth); } + public static void rerun(MethodNode mth) { + mth.remove(AFlag.RERUN_SSA_TRANSFORM); + resetSSAVars(mth); + process(mth); + } + private static void process(MethodNode mth) { if (!mth.getSVars().isEmpty()) { return; } - LiveVarAnalysis la = new LiveVarAnalysis(mth); la.runAnalysis(); int regsCount = mth.getRegsCount(); @@ -433,4 +438,15 @@ public class SSATransform extends AbstractVisitor { block.getInstructions().removeIf(insn -> insn.getType() == InsnType.PHI); } } + + private static void resetSSAVars(MethodNode mth) { + for (SSAVar ssaVar : mth.getSVars()) { + ssaVar.getAssign().resetSSAVar(); + ssaVar.getUseList().forEach(RegisterArg::resetSSAVar); + } + for (BlockNode block : mth.getBasicBlocks()) { + block.remove(AType.PHI_LIST); + } + mth.getSVars().clear(); + } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/others/TestConstructorBranched.java b/jadx-core/src/test/java/jadx/tests/integration/others/TestConstructorBranched.java new file mode 100644 index 000000000..f0d6bff62 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/others/TestConstructorBranched.java @@ -0,0 +1,33 @@ +package jadx.tests.integration.others; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +@SuppressWarnings("CommentedOutCode") +public class TestConstructorBranched extends SmaliTest { + // @formatter:off + /* + public Set test(Collection collection) { + Set set; + if (collection == null) { + set = new HashSet<>(); + } else { + set = new HashSet<>(collection); + } + set.add("end"); + return set; + } + */ + // @formatter:on + + @Test + public void test() { + assertThat(getClassNodeFromSmali()) + .code() + .containsOne("new HashSet()") + .containsOne("new HashSet(collection)"); + } +} diff --git a/jadx-core/src/test/smali/others/TestConstructorBranched.smali b/jadx-core/src/test/smali/others/TestConstructorBranched.smali new file mode 100644 index 000000000..4e68c967f --- /dev/null +++ b/jadx-core/src/test/smali/others/TestConstructorBranched.smali @@ -0,0 +1,33 @@ +.class public Lothers/TestConstructorBranched; +.super Ljava/lang/Object; + +.method public test(Ljava/util/Collection;)Ljava/util/Set; + .registers 4 + .annotation system Ldalvik/annotation/Signature; + value = { + "(", + "Ljava/util/Collection", + "<", + "Ljava/lang/String;", + ">;)", + "Ljava/util/Set", + "<", + "Ljava/lang/String;", + ">;" + } + .end annotation + + new-instance v0, Ljava/util/HashSet; + + if-nez p1, :cond_d + invoke-direct {v0}, Ljava/util/HashSet;->()V + goto :goto_7 + + :cond_d + invoke-direct {v0, p1}, Ljava/util/HashSet;->(Ljava/util/Collection;)V + + :goto_7 + const-string v1, "end" + invoke-interface {v0, v1}, Ljava/util/Set;->add(Ljava/lang/Object;)Z + return-object v0 +.end method