jessikitty/jadx
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

core: fix variable declaration in else-if chain (#273)

Browse Source
This commit is contained in:
Skylot
2018-05-26 19:57:02 +03:00
parent 7562ec9e1a
commit 4f02864e12
14 changed files with 144 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files
jadx-core/src/main/java/jadx/core/Jadx.java
+4 -4
View File
@@ -94,10 +94,6 @@ public class Jadx {
passes.add(new SimplifyVisitor());
passes.add(new CheckRegions());
if (args.isCfgOutput()) {
passes.add(DotGraphVisitor.dumpRegions());
}
passes.add(new MethodInlineVisitor());
passes.add(new ExtractFieldInit());
passes.add(new ClassModifier());
@@ -106,6 +102,10 @@ public class Jadx {
passes.add(new LoopRegionVisitor());
passes.add(new ProcessVariables());
if (args.isCfgOutput()) {
passes.add(DotGraphVisitor.dumpRegions());
}
passes.add(new DependencyCollector());
passes.add(new RenameVisitor());
jadx-core/src/main/java/jadx/core/ProcessClass.java
+1 -3
View File
@@ -56,8 +56,6 @@ public final class ProcessClass {
}
private static void processDependencies(ClassNode cls, List<IDexTreeVisitor> passes) {
for (ClassNode depCls : cls.getDependencies()) {
process(depCls, passes, null);
}
cls.getDependencies().forEach(depCls -> process(depCls, passes, null));
}
}
jadx-core/src/main/java/jadx/core/codegen/RegionGen.java
+10 -11
View File
@@ -134,17 +134,16 @@ public class RegionGen extends InsnGen {
* Connect if-else-if block
*/
private boolean connectElseIf(CodeWriter code, IContainer els) throws CodegenException {
if (!els.contains(AFlag.ELSE_IF_CHAIN)) {
return false;
}
if (!(els instanceof Region)) {
return false;
}
List<IContainer> subBlocks = ((Region) els).getSubBlocks();
if (subBlocks.size() == 1
&& subBlocks.get(0) instanceof IfRegion) {
makeIf((IfRegion) subBlocks.get(0), code, false);
return true;
if (els.contains(AFlag.ELSE_IF_CHAIN) && els instanceof Region) {
List<IContainer> subBlocks = ((Region) els).getSubBlocks();
if (subBlocks.size() == 1) {
IContainer elseBlock = subBlocks.get(0);
if (elseBlock instanceof IfRegion) {
declareVars(code, elseBlock);
makeIf((IfRegion) elseBlock, code, false);
return true;
}
}
}
return false;
}
jadx-core/src/main/java/jadx/core/dex/nodes/IRegion.java
+2
View File
@@ -6,6 +6,8 @@ public interface IRegion extends IContainer {
IRegion getParent();
void setParent(IRegion parent);
List<IContainer> getSubBlocks();
boolean replaceSubBlock(IContainer oldBlock, IContainer newBlock);
jadx-core/src/main/java/jadx/core/dex/regions/AbstractRegion.java
+7
View File
@@ -21,6 +21,7 @@ public abstract class AbstractRegion extends AttrNode implements IRegion {
return parent;
}
@Override
public void setParent(IRegion parent) {
this.parent = parent;
}
@@ -30,4 +31,10 @@ public abstract class AbstractRegion extends AttrNode implements IRegion {
LOG.warn("Replace sub block not supported for class \"{}\"", this.getClass());
return false;
}
public void updateParent(IContainer container, IRegion newParent) {
if (container instanceof IRegion) {
((IRegion) container).setParent(newParent);
}
}
}
jadx-core/src/main/java/jadx/core/dex/regions/Region.java
+2 -3
View File
@@ -21,9 +21,7 @@ public final class Region extends AbstractRegion {
}
public void add(IContainer region) {
if (region instanceof AbstractRegion) {
((AbstractRegion) region).setParent(this);
}
updateParent(region, this);
blocks.add(region);
}
@@ -32,6 +30,7 @@ public final class Region extends AbstractRegion {
int i = blocks.indexOf(oldBlock);
if (i != -1) {
blocks.set(i, newBlock);
updateParent(newBlock, this);
return true;
}
return false;
jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfRegion.java
+3 -1
View File
@@ -105,10 +105,12 @@ public final class IfRegion extends AbstractRegion implements IBranchRegion {
public boolean replaceSubBlock(IContainer oldBlock, IContainer newBlock) {
if (oldBlock == thenRegion) {
thenRegion = newBlock;
updateParent(thenRegion, this);
return true;
}
if (oldBlock == elseRegion) {
elseRegion = newBlock;
updateParent(elseRegion, this);
return true;
}
return false;
@@ -128,6 +130,6 @@ public final class IfRegion extends AbstractRegion implements IBranchRegion {
@Override
public String toString() {
return "IF " + header + " then " + thenRegion + " else " + elseRegion;
return "IF " + header + " then (" + thenRegion + ") else (" + elseRegion + ")";
}
}
jadx-core/src/main/java/jadx/core/dex/visitors/DepthTraversal.java
+2 -6
View File
@@ -10,12 +10,8 @@ public class DepthTraversal {
public static void visit(IDexTreeVisitor visitor, ClassNode cls) {
try {
if (visitor.visit(cls)) {
for (ClassNode inCls : cls.getInnerClasses()) {
visit(visitor, inCls);
}
for (MethodNode mth : cls.getMethods()) {
visit(visitor, mth);
}
cls.getInnerClasses().forEach(inCls -> visit(visitor, inCls));
cls.getMethods().forEach(mth -> visit(visitor, mth));
}
} catch (Exception e) {
ErrorsCounter.classError(cls,
jadx-core/src/main/java/jadx/core/dex/visitors/regions/CleanRegions.java
+10 -26
View File
@@ -1,21 +1,11 @@
package jadx.core.dex.visitors.regions;
import java.util.Iterator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import jadx.core.dex.nodes.BlockNode;
import jadx.core.dex.nodes.IContainer;
import jadx.core.dex.nodes.IRegion;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.regions.Region;
public class CleanRegions {
private static final Logger LOG = LoggerFactory.getLogger(CleanRegions.class);
private CleanRegions() {
}
public static void process(MethodNode mth) {
if (mth.isNoCode() || mth.getBasicBlocks().isEmpty()) {
@@ -24,27 +14,21 @@ public class CleanRegions {
IRegionVisitor removeEmptyBlocks = new AbstractRegionVisitor() {
@Override
public boolean enterRegion(MethodNode mth, IRegion region) {
if (!(region instanceof Region)) {
return true;
}
for (Iterator<IContainer> it = region.getSubBlocks().iterator(); it.hasNext(); ) {
IContainer container = it.next();
if (container instanceof BlockNode) {
BlockNode block = (BlockNode) container;
if (block.getInstructions().isEmpty()) {
try {
it.remove();
} catch (UnsupportedOperationException e) {
LOG.warn("Can't remove block: {} from: {}, mth: {}", block, region, mth);
}
if (region instanceof Region) {
region.getSubBlocks().removeIf(container -> {
if (container instanceof BlockNode) {
BlockNode block = (BlockNode) container;
return block.getInstructions().isEmpty();
}
}
return false;
});
}
return true;
}
};
DepthRegionTraversal.traverse(mth, removeEmptyBlocks);
}
private CleanRegions() {
}
}
jadx-core/src/main/java/jadx/core/dex/visitors/regions/DepthRegionTraversal.java
+1 -3
View File
@@ -54,9 +54,7 @@ public class DepthRegionTraversal {
} else if (container instanceof IRegion) {
IRegion region = (IRegion) container;
if (visitor.enterRegion(mth, region)) {
for (IContainer subCont : region.getSubBlocks()) {
traverseInternal(mth, visitor, subCont);
}
region.getSubBlocks().forEach(subCont -> traverseInternal(mth, visitor, subCont));
}
visitor.leaveRegion(mth, region);
}
jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java
+26 -31
View File
@@ -4,7 +4,6 @@ import java.util.List;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.nodes.IBlock;
import jadx.core.dex.nodes.IContainer;
import jadx.core.dex.nodes.IRegion;
import jadx.core.dex.nodes.MethodNode;
@@ -17,18 +16,22 @@ import jadx.core.utils.RegionUtils;
import static jadx.core.utils.RegionUtils.insnsCount;
public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor, IRegionIterativeVisitor {
public class IfRegionVisitor extends AbstractVisitor {
private static final TernaryVisitor TERNARY_VISITOR = new TernaryVisitor();
private static final ProcessIfRegionVisitor PROCESS_IF_REGION_VISITOR = new ProcessIfRegionVisitor();
private static final RemoveRedundantElseVisitor REMOVE_REDUNDANT_ELSE_VISITOR = new RemoveRedundantElseVisitor();
@Override
public void visit(MethodNode mth) {
// collapse ternary operators
DepthRegionTraversal.traverseIterative(mth, TERNARY_VISITOR);
DepthRegionTraversal.traverse(mth, this);
DepthRegionTraversal.traverseIterative(mth, this);
DepthRegionTraversal.traverse(mth, PROCESS_IF_REGION_VISITOR);
DepthRegionTraversal.traverseIterative(mth, REMOVE_REDUNDANT_ELSE_VISITOR);
}
/**
* Collapse ternary operators
*/
private static class TernaryVisitor implements IRegionIterativeVisitor {
@Override
public boolean visitRegion(MethodNode mth, IRegion region) {
@@ -37,35 +40,28 @@ public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor,
}
}
@Override
public boolean enterRegion(MethodNode mth, IRegion region) {
if (region instanceof IfRegion) {
processIfRegion(mth, (IfRegion) region);
private static class ProcessIfRegionVisitor extends AbstractRegionVisitor {
@Override
public boolean enterRegion(MethodNode mth, IRegion region) {
if (region instanceof IfRegion) {
IfRegion ifRegion = (IfRegion) region;
simplifyIfCondition(ifRegion);
moveReturnToThenBlock(mth, ifRegion);
moveBreakToThenBlock(ifRegion);
markElseIfChains(ifRegion);
}
return true;
}
return true;
}
@Override
public boolean visitRegion(MethodNode mth, IRegion region) {
if (region instanceof IfRegion) {
return removeRedundantElseBlock(mth, (IfRegion) region);
private static class RemoveRedundantElseVisitor implements IRegionIterativeVisitor {
@Override
public boolean visitRegion(MethodNode mth, IRegion region) {
if (region instanceof IfRegion) {
return removeRedundantElseBlock(mth, (IfRegion) region);
}
return false;
}
return false;
}
@Override
public void processBlock(MethodNode mth, IBlock container) {
}
@Override
public void leaveRegion(MethodNode mth, IRegion region) {
}
private static void processIfRegion(MethodNode mth, IfRegion ifRegion) {
simplifyIfCondition(ifRegion);
moveReturnToThenBlock(mth, ifRegion);
moveBreakToThenBlock(ifRegion);
markElseIfChains(ifRegion);
}
private static void simplifyIfCondition(IfRegion ifRegion) {
@@ -90,7 +86,6 @@ public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor,
&& !isIfRegion(elseRegion)) {
invertIfRegion(ifRegion);
}
}
}
jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessVariables.java
+10 -5
View File
@@ -64,7 +64,7 @@ public class ProcessVariables extends AbstractVisitor {
@Override
public String toString() {
return regNum + " " + type;
return "r" + regNum + ":" + type;
}
}
@@ -119,7 +119,7 @@ public class ProcessVariables extends AbstractVisitor {
public CollectUsageRegionVisitor(Map<Variable, Usage> usageMap) {
this.usageMap = usageMap;
args = new ArrayList<>();
this.args = new ArrayList<>();
}
@Override
@@ -177,8 +177,10 @@ public class ProcessVariables extends AbstractVisitor {
if (mth.isNoCode()) {
return;
}
List<RegisterArg> mthArguments = mth.getArguments(true);
Map<Variable, Usage> usageMap = new LinkedHashMap<>();
for (RegisterArg arg : mth.getArguments(true)) {
for (RegisterArg arg : mthArguments) {
addToUsageMap(arg, usageMap);
}
@@ -187,8 +189,7 @@ public class ProcessVariables extends AbstractVisitor {
DepthRegionTraversal.traverse(mth, collect);
// reduce assigns map
List<RegisterArg> mthArgs = mth.getArguments(true);
for (RegisterArg arg : mthArgs) {
for (RegisterArg arg : mthArguments) {
usageMap.remove(new Variable(arg));
}
@@ -350,6 +351,10 @@ public class ProcessVariables extends AbstractVisitor {
}
}
}
// can't declare in else-if chain between 'else' and next 'if'
if (region.contains(AFlag.ELSE_IF_CHAIN)) {
return false;
}
return isAllRegionsAfter(region, pos, u.getAssigns(), regionsOrder)
&& isAllRegionsAfter(region, pos, u.getUseRegions(), regionsOrder);
}
jadx-core/src/main/java/jadx/core/utils/DebugUtils.java
+3 -3
View File
@@ -76,7 +76,7 @@ public class DebugUtils {
}
private static void printRegion(MethodNode mth, IRegion region, String indent, boolean printInsns) {
LOG.debug("{}{}", indent, region);
LOG.debug("{}{} {}", indent, region, region.getAttributesString());
indent += "| ";
for (IContainer container : region.getSubBlocks()) {
if (container instanceof IRegion) {
@@ -99,9 +99,9 @@ public class DebugUtils {
CodeWriter code = new CodeWriter();
ig.makeInsn(insn, code);
String insnStr = code.toString().substring(CodeWriter.NL.length());
LOG.debug("{} - {}", indent, insnStr);
LOG.debug("{}> {}", indent, insnStr);
} catch (CodegenException e) {
LOG.debug("{} - {}", indent, insn);
LOG.debug("{}>!! {}", indent, insn);
}
}
}
jadx-core/src/test/java/jadx/tests/integration/variables/TestVariablesIfElseChain.java
+63
View File
@@ -0,0 +1,63 @@
package jadx.tests.integration.variables;
import org.junit.Test;
import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.IntegrationTest;
import static jadx.tests.api.utils.JadxMatchers.containsOne;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThat;
public class TestVariablesIfElseChain extends IntegrationTest {
public static class TestCls {
String used;
public String test(int a) {
if (a == 0) {
use("zero");
} else if (a == 1) {
String r = m(a);
if (r != null) {
use(r);
}
} else if (a == 2) {
String r = m(a);
if (r != null) {
use(r);
}
} else {
return "miss";
}
return null;
}
public String m(int a) {
return "hit" + a;
}
public void use(String s) {
used = s;
}
public void check() {
test(0);
assertThat(used, is("zero"));
test(1);
assertThat(used, is("hit1"));
test(2);
assertThat(used, is("hit2"));
assertThat(test(5), is("miss"));
}
}
@Test
public void test() {
ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();
assertThat(code, containsOne("return \"miss\";"));
// and compilable
}
}