Skip to content

Commit

Permalink
fix: inline constant strings (#685)
Browse files Browse the repository at this point in the history
  • Loading branch information
@skylot
skylot committed Jul 5, 2019
1 parent 8410e62 commit 424a8ff
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 83 deletions.
6 changes: 6 additions & 0 deletions jadx-core/src/main/java/jadx/core/dex/instructions/args/InsnArg.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ public boolean isThis() {
return contains(AFlag.THIS);
}

protected InsnArg copyCommonParams(InsnArg copy) {
copy.copyAttributesFrom(this);
copy.setParentInsn(parentInsn);
return copy;
}

public InsnArg duplicate() {
return this;
}
Expand Down
5 changes: 5 additions & 0 deletions jadx-core/src/main/java/jadx/core/dex/instructions/args/InsnWrapArg.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ public void setParentInsn(InsnNode parentInsn) {
this.parentInsn = parentInsn;
}

@Override
public InsnArg duplicate() {
return copyCommonParams(new InsnWrapArg(wrappedInsn.copy()));
}

@Override
public boolean isInsnWrap() {
return true;
Expand Down
107 changes: 66 additions & 41 deletions jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.List;

import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.instructions.ConstStringNode;
import jadx.core.dex.instructions.IndexInsnNode;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.InvokeNode;
Expand Down Expand Up @@ -51,36 +52,53 @@ public void visit(MethodNode mth) throws JadxException {
}

private static void checkInsn(MethodNode mth, InsnNode insn, List<InsnNode> toRemove) {
if (insn.contains(AFlag.DONT_INLINE) || insn.contains(AFlag.DONT_GENERATE)) {
if (insn.contains(AFlag.DONT_INLINE)
|| insn.contains(AFlag.DONT_GENERATE)
|| insn.getResult() == null) {
return;
}
InsnType insnType = insn.getType();
if (insnType != InsnType.CONST && insnType != InsnType.MOVE) {
return;
}
InsnArg arg = insn.getArg(0);
if (!arg.isLiteral()) {
return;
}
long lit = ((LiteralArg) arg).getLiteral();

SSAVar sVar = insn.getResult().getSVar();
if (lit == 0 && checkObjectInline(sVar)) {
if (sVar.getUseCount() == 1) {
InsnNode assignInsn = insn.getResult().getAssignInsn();
if (assignInsn != null) {
assignInsn.add(AFlag.DONT_INLINE);
InsnArg constArg;

InsnType insnType = insn.getType();
if (insnType == InsnType.CONST || insnType == InsnType.MOVE) {
constArg = insn.getArg(0);
if (!constArg.isLiteral()) {
return;
}
long lit = ((LiteralArg) constArg).getLiteral();
if (lit == 0 && checkObjectInline(sVar)) {
if (sVar.getUseCount() == 1) {
InsnNode assignInsn = insn.getResult().getAssignInsn();
if (assignInsn != null) {
assignInsn.add(AFlag.DONT_INLINE);
}
}
return;
}
} else if (insnType == InsnType.CONST_STR) {
if (sVar.isUsedInPhi()) {
return;
}
String s = ((ConstStringNode) insn).getString();
FieldNode f = mth.getParentClass().getConstField(s);
if (f == null) {
constArg = InsnArg.wrapArg(insn.copy());
} else {
InsnNode constGet = new IndexInsnNode(InsnType.SGET, f.getFieldInfo(), 0);
constGet.setResult(insn.getResult().duplicate());
constArg = InsnArg.wrapArg(constGet);
}
} else {
return;
}

if (checkForFinallyBlock(sVar)) {
return;
}

// all check passed, run replace
replaceConst(mth, insn, lit, toRemove);
replaceConst(mth, insn, constArg, toRemove);
}

private static boolean checkForFinallyBlock(SSAVar sVar) {
Expand Down Expand Up @@ -131,12 +149,12 @@ private static boolean checkObjectInline(SSAVar sVar) {
return false;
}

private static int replaceConst(MethodNode mth, InsnNode constInsn, long literal, List<InsnNode> toRemove) {
private static int replaceConst(MethodNode mth, InsnNode constInsn, InsnArg constArg, List<InsnNode> toRemove) {
SSAVar ssaVar = constInsn.getResult().getSVar();
List<RegisterArg> useList = new ArrayList<>(ssaVar.getUseList());
int replaceCount = 0;
for (RegisterArg arg : useList) {
if (replaceArg(mth, arg, literal, constInsn, toRemove)) {
if (replaceArg(mth, arg, constArg, constInsn, toRemove)) {
replaceCount++;
}
}
Expand All @@ -146,7 +164,7 @@ private static int replaceConst(MethodNode mth, InsnNode constInsn, long literal
return replaceCount;
}

private static boolean replaceArg(MethodNode mth, RegisterArg arg, long literal, InsnNode constInsn, List<InsnNode> toRemove) {
private static boolean replaceArg(MethodNode mth, RegisterArg arg, InsnArg constArg, InsnNode constInsn, List<InsnNode> toRemove) {
InsnNode useInsn = arg.getParentInsn();
if (useInsn == null) {
return false;
Expand All @@ -155,33 +173,40 @@ private static boolean replaceArg(MethodNode mth, RegisterArg arg, long literal,
if (insnType == InsnType.PHI) {
return false;
}
ArgType argType = arg.getInitType();
if (argType.isObject() && literal != 0) {
argType = ArgType.NARROW_NUMBERS;
}
LiteralArg litArg = InsnArg.lit(literal, argType);
if (!useInsn.replaceArg(arg, litArg)) {
return false;
}
// arg replaced, made some optimizations
litArg.setType(arg.getInitType());

FieldNode fieldNode = null;
ArgType litArgType = litArg.getType();
if (litArgType.isTypeKnown()) {
fieldNode = mth.getParentClass().getConstFieldByLiteralArg(litArg);
} else if (litArgType.contains(PrimitiveType.INT)) {
fieldNode = mth.getParentClass().getConstField((int) literal, false);
}
if (fieldNode != null) {
litArg.wrapInstruction(new IndexInsnNode(InsnType.SGET, fieldNode.getFieldInfo(), 0));
}
if (constArg.isLiteral()) {
long literal = ((LiteralArg) constArg).getLiteral();
ArgType argType = arg.getInitType();
if (argType.isObject() && literal != 0) {
argType = ArgType.NARROW_NUMBERS;
}
LiteralArg litArg = InsnArg.lit(literal, argType);
if (!useInsn.replaceArg(arg, litArg)) {
return false;
}
// arg replaced, made some optimizations
litArg.setType(arg.getInitType());

FieldNode fieldNode = null;
ArgType litArgType = litArg.getType();
if (litArgType.isTypeKnown()) {
fieldNode = mth.getParentClass().getConstFieldByLiteralArg(litArg);
} else if (litArgType.contains(PrimitiveType.INT)) {
fieldNode = mth.getParentClass().getConstField((int) literal, false);
}
if (fieldNode != null) {
litArg.wrapInstruction(new IndexInsnNode(InsnType.SGET, fieldNode.getFieldInfo(), 0));
}
} else {
if (!useInsn.replaceArg(arg, constArg.duplicate())) {
return false;
}
}
if (insnType == InsnType.RETURN) {
useInsn.setSourceLine(constInsn.getSourceLine());
} else if (insnType == InsnType.MOVE) {
try {
replaceConst(mth, useInsn, literal, toRemove);
replaceConst(mth, useInsn, constArg, toRemove);
} catch (StackOverflowError e) {
throw new JadxOverflowException("Stack overflow at const inline visitor");
}
Expand Down
86 changes: 44 additions & 42 deletions jadx-core/src/main/java/jadx/core/dex/visitors/shrink/CodeShrinkVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,48 +57,11 @@ private static void shrinkBlock(MethodNode mth, BlockNode block) {
List<WrapInfo> wrapList = new ArrayList<>();
for (ArgsInfo argsInfo : argsList) {
List<RegisterArg> args = argsInfo.getArgs();
if (args.isEmpty()) {
continue;
}
ListIterator<RegisterArg> it = args.listIterator(args.size());
while (it.hasPrevious()) {
RegisterArg arg = it.previous();
// if (arg.getName() != null) {
// continue;
// }
SSAVar sVar = arg.getSVar();
// allow inline only one use arg
if (sVar == null
|| sVar.getVariableUseCount() != 1
|| sVar.contains(AFlag.DONT_INLINE)) {
continue;
}
InsnNode assignInsn = sVar.getAssign().getParentInsn();
if (assignInsn == null || assignInsn.contains(AFlag.DONT_INLINE)) {
continue;
}
List<RegisterArg> useList = sVar.getUseList();
if (!useList.isEmpty()) {
InsnNode parentInsn = useList.get(0).getParentInsn();
if (parentInsn != null && parentInsn.contains(AFlag.DONT_GENERATE)) {
continue;
}
}

int assignPos = insnList.getIndex(assignInsn);
if (assignPos != -1) {
WrapInfo wrapInfo = argsInfo.checkInline(assignPos, arg);
if (wrapInfo != null) {
wrapList.add(wrapInfo);
}
} else {
// another block
BlockNode assignBlock = BlockUtils.getBlockByInsn(mth, assignInsn);
if (assignBlock != null
&& assignInsn != arg.getParentInsn()
&& canMoveBetweenBlocks(assignInsn, assignBlock, block, argsInfo.getInsn())) {
inline(arg, assignInsn, assignBlock);
}
if (!args.isEmpty()) {
ListIterator<RegisterArg> it = args.listIterator(args.size());
while (it.hasPrevious()) {
RegisterArg arg = it.previous();
checkInline(mth, block, insnList, wrapList, argsInfo, arg);
}
}
}
Expand All @@ -109,6 +72,45 @@ && canMoveBetweenBlocks(assignInsn, assignBlock, block, argsInfo.getInsn())) {
}
}

private static void checkInline(MethodNode mth, BlockNode block, InsnList insnList,
List<WrapInfo> wrapList, ArgsInfo argsInfo, RegisterArg arg) {
SSAVar sVar = arg.getSVar();
if (sVar == null || sVar.contains(AFlag.DONT_INLINE)) {
return;
}
// allow inline only one use arg
if (sVar.getVariableUseCount() != 1) {
return;
}
InsnNode assignInsn = sVar.getAssign().getParentInsn();
if (assignInsn == null || assignInsn.contains(AFlag.DONT_INLINE)) {
return;
}
List<RegisterArg> useList = sVar.getUseList();
if (!useList.isEmpty()) {
InsnNode parentInsn = useList.get(0).getParentInsn();
if (parentInsn != null && parentInsn.contains(AFlag.DONT_GENERATE)) {
return;
}
}

int assignPos = insnList.getIndex(assignInsn);
if (assignPos != -1) {
WrapInfo wrapInfo = argsInfo.checkInline(assignPos, arg);
if (wrapInfo != null) {
wrapList.add(wrapInfo);
}
} else {
// another block
BlockNode assignBlock = BlockUtils.getBlockByInsn(mth, assignInsn);
if (assignBlock != null
&& assignInsn != arg.getParentInsn()
&& canMoveBetweenBlocks(assignInsn, assignBlock, block, argsInfo.getInsn())) {
inline(arg, assignInsn, assignBlock);
}
}
}

private static boolean inline(RegisterArg arg, InsnNode insn, BlockNode block) {
InsnNode parentInsn = arg.getParentInsn();
if (parentInsn != null && parentInsn.getType() == InsnType.RETURN) {
Expand Down
37 changes: 37 additions & 0 deletions ...e/src/test/java/jadx/tests/integration/conditions/TestTernaryOneBranchInConstructor2.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package jadx.tests.integration.conditions;

import org.junit.jupiter.api.Test;

import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.SmaliTest;

import static jadx.tests.api.utils.JadxMatchers.containsOne;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;

public class TestTernaryOneBranchInConstructor2 extends SmaliTest {

// @formatter:off
/*
public class A {
public A(String str, String str2, String str3, boolean z) {}
public A(String str, String str2, String str3, boolean z, int i, int i2) {
this(str, (i & 2) != 0 ? "" : str2, (i & 4) != 0 ? "" : str3, (i & 8) != 0 ? false : z);
}
}
*/
// @formatter:on

@Test
public void test() {
ClassNode cls = getClassNodeFromSmali();
String code = cls.getCode().toString();

assertThat(code, containsOne("this(str, (i & 2) != 0 ? \"\" : str2,"
+ " (i & 4) != 0 ? \"\" : str3,"
+ " (i & 8) != 0 ? false : z);"));
assertThat(code, not(containsString("//")));
}
}
42 changes: 42 additions & 0 deletions jadx-core/src/test/smali/conditions/TestTernaryOneBranchInConstructor2.smali
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
.class public final Lconditions/TestTernaryOneBranchInConstructor2;
.super Ljava/lang/Object;


.method public constructor <init>(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V
.locals 1

invoke-direct {p0}, Ljava/lang/Object;-><init>()V

return-void
.end method

.method public synthetic constructor <init>(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZII)V
.locals 1

and-int/lit8 p6, p5, 0x2

const-string v0, ""

if-eqz p6, :cond_0

move-object p2, v0

:cond_0
and-int/lit8 p6, p5, 0x4

if-eqz p6, :cond_1

move-object p3, v0

:cond_1
and-int/lit8 p5, p5, 0x8

if-eqz p5, :cond_2

const/4 p4, 0x0

:cond_2
invoke-direct {p0, p1, p2, p3, p4}, Lconditions/TestTernaryOneBranchInConstructor2;-><init>(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V

return-void
.end method

0 comments on commit 424a8ff

Please sign in to comment.