From cbdc2496fc4b11655005472ad21505113b9071e2 Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 1 Mar 2019 23:35:09 +0300 Subject: [PATCH] fix: check block before insert additional move instruction for type inference --- .../visitors/blocksmaker/BlockSplitter.java | 2 +- .../dex/visitors/regions/CheckRegions.java | 7 +- .../typeinference/TypeInferenceVisitor.java | 15 +++- .../visitors/typeinference/TypeUpdate.java | 3 +- .../main/java/jadx/core/utils/DebugUtils.java | 12 +++ .../conditions/TestComplexIf2.java | 38 +++++++++ .../smali/conditions/TestComplexIf2.smali | 85 +++++++++++++++++++ 7 files changed, 155 insertions(+), 7 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/conditions/TestComplexIf2.java create mode 100644 jadx-core/src/test/smali/conditions/TestComplexIf2.smali diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockSplitter.java b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockSplitter.java index e2f1fd08e..a08e090cf 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockSplitter.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockSplitter.java @@ -29,7 +29,7 @@ import jadx.core.utils.exceptions.JadxRuntimeException; public class BlockSplitter extends AbstractVisitor { // leave these instructions alone in block node - private static final Set SEPARATE_INSNS = EnumSet.of( + public static final Set SEPARATE_INSNS = EnumSet.of( InsnType.RETURN, InsnType.IF, InsnType.SWITCH, diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/CheckRegions.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/CheckRegions.java index b458a5b67..5ed55f609 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/CheckRegions.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/CheckRegions.java @@ -62,16 +62,16 @@ public class CheckRegions extends AbstractVisitor { && !block.contains(AFlag.DONT_GENERATE) && !block.contains(AFlag.REMOVE)) { String blockCode = getBlockInsnStr(mth, block); - mth.addWarn("Missing block: " + block + ", code skipped:" + CodeWriter.NL + blockCode); + mth.addWarn("Code restructure failed: missing block: " + block + ", code lost:" + blockCode); } } } - // check loop conditions DepthRegionTraversal.traverse(mth, new AbstractRegionVisitor() { @Override public boolean enterRegion(MethodNode mth, IRegion region) { if (region instanceof LoopRegion) { + // check loop conditions BlockNode loopHeader = ((LoopRegion) region).getHeader(); if (loopHeader != null && loopHeader.getInstructions().size() != 1) { mth.addWarn("Incorrect condition in loop: " + loopHeader); @@ -82,8 +82,9 @@ public class CheckRegions extends AbstractVisitor { }); } - private static String getBlockInsnStr(MethodNode mth, BlockNode block) { + private static String getBlockInsnStr(MethodNode mth, IBlock block) { CodeWriter code = new CodeWriter(); + code.newLine(); code.setIndent(3); MethodGen mg = MethodGen.getFallbackMethodGen(mth); InsnGen ig = new InsnGen(mg, true); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java index c80ab33ef..925ab616b 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java @@ -35,7 +35,9 @@ import jadx.core.dex.visitors.AbstractVisitor; import jadx.core.dex.visitors.ConstInlineVisitor; import jadx.core.dex.visitors.InitCodeVariables; import jadx.core.dex.visitors.JadxVisitor; +import jadx.core.dex.visitors.blocksmaker.BlockSplitter; import jadx.core.dex.visitors.ssa.SSATransform; +import jadx.core.utils.BlockUtils; import jadx.core.utils.Utils; @JadxVisitor( @@ -139,7 +141,7 @@ public final class TypeInferenceVisitor extends AbstractVisitor { TypeUpdateResult result = typeUpdate.apply(ssaVar, initType); if (result == TypeUpdateResult.REJECT) { if (Consts.DEBUG) { - LOG.info("Initial immutable type set rejected: {} -> {}", ssaVar, initType); + LOG.warn("Initial immutable type set rejected: {} -> {}", ssaVar, initType); } return false; } @@ -317,6 +319,15 @@ public final class TypeInferenceVisitor extends AbstractVisitor { for (Map.Entry entry : phiInsn.getBlockBinds().entrySet()) { RegisterArg reg = entry.getKey(); if (reg.getSVar() == var) { + BlockNode blockNode = entry.getValue(); + InsnNode lastInsn = BlockUtils.getLastInsn(blockNode); + if (lastInsn != null && BlockSplitter.SEPARATE_INSNS.contains(lastInsn.getType())) { + if (Consts.DEBUG) { + LOG.warn("Can't insert move for PHI in block with separate insn: {}", lastInsn); + } + return false; + } + int regNum = reg.getRegNum(); RegisterArg resultArg = reg.duplicate(regNum, null); SSAVar newSsaVar = mth.makeNewSVar(regNum, resultArg); @@ -326,7 +337,7 @@ public final class TypeInferenceVisitor extends AbstractVisitor { moveInsn.setResult(resultArg); moveInsn.addArg(arg); moveInsn.add(AFlag.SYNTHETIC); - entry.getValue().getInstructions().add(moveInsn); + blockNode.getInstructions().add(moveInsn); phiInsn.replaceArg(reg, reg.duplicate(regNum, newSsaVar)); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java index e445ef8af..f9738b40f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java @@ -12,6 +12,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import jadx.core.Consts; +import jadx.core.dex.attributes.AFlag; import jadx.core.dex.instructions.InsnType; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; @@ -278,7 +279,7 @@ public final class TypeUpdate { return REJECT; } } else { - allowReject = false; + allowReject = arg.isThis() || arg.contains(AFlag.IMMUTABLE_TYPE); } TypeUpdateResult result = updateTypeChecked(updateInfo, changeArg, candidateType); diff --git a/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java b/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java index 555ec29b2..fa7423427 100644 --- a/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java @@ -27,10 +27,13 @@ import jadx.core.dex.nodes.IContainer; import jadx.core.dex.nodes.IRegion; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; +import jadx.core.dex.visitors.AbstractVisitor; import jadx.core.dex.visitors.DotGraphVisitor; +import jadx.core.dex.visitors.IDexTreeVisitor; import jadx.core.dex.visitors.regions.DepthRegionTraversal; import jadx.core.dex.visitors.regions.TracedRegionVisitor; import jadx.core.utils.exceptions.CodegenException; +import jadx.core.utils.exceptions.JadxException; import jadx.core.utils.exceptions.JadxRuntimeException; @Deprecated @@ -70,6 +73,15 @@ public class DebugUtils { LOG.debug(" Found block: {} in regions: {}", block, regions); } + public static IDexTreeVisitor printRegionsVisitor() { + return new AbstractVisitor() { + @Override + public void visit(MethodNode mth) throws JadxException { + printRegions(mth, true); + } + }; + } + public static void printRegions(MethodNode mth) { printRegions(mth, false); } diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestComplexIf2.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestComplexIf2.java new file mode 100644 index 000000000..21ddc3311 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestComplexIf2.java @@ -0,0 +1,38 @@ +package jadx.tests.integration.conditions; + +import org.junit.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.junit.Assert.assertThat; + +public class TestComplexIf2 extends SmaliTest { + +/* + public void test() { + if (this.isSaved) { + throw new RuntimeException("Error"); + } + if (LoaderUtils.isContextLoaderAvailable()) { + this.savedContextLoader = LoaderUtils.getContextClassLoader(); + ClassLoader loader = this; + if (this.project != null && "simple".equals(this.project)) { + loader = getClass().getClassLoader(); + } + LoaderUtils.setContextClassLoader(loader); + this.isSaved = true; + } + } +*/ + + @Test + public void test() { + disableCompilation(); + ClassNode cls = getClassNodeFromSmaliWithPkg("conditions", "TestComplexIf2"); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("if (this.project != null && \"simple\".equals(this.project)) {")); + } +} diff --git a/jadx-core/src/test/smali/conditions/TestComplexIf2.smali b/jadx-core/src/test/smali/conditions/TestComplexIf2.smali new file mode 100644 index 000000000..b8a9190ad --- /dev/null +++ b/jadx-core/src/test/smali/conditions/TestComplexIf2.smali @@ -0,0 +1,85 @@ +.class public final Lconditions/TestComplexIf2; +.super Ljava/lang/ClassLoader; + + +# instance fields +.field private isSaved:Z + +.field private project:Ljava/lang/String; + + +.method public test()V + .locals 4 + + .line 415 + iget-boolean v0, p0, Lconditions/TestComplexIf2;->isSaved:Z + + if-eqz v0, :cond_0 + + .line 416 + new-instance v0, Ljava/lang/RuntimeException; + + const-string v1, "Error" + + invoke-direct {v0, v1}, Ljava/lang/RuntimeException;->(Ljava/lang/String;)V + + throw v0 + + .line 418 + :cond_0 + invoke-static {}, Lorg/apache/tools/ant/util/LoaderUtils;->isContextLoaderAvailable()Z + + move-result v0 + + if-eqz v0, :cond_2 + + .line 419 + invoke-static {}, Lorg/apache/tools/ant/util/LoaderUtils;->getContextClassLoader()Ljava/lang/ClassLoader; + + move-result-object v0 + + iput-object v0, p0, Lconditions/TestComplexIf2;->savedContextLoader:Ljava/lang/ClassLoader; + + .line 420 + move-object v0, p0 + + .line 421 + .local v0, "loader":Ljava/lang/ClassLoader; + iget-object v1, p0, Lconditions/TestComplexIf2;->project:Ljava/lang/String; + + if-eqz v1, :cond_1 + + const-string v1, "simple" + + iget-object v2, p0, Lconditions/TestComplexIf2;->project:Ljava/lang/String; + + invoke-virtual {v1, v2}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v1 + + if-eqz v1, :cond_1 + + .line 423 + invoke-virtual {p0}, Ljava/lang/Object;->getClass()Ljava/lang/Class; + + move-result-object v1 + + invoke-virtual {v1}, Ljava/lang/Class;->getClassLoader()Ljava/lang/ClassLoader; + + move-result-object v0 + + .line 425 + :cond_1 + invoke-static {v0}, Lorg/apache/tools/ant/util/LoaderUtils;->setContextClassLoader(Ljava/lang/ClassLoader;)V + + .line 426 + const/4 v1, 0x1 + + iput-boolean v1, p0, Lconditions/TestComplexIf2;->isSaved:Z + + .line 428 + .end local v0 # "loader":Ljava/lang/ClassLoader; + :cond_2 + return-void +.end method +