fix: better code styling for if-else blocks (#1455)

This commit is contained in:
Skylot 2022-04-26 20:10:33 +01:00
parent 3366bf3dec
commit a71b3a71d8
No known key found for this signature in database
GPG Key ID: 1E23F5B52567AA39
17 changed files with 782 additions and 144 deletions

View File

@ -80,6 +80,7 @@ public enum AFlag {
RERUN_SSA_TRANSFORM,
METHOD_CANDIDATE_FOR_INLINE,
USE_LINES_HINTS, // source lines info in methods can be trusted
DISABLE_BLOCKS_LOCK,

View File

@ -278,6 +278,16 @@ public final class IfCondition extends AttrNode {
return list;
}
public int getSourceLine() {
for (InsnNode insn : collectInsns()) {
int line = insn.getSourceLine();
if (line != 0) {
return line;
}
}
return 0;
}
@Nullable
public InsnNode getFirstInsn() {
if (mode == Mode.COMPARE) {

View File

@ -1,12 +1,15 @@
package jadx.core.dex.visitors.debuginfo;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import jadx.api.plugins.input.data.IDebugInfo;
import jadx.api.plugins.input.data.ILocalVar;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.attributes.nodes.LocalVarsDebugInfoAttr;
import jadx.core.dex.attributes.nodes.RegDebugInfoAttr;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.InsnArg;
import jadx.core.dex.instructions.args.RegisterArg;
@ -17,6 +20,7 @@ import jadx.core.dex.visitors.AbstractVisitor;
import jadx.core.dex.visitors.JadxVisitor;
import jadx.core.dex.visitors.blocks.BlockSplitter;
import jadx.core.dex.visitors.ssa.SSATransform;
import jadx.core.utils.ListUtils;
import jadx.core.utils.exceptions.JadxException;
@JadxVisitor(
@ -52,17 +56,30 @@ public class DebugInfoAttachVisitor extends AbstractVisitor {
if (lineMapping.isEmpty()) {
return;
}
Map<Integer, Integer> linesStat = new HashMap<>(); // count repeating lines
for (Map.Entry<Integer, Integer> entry : lineMapping.entrySet()) {
try {
Integer offset = entry.getKey();
InsnNode insn = insnArr[offset];
if (insn != null) {
insn.setSourceLine(entry.getValue());
int line = entry.getValue();
insn.setSourceLine(line);
if (insn.getType() != InsnType.NOP) {
linesStat.merge(line, 1, (v, one) -> v + 1);
}
}
} catch (Exception e) {
mth.addWarnComment("Error attach source line", e);
}
}
// 3 here is allowed maximum for lines repeat,
// can occur in indexed 'for' loops (3 instructions with same line)
List<Map.Entry<Integer, Integer>> repeatingLines = ListUtils.filter(linesStat.entrySet(), p -> p.getValue() > 3);
if (repeatingLines.isEmpty()) {
mth.add(AFlag.USE_LINES_HINTS);
} else {
mth.addDebugComment("Don't trust debug lines info. Repeating lines: " + repeatingLines);
}
}
private void attachDebugInfo(MethodNode mth, List<ILocalVar> localVars, InsnNode[] insnArr) {

View File

@ -38,15 +38,105 @@ public class IfRegionVisitor extends AbstractVisitor {
public boolean enterRegion(MethodNode mth, IRegion region) {
if (region instanceof IfRegion) {
IfRegion ifRegion = (IfRegion) region;
simplifyIfCondition(ifRegion);
moveReturnToThenBlock(mth, ifRegion);
moveBreakToThenBlock(ifRegion);
markElseIfChains(ifRegion);
orderBranches(mth, ifRegion);
markElseIfChains(mth, ifRegion);
}
return true;
}
}
@SuppressWarnings("UnnecessaryReturnStatement")
private static void orderBranches(MethodNode mth, IfRegion ifRegion) {
if (RegionUtils.isEmpty(ifRegion.getElseRegion())) {
return;
}
if (RegionUtils.isEmpty(ifRegion.getThenRegion())) {
invertIfRegion(ifRegion);
return;
}
if (mth.contains(AFlag.USE_LINES_HINTS)) {
int thenLine = RegionUtils.getFirstSourceLine(ifRegion.getThenRegion());
int elseLine = RegionUtils.getFirstSourceLine(ifRegion.getElseRegion());
if (thenLine != 0 && elseLine != 0) {
if (thenLine > elseLine) {
invertIfRegion(ifRegion);
}
return;
}
}
if (ifRegion.simplifyCondition()) {
IfCondition condition = ifRegion.getCondition();
if (condition != null && condition.getMode() == Mode.NOT) {
invertIfRegion(ifRegion);
}
}
int thenSize = insnsCount(ifRegion.getThenRegion());
int elseSize = insnsCount(ifRegion.getElseRegion());
if (isSimpleExitBlock(mth, ifRegion.getElseRegion())) {
if (isSimpleExitBlock(mth, ifRegion.getThenRegion())) {
if (elseSize < thenSize) {
invertIfRegion(ifRegion);
return;
}
}
boolean lastRegion = ifRegion == RegionUtils.getLastRegion(mth.getRegion());
if (elseSize == 1 && lastRegion && mth.isVoidReturn()) {
// single return at method end will be removed later
return;
}
if (!lastRegion) {
invertIfRegion(ifRegion);
}
return;
}
boolean thenExit = RegionUtils.hasExitBlock(ifRegion.getThenRegion());
boolean elseExit = RegionUtils.hasExitBlock(ifRegion.getElseRegion());
if (elseExit && (!thenExit || elseSize < thenSize)) {
invertIfRegion(ifRegion);
return;
}
// move 'if' from 'then' branch to make 'else if' chain
if (isIfRegion(ifRegion.getThenRegion())
&& !isIfRegion(ifRegion.getElseRegion())
&& !thenExit) {
invertIfRegion(ifRegion);
return;
}
// move 'break' into 'then' branch
if (RegionUtils.hasBreakInsn(ifRegion.getElseRegion())) {
invertIfRegion(ifRegion);
return;
}
}
private static boolean isIfRegion(IContainer container) {
if (container instanceof IfRegion) {
return true;
}
if (container instanceof IRegion) {
List<IContainer> subBlocks = ((IRegion) container).getSubBlocks();
return subBlocks.size() == 1 && subBlocks.get(0) instanceof IfRegion;
}
return false;
}
/**
* Mark if-else-if chains
*/
private static void markElseIfChains(MethodNode mth, IfRegion ifRegion) {
if (isSimpleExitBlock(mth, ifRegion.getThenRegion())) {
return;
}
IContainer elsRegion = ifRegion.getElseRegion();
if (elsRegion instanceof Region) {
List<IContainer> subBlocks = ((Region) elsRegion).getSubBlocks();
if (subBlocks.size() == 1 && subBlocks.get(0) instanceof IfRegion) {
subBlocks.get(0).add(AFlag.ELSE_IF_CHAIN);
elsRegion.add(AFlag.ELSE_IF_CHAIN);
}
}
}
private static class RemoveRedundantElseVisitor implements IRegionIterativeVisitor {
@Override
public boolean visitRegion(MethodNode mth, IRegion region) {
@ -57,76 +147,6 @@ public class IfRegionVisitor extends AbstractVisitor {
}
}
private static void simplifyIfCondition(IfRegion ifRegion) {
if (ifRegion.simplifyCondition()) {
IfCondition condition = ifRegion.getCondition();
if (condition.getMode() == Mode.NOT) {
invertIfRegion(ifRegion);
}
}
IContainer elseRegion = ifRegion.getElseRegion();
if (elseRegion == null || RegionUtils.isEmpty(elseRegion)) {
return;
}
boolean thenIsEmpty = RegionUtils.isEmpty(ifRegion.getThenRegion());
if (thenIsEmpty || hasSimpleReturnBlock(ifRegion.getThenRegion())) {
invertIfRegion(ifRegion);
}
if (!thenIsEmpty) {
// move 'if' from then to make 'else if' chain
if (isIfRegion(ifRegion.getThenRegion())
&& !isIfRegion(elseRegion)) {
invertIfRegion(ifRegion);
}
}
}
private static boolean isIfRegion(IContainer container) {
if (container instanceof IfRegion) {
return true;
}
if (container instanceof IRegion) {
List<IContainer> subBlocks = ((IRegion) container).getSubBlocks();
if (subBlocks.size() == 1 && subBlocks.get(0) instanceof IfRegion) {
return true;
}
}
return false;
}
private static void moveReturnToThenBlock(MethodNode mth, IfRegion ifRegion) {
if (!mth.isVoidReturn()
&& hasSimpleReturnBlock(ifRegion.getElseRegion())
/* && insnsCount(ifRegion.getThenRegion()) < 2 */) {
invertIfRegion(ifRegion);
}
}
private static void moveBreakToThenBlock(IfRegion ifRegion) {
if (ifRegion.getElseRegion() != null
&& RegionUtils.hasBreakInsn(ifRegion.getElseRegion())) {
invertIfRegion(ifRegion);
}
}
/**
* Mark if-else-if chains
*/
private static void markElseIfChains(IfRegion ifRegion) {
if (hasSimpleReturnBlock(ifRegion.getThenRegion())) {
return;
}
IContainer elsRegion = ifRegion.getElseRegion();
if (elsRegion instanceof Region) {
List<IContainer> subBlocks = ((Region) elsRegion).getSubBlocks();
if (subBlocks.size() == 1 && subBlocks.get(0) instanceof IfRegion) {
subBlocks.get(0).add(AFlag.ELSE_IF_CHAIN);
elsRegion.add(AFlag.ELSE_IF_CHAIN);
}
}
}
private static boolean removeRedundantElseBlock(MethodNode mth, IfRegion ifRegion) {
if (ifRegion.getElseRegion() == null
|| ifRegion.contains(AFlag.ELSE_IF_CHAIN)
@ -162,16 +182,16 @@ public class IfRegionVisitor extends AbstractVisitor {
}
}
private static boolean hasSimpleReturnBlock(IContainer region) {
if (region == null) {
private static boolean isSimpleExitBlock(MethodNode mth, IContainer container) {
if (container == null) {
return false;
}
if (region.contains(AFlag.RETURN)) {
if (container.contains(AFlag.RETURN) || RegionUtils.isExitBlock(mth, container)) {
return true;
}
if (region instanceof IRegion) {
List<IContainer> subBlocks = ((IRegion) region).getSubBlocks();
return subBlocks.size() == 1 && subBlocks.get(0).contains(AFlag.RETURN);
if (container instanceof IRegion) {
List<IContainer> subBlocks = ((IRegion) container).getSubBlocks();
return subBlocks.size() == 1 && RegionUtils.isExitBlock(mth, subBlocks.get(0));
}
return false;
}

View File

@ -93,7 +93,8 @@ public class TernaryMod extends AbstractRegionVisitor implements IRegionIterativ
InsnNode thenInsn = tb.getInstructions().get(0);
InsnNode elseInsn = eb.getInstructions().get(0);
if (thenInsn.getSourceLine() != elseInsn.getSourceLine()) {
if (mth.contains(AFlag.USE_LINES_HINTS)
&& thenInsn.getSourceLine() != elseInsn.getSourceLine()) {
if (thenInsn.getSourceLine() != 0 && elseInsn.getSourceLine() != 0) {
// sometimes source lines incorrect
if (!checkLineStats(thenInsn, elseInsn)) {
@ -134,6 +135,8 @@ public class TernaryMod extends AbstractRegionVisitor implements IRegionIterativ
InsnArg thenArg = InsnArg.wrapInsnIntoArg(thenInsn);
InsnArg elseArg = InsnArg.wrapInsnIntoArg(elseInsn);
TernaryInsn ternInsn = new TernaryInsn(ifRegion.getCondition(), resArg, thenArg, elseArg);
int branchLine = Math.max(thenInsn.getSourceLine(), elseInsn.getSourceLine());
ternInsn.setSourceLine(Math.max(ifRegion.getSourceLine(), branchLine));
InsnRemover.unbindResult(mth, elseInsn);

View File

@ -182,6 +182,16 @@ public class BlockUtils {
return null;
}
public static int getFirstSourceLine(IBlock block) {
for (InsnNode insn : block.getInstructions()) {
int line = insn.getSourceLine();
if (line != 0) {
return line;
}
}
return 0;
}
@Nullable
public static InsnNode getFirstInsn(@Nullable IBlock block) {
if (block == null) {
@ -207,11 +217,22 @@ public class BlockUtils {
}
public static boolean isExitBlock(MethodNode mth, BlockNode block) {
BlockNode exitBlock = mth.getExitBlock();
if (block == exitBlock) {
if (block == mth.getExitBlock()) {
return true;
}
return exitBlock.getPredecessors().contains(block);
return isExitBlock(block);
}
public static boolean isExitBlock(BlockNode block) {
List<BlockNode> successors = block.getSuccessors();
if (successors.isEmpty()) {
return true;
}
if (successors.size() == 1) {
BlockNode next = successors.get(0);
return next.getSuccessors().isEmpty();
}
return false;
}
public static boolean containsExitInsn(IBlock block) {

View File

@ -16,6 +16,7 @@ import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.nodes.BlockNode;
import jadx.core.dex.nodes.IBlock;
import jadx.core.dex.nodes.IBranchRegion;
import jadx.core.dex.nodes.IConditionRegion;
import jadx.core.dex.nodes.IContainer;
import jadx.core.dex.nodes.IRegion;
import jadx.core.dex.nodes.InsnNode;
@ -52,6 +53,7 @@ public class RegionUtils {
throw new JadxRuntimeException(unknownContainerType(container));
}
@Nullable
public static InsnNode getFirstInsn(IContainer container) {
if (container instanceof IBlock) {
IBlock block = (IBlock) container;
@ -74,6 +76,37 @@ public class RegionUtils {
}
}
public static int getFirstSourceLine(IContainer container) {
if (container instanceof IBlock) {
return BlockUtils.getFirstSourceLine((IBlock) container);
}
if (container instanceof IConditionRegion) {
return ((IConditionRegion) container).getConditionSourceLine();
}
if (container instanceof IBranchRegion) {
IBranchRegion branchRegion = (IBranchRegion) container;
return getFirstSourceLine(branchRegion.getBranches());
}
if (container instanceof IRegion) {
IRegion region = (IRegion) container;
return getFirstSourceLine(region.getSubBlocks());
}
return 0;
}
private static int getFirstSourceLine(List<IContainer> containers) {
if (containers.isEmpty()) {
return 0;
}
for (IContainer container : containers) {
int line = getFirstSourceLine(container);
if (line != 0) {
return line;
}
}
return 0;
}
public static InsnNode getLastInsn(IContainer container) {
if (container instanceof IBlock) {
IBlock block = (IBlock) container;
@ -112,31 +145,58 @@ public class RegionUtils {
}
}
@Nullable
public static IContainer getLastRegion(@Nullable IContainer container) {
if (container == null) {
return null;
}
if (container instanceof IBlock || container instanceof IBranchRegion) {
return container;
}
if (container instanceof IRegion) {
return getLastRegion(Utils.last(((IRegion) container).getSubBlocks()));
}
throw new JadxRuntimeException(unknownContainerType(container));
}
public static boolean isExitBlock(MethodNode mth, IContainer container) {
if (container instanceof BlockNode) {
return BlockUtils.isExitBlock(mth, (BlockNode) container);
}
return false;
}
/**
* Return true if last block in region has no successors or jump out insn (return or break)
*/
public static boolean hasExitBlock(IContainer container) {
if (container == null) {
return false;
}
return hasExitBlock(container, container);
}
private static boolean hasExitBlock(IContainer rootContainer, IContainer container) {
if (container instanceof BlockNode) {
BlockNode blockNode = (BlockNode) container;
if (blockNode.getSuccessors().isEmpty()) {
if (BlockUtils.isExitBlock(blockNode)) {
return true;
}
return isInsnExitContainer(rootContainer, (IBlock) container);
} else if (container instanceof IBranchRegion) {
return false;
} else if (container instanceof IBlock) {
}
if (container instanceof IBranchRegion) {
IBranchRegion branchRegion = (IBranchRegion) container;
return ListUtils.allMatch(branchRegion.getBranches(), RegionUtils::hasExitBlock);
}
if (container instanceof IBlock) {
return isInsnExitContainer(rootContainer, (IBlock) container);
} else if (container instanceof IRegion) {
}
if (container instanceof IRegion) {
List<IContainer> blocks = ((IRegion) container).getSubBlocks();
return !blocks.isEmpty()
&& hasExitBlock(rootContainer, blocks.get(blocks.size() - 1));
} else {
throw new JadxRuntimeException(unknownContainerType(container));
}
throw new JadxRuntimeException(unknownContainerType(container));
}
private static boolean isInsnExitContainer(IContainer rootContainer, IBlock block) {
@ -192,17 +252,25 @@ public class RegionUtils {
public static int insnsCount(IContainer container) {
if (container instanceof IBlock) {
return ((IBlock) container).getInstructions().size();
} else if (container instanceof IRegion) {
List<InsnNode> insnList = ((IBlock) container).getInstructions();
int count = 0;
for (InsnNode insn : insnList) {
if (insn.contains(AFlag.DONT_GENERATE)) {
continue;
}
count++;
}
return count;
}
if (container instanceof IRegion) {
IRegion region = (IRegion) container;
int count = 0;
for (IContainer block : region.getSubBlocks()) {
count += insnsCount(block);
}
return count;
} else {
throw new JadxRuntimeException(unknownContainerType(container));
}
throw new JadxRuntimeException(unknownContainerType(container));
}
public static boolean isEmpty(IContainer container) {

View File

@ -150,9 +150,9 @@ public abstract class IntegrationTest extends TestUtils {
close(decompiledCompiler);
}
private void close(Closeable cloaseble) throws IOException {
if (cloaseble != null) {
cloaseble.close();
private void close(Closeable closeable) throws IOException {
if (closeable != null) {
closeable.close();
}
}

View File

@ -8,6 +8,7 @@ import jadx.tests.api.IntegrationTest;
import static jadx.tests.api.utils.JadxMatchers.containsOne;
import static org.hamcrest.MatcherAssert.assertThat;
@SuppressWarnings({ "PointlessBooleanExpression", "unused" })
public class TestBitwiseAnd extends IntegrationTest {
public static class TestCls {

View File

@ -10,6 +10,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
public class TestConditions14 extends IntegrationTest {
@SuppressWarnings({ "EqualsReplaceableByObjectsCall", "ConstantConditions" })
public static class TestCls {
public static boolean test(Object a, Object b) {

View File

@ -0,0 +1,160 @@
package jadx.tests.integration.conditions;
import org.junit.jupiter.api.Test;
import jadx.tests.api.SmaliTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
/**
* Issue #1455
*/
public class TestIfCodeStyle extends SmaliTest {
@SuppressWarnings({ "ConstantConditions", "FieldCanBeLocal", "unused" })
public static class TestCls {
private String moduleName;
private String modulePath;
private String preinstalledModulePath;
private long versionCode;
private String versionName;
private boolean isFactory;
private boolean isActive;
public void test(Parcel parcel) {
int startPos = parcel.dataPosition();
int size = parcel.readInt();
if (size < 0) {
if (startPos > Integer.MAX_VALUE - size) {
throw new RuntimeException("Overflow in the size of parcelable");
}
parcel.setDataPosition(startPos + size);
return;
}
try {
if (parcel.dataPosition() - startPos >= size) {
if (startPos > Integer.MAX_VALUE - size) {
throw new RuntimeException("Overflow in the size of parcelable");
}
parcel.setDataPosition(startPos + size);
return;
}
this.moduleName = parcel.readString();
if (parcel.dataPosition() - startPos >= size) {
if (startPos > Integer.MAX_VALUE - size) {
throw new RuntimeException("Overflow in the size of parcelable");
}
parcel.setDataPosition(startPos + size);
return;
}
this.modulePath = parcel.readString();
if (parcel.dataPosition() - startPos >= size) {
if (startPos > Integer.MAX_VALUE - size) {
throw new RuntimeException("Overflow in the size of parcelable");
}
parcel.setDataPosition(startPos + size);
return;
}
this.preinstalledModulePath = parcel.readString();
if (parcel.dataPosition() - startPos >= size) {
if (startPos > Integer.MAX_VALUE - size) {
throw new RuntimeException("Overflow in the size of parcelable");
}
parcel.setDataPosition(startPos + size);
return;
}
this.versionCode = parcel.readLong();
if (parcel.dataPosition() - startPos >= size) {
if (startPos > Integer.MAX_VALUE - size) {
throw new RuntimeException("Overflow in the size of parcelable");
}
parcel.setDataPosition(startPos + size);
return;
}
this.versionName = parcel.readString();
if (parcel.dataPosition() - startPos >= size) {
if (startPos > Integer.MAX_VALUE - size) {
throw new RuntimeException("Overflow in the size of parcelable");
}
parcel.setDataPosition(startPos + size);
return;
}
this.isFactory = parcel.readInt() != 0;
if (parcel.dataPosition() - startPos >= size) {
if (startPos > Integer.MAX_VALUE - size) {
throw new RuntimeException("Overflow in the size of parcelable");
}
parcel.setDataPosition(startPos + size);
return;
}
this.isActive = parcel.readInt() != 0;
if (startPos > Integer.MAX_VALUE - size) {
throw new RuntimeException("Overflow in the size of parcelable");
}
parcel.setDataPosition(startPos + size);
} catch (Throwable e) {
if (startPos <= Integer.MAX_VALUE - size) {
parcel.setDataPosition(startPos + size);
throw e;
}
throw new RuntimeException("Overflow in the size of parcelable");
}
}
private static class Parcel {
public void setDataPosition(int i) {
}
public int dataPosition() {
return 0;
}
public int readInt() {
return 0;
}
public String readString() {
return null;
}
public long readLong() {
return 0;
}
}
}
@Test
public void test() {
noDebugInfo();
assertThat(getClassNode(TestCls.class))
.code()
.doesNotContain("else")
.countString(8, "return;")
.containsLines(2,
"if (readInt < 0) {",
indent() + "if (dataPosition > Integer.MAX_VALUE - readInt) {",
indent(2) + "throw new RuntimeException(\"Overflow in the size of parcelable\");",
indent() + "}",
indent() + "parcel.setDataPosition(dataPosition + readInt);",
indent() + "return;",
"}");
}
@Test
public void testSmali() {
disableCompilation();
assertThat(getClassNodeFromSmali())
.code()
.doesNotContain("else")
.countString(8, "return;")
.containsLines(2,
"if (_aidl_parcelable_size < 0) {",
indent() + "if (_aidl_start_pos > Integer.MAX_VALUE - _aidl_parcelable_size) {",
indent(2) + "throw new RuntimeException(\"Overflow in the size of parcelable\");",
indent() + "}",
indent() + "_aidl_parcel.setDataPosition(_aidl_start_pos + _aidl_parcelable_size);",
indent() + "return;",
"}");
}
}

View File

@ -2,10 +2,10 @@ package jadx.tests.integration.debuginfo;
import java.lang.ref.WeakReference;
import org.junit.jupiter.api.Test;
import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.IntegrationTest;
import jadx.tests.api.extensions.profiles.TestProfile;
import jadx.tests.api.extensions.profiles.TestWithProfiles;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -32,17 +32,16 @@ public class TestLineNumbers2 extends IntegrationTest {
}
}
@Test
@TestWithProfiles({ TestProfile.DX_J8, TestProfile.JAVA8 })
public void test() {
printLineNumbers();
ClassNode cls = getClassNode(TestCls.class);
String linesMapStr = cls.getCode().getLineMapping().toString();
if (isJavaInput()) {
assertEquals("{6=16, 9=17, 12=21, 13=22, 14=23, 16=25, 18=27, 21=30}", linesMapStr);
assertEquals("{6=16, 9=17, 12=21, 13=22, 14=23, 15=24, 16=25, 18=27, 21=30, 22=31}", linesMapStr);
} else {
// TODO: invert condition to match source lines
assertEquals("{6=16, 9=17, 12=21, 13=22, 14=23, 15=27, 17=24, 18=25, 19=27, 22=30, 23=31}", linesMapStr);
assertEquals("{6=16, 9=17, 12=21, 13=22, 14=23, 15=24, 16=25, 17=27, 19=27, 22=30, 23=31}", linesMapStr);
}
}
}

View File

@ -2,14 +2,9 @@ package jadx.tests.integration.loops;
import org.junit.jupiter.api.Test;
import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.IntegrationTest;
import static jadx.tests.api.utils.JadxMatchers.containsOne;
import static jadx.tests.api.utils.JadxMatchers.countString;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
public class TestSequentialLoops extends IntegrationTest {
@ -35,13 +30,10 @@ public class TestSequentialLoops extends IntegrationTest {
@Test
public void test() {
disableCompilation();
ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();
assertThat(code, countString(2, "while ("));
assertThat(code, containsOne("break;"));
assertThat(code, containsOne("return c;"));
assertThat(code, not(containsString("else")));
assertThat(getClassNode(TestCls.class))
.code()
.countString(2, "while (")
.containsOne("break;")
.containsOne("return c;");
}
}

View File

@ -23,7 +23,8 @@ public class TestLoopInTryCatch extends SmaliTest {
" break;",
" }",
"}",
"if (i == 1) {",
"if (i != 1) {",
" getI();",
"}"),
c -> c.containsLines(2,
"int i;",
@ -37,7 +38,8 @@ public class TestLoopInTryCatch extends SmaliTest {
" return;",
" }",
"}",
"if (i == 1) {",
"if (i != 1) {",
" getI();",
"}"));
}
}

View File

@ -0,0 +1,354 @@
.class public Lconditions/TestIfCodeStyle;
.super Ljava/lang/Object;
.implements Landroid/os/Parcelable;
.field public isActive:Z
.field public isFactory:Z
.field public moduleName:Ljava/lang/String;
.field public modulePath:Ljava/lang/String;
.field public preinstalledModulePath:Ljava/lang/String;
.field public versionCode:J
.field public versionName:Ljava/lang/String;
.method public final readFromParcel(Landroid/os/Parcel;)V
.registers 9
.param p1, "_aidl_parcel" # Landroid/os/Parcel;
.line 44
invoke-virtual {p1}, Landroid/os/Parcel;->dataPosition()I
move-result v0
.line 45
.local v0, "_aidl_start_pos":I
invoke-virtual {p1}, Landroid/os/Parcel;->readInt()I
move-result v1
.line 47
.local v1, "_aidl_parcelable_size":I
const-string v2, "Overflow in the size of parcelable"
const v3, 0x7fffffff
if-gez v1, :cond_1e
.line 63
sub-int/2addr v3, v1
if-gt v0, v3, :cond_18
.line 66
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 47
return-void
.line 64
:cond_18
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 48
:cond_1e
:try_start_1e
invoke-virtual {p1}, Landroid/os/Parcel;->dataPosition()I
move-result v4
:try_end_22
.catchall {:try_start_1e .. :try_end_22} :catchall_fd
sub-int/2addr v4, v0
if-lt v4, v1, :cond_34
.line 63
sub-int/2addr v3, v1
if-gt v0, v3, :cond_2e
.line 66
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 48
return-void
.line 64
:cond_2e
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 49
:cond_34
:try_start_34
invoke-virtual {p1}, Landroid/os/Parcel;->readString()Ljava/lang/String;
move-result-object v4
iput-object v4, p0, Lconditions/TestIfCodeStyle;->moduleName:Ljava/lang/String;
.line 50
invoke-virtual {p1}, Landroid/os/Parcel;->dataPosition()I
move-result v4
:try_end_3e
.catchall {:try_start_34 .. :try_end_3e} :catchall_fd
sub-int/2addr v4, v0
if-lt v4, v1, :cond_50
.line 63
sub-int/2addr v3, v1
if-gt v0, v3, :cond_4a
.line 66
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 50
return-void
.line 64
:cond_4a
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 51
:cond_50
:try_start_50
invoke-virtual {p1}, Landroid/os/Parcel;->readString()Ljava/lang/String;
move-result-object v4
iput-object v4, p0, Lconditions/TestIfCodeStyle;->modulePath:Ljava/lang/String;
.line 52
invoke-virtual {p1}, Landroid/os/Parcel;->dataPosition()I
move-result v4
:try_end_5a
.catchall {:try_start_50 .. :try_end_5a} :catchall_fd
sub-int/2addr v4, v0
if-lt v4, v1, :cond_6c
.line 63
sub-int/2addr v3, v1
if-gt v0, v3, :cond_66
.line 66
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 52
return-void
.line 64
:cond_66
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 53
:cond_6c
:try_start_6c
invoke-virtual {p1}, Landroid/os/Parcel;->readString()Ljava/lang/String;
move-result-object v4
iput-object v4, p0, Lconditions/TestIfCodeStyle;->preinstalledModulePath:Ljava/lang/String;
.line 54
invoke-virtual {p1}, Landroid/os/Parcel;->dataPosition()I
move-result v4
:try_end_76
.catchall {:try_start_6c .. :try_end_76} :catchall_fd
sub-int/2addr v4, v0
if-lt v4, v1, :cond_88
.line 63
sub-int/2addr v3, v1
if-gt v0, v3, :cond_82
.line 66
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 54
return-void
.line 64
:cond_82
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 55
:cond_88
:try_start_88
invoke-virtual {p1}, Landroid/os/Parcel;->readLong()J
move-result-wide v4
iput-wide v4, p0, Lconditions/TestIfCodeStyle;->versionCode:J
.line 56
invoke-virtual {p1}, Landroid/os/Parcel;->dataPosition()I
move-result v4
:try_end_92
.catchall {:try_start_88 .. :try_end_92} :catchall_fd
sub-int/2addr v4, v0
if-lt v4, v1, :cond_a4
.line 63
sub-int/2addr v3, v1
if-gt v0, v3, :cond_9e
.line 66
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 56
return-void
.line 64
:cond_9e
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 57
:cond_a4
:try_start_a4
invoke-virtual {p1}, Landroid/os/Parcel;->readString()Ljava/lang/String;
move-result-object v4
iput-object v4, p0, Lconditions/TestIfCodeStyle;->versionName:Ljava/lang/String;
.line 58
invoke-virtual {p1}, Landroid/os/Parcel;->dataPosition()I
move-result v4
:try_end_ae
.catchall {:try_start_a4 .. :try_end_ae} :catchall_fd
sub-int/2addr v4, v0
if-lt v4, v1, :cond_c0
.line 63
sub-int/2addr v3, v1
if-gt v0, v3, :cond_ba
.line 66
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 58
return-void
.line 64
:cond_ba
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 59
:cond_c0
:try_start_c0
invoke-virtual {p1}, Landroid/os/Parcel;->readInt()I
move-result v4
const/4 v5, 0x1
const/4 v6, 0x0
if-eqz v4, :cond_ca
move v4, v5
goto :goto_cb
:cond_ca
move v4, v6
:goto_cb
iput-boolean v4, p0, Lconditions/TestIfCodeStyle;->isFactory:Z
.line 60
invoke-virtual {p1}, Landroid/os/Parcel;->dataPosition()I
move-result v4
:try_end_d1
.catchall {:try_start_c0 .. :try_end_d1} :catchall_fd
sub-int/2addr v4, v0
if-lt v4, v1, :cond_e3
.line 63
sub-int/2addr v3, v1
if-gt v0, v3, :cond_dd
.line 66
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 60
return-void
.line 64
:cond_dd
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 61
:cond_e3
:try_start_e3
invoke-virtual {p1}, Landroid/os/Parcel;->readInt()I
move-result v4
if-eqz v4, :cond_ea
goto :goto_eb
:cond_ea
move v5, v6
:goto_eb
iput-boolean v5, p0, Lconditions/TestIfCodeStyle;->isActive:Z
:try_end_ed
.catchall {:try_start_e3 .. :try_end_ed} :catchall_fd
.line 63
sub-int/2addr v3, v1
if-gt v0, v3, :cond_f7
.line 66
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 67
nop
.line 68
return-void
.line 64
:cond_f7
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 63
:catchall_fd
move-exception v4
sub-int/2addr v3, v1
if-le v0, v3, :cond_107
.line 64
new-instance v3, Ljava/lang/RuntimeException;
invoke-direct {v3, v2}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
throw v3
.line 66
:cond_107
add-int v2, v0, v1
invoke-virtual {p1, v2}, Landroid/os/Parcel;->setDataPosition(I)V
.line 67
throw v4
.end method

View File

@ -24,7 +24,7 @@
:cond
if-eq v1, v2, :end
invoke-static {}, Ltrycatch/TestLoopInTryCatch;->getI()I
return-void
:try_end

View File

@ -101,25 +101,21 @@ public class DebugInfoParser {
varsInfoFound = true;
}
}
// process '0' instruction
addrChange(-1, 1, line);
setLine(addr, line);
while (true) {
int c = in.readUByte();
while (c != DBG_END_SEQUENCE) {
if (c == DBG_END_SEQUENCE) {
break;
}
switch (c) {
case DBG_ADVANCE_PC: {
int addrInc = in.readUleb128();
addr = addrChange(addr, addrInc, line);
setLine(addr, line);
addr = addrChange(addr, addrInc);
break;
}
case DBG_ADVANCE_LINE: {
line += in.readSleb128();
break;
}
case DBG_START_LOCAL: {
int regNum = in.readUleb128();
int nameId = in.readUleb128() - 1;
@ -154,30 +150,26 @@ public class DebugInfoParser {
varsInfoFound = true;
break;
}
case DBG_SET_PROLOGUE_END:
case DBG_SET_EPILOGUE_BEGIN:
case DBG_SET_EPILOGUE_BEGIN: {
// do nothing
break;
}
case DBG_SET_FILE: {
int idx = in.readUleb128() - 1;
this.sourceFile = ext.getString(idx);
break;
}
default: {
int adjustedOpCode = c - DBG_FIRST_SPECIAL;
int addrInc = adjustedOpCode / DBG_LINE_RANGE;
addr = addrChange(addr, addrInc, line);
addr = addrChange(addr, addrInc);
line += DBG_LINE_BASE + adjustedOpCode % DBG_LINE_RANGE;
setLine(addr, line);
break;
}
}
c = in.readUByte();
}
if (varsInfoFound) {
for (DexLocalVar var : locals) {
if (var != null && !var.isEnd()) {
@ -185,14 +177,11 @@ public class DebugInfoParser {
}
}
}
return new DebugInfo(linesMap, resultList);
}
private int addrChange(int addr, int addrInc, int line) {
int newAddr = Math.min(addr + addrInc, codeSize - 1);
setLine(newAddr, line);
return newAddr;
private int addrChange(int addr, int addrInc) {
return Math.min(addr + addrInc, codeSize - 1);
}
private void setLine(int offset, int line) {