Skip to content

Commit

Permalink
fix: check block before insert additional move instruction for type i…
Browse files Browse the repository at this point in the history
…nference
  • Loading branch information
skylot committed Mar 1, 2019
1 parent 3de04cb commit cbdc249
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
public class BlockSplitter extends AbstractVisitor {

// leave these instructions alone in block node
private static final Set<InsnType> SEPARATE_INSNS = EnumSet.of(
public static final Set<InsnType> SEPARATE_INSNS = EnumSet.of(
InsnType.RETURN,
InsnType.IF,
InsnType.SWITCH,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,16 @@ public void processBlock(MethodNode mth, IBlock container) {
&& !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);
Expand All @@ -82,8 +82,9 @@ public boolean enterRegion(MethodNode mth, IRegion region) {
});
}

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
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(
Expand Down Expand Up @@ -139,7 +141,7 @@ private boolean applyImmutableType(SSAVar ssaVar, ArgType initType) {
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;
}
Expand Down Expand Up @@ -317,6 +319,15 @@ private boolean tryInsertAdditionalInsn(MethodNode mth, SSAVar var) {
for (Map.Entry<RegisterArg, BlockNode> 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);
Expand All @@ -326,7 +337,7 @@ private boolean tryInsertAdditionalInsn(MethodNode mth, SSAVar var) {
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));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
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;
Expand Down Expand Up @@ -278,7 +279,7 @@ private TypeUpdateResult moveListener(TypeUpdateInfo updateInfo, InsnNode insn,
return REJECT;
}
} else {
allowReject = false;
allowReject = arg.isThis() || arg.contains(AFlag.IMMUTABLE_TYPE);
}

TypeUpdateResult result = updateTypeChecked(updateInfo, changeArg, candidateType);
Expand Down
12 changes: 12 additions & 0 deletions jadx-core/src/main/java/jadx/core/utils/DebugUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,13 @@
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
Expand Down Expand Up @@ -70,6 +73,15 @@ public void processBlockTraced(MethodNode mth, IBlock container, IRegion current
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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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)) {"));
}
}
85 changes: 85 additions & 0 deletions jadx-core/src/test/smali/conditions/TestComplexIf2.smali
Original file line number Diff line number Diff line change
@@ -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;-><init>(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

0 comments on commit cbdc249

Please sign in to comment.