Skip to content

Commit

Permalink
fix: don't use static vars of mutable LiteralArg class (#1005)
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Nov 1, 2020
1 parent 2b7d7ce commit 8ca3cd3
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 36 deletions.
2 changes: 1 addition & 1 deletion jadx-core/src/main/java/jadx/core/codegen/InsnGen.java
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ private void makeTernary(TernaryInsn insn, CodeWriter code, Set<Flags> state) th
InsnArg first = insn.getArg(0);
InsnArg second = insn.getArg(1);
ConditionGen condGen = new ConditionGen(this);
if (first.equals(LiteralArg.TRUE) && second.equals(LiteralArg.FALSE)) {
if (first.isTrue() && second.isFalse()) {
condGen.add(code, insn.getCondition());
} else {
condGen.wrap(code, insn.getCondition());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package jadx.core.dex.instructions.args;

import java.util.Objects;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
Expand Down Expand Up @@ -56,7 +58,7 @@ public static RegisterArg reg(int regNum, ArgType type, boolean typeImmutable) {
}

public static LiteralArg lit(long literal, ArgType type) {
return new LiteralArg(literal, type);
return LiteralArg.makeWithFixedType(literal, type);
}

public static LiteralArg lit(InsnData insn, ArgType type) {
Expand Down Expand Up @@ -210,6 +212,22 @@ public boolean isZeroLiteral() {
return isLiteral() && (((LiteralArg) this)).getLiteral() == 0;
}

public boolean isFalse() {
if (isLiteral()) {
LiteralArg litArg = (LiteralArg) this;
return litArg.getLiteral() == 0 && Objects.equals(litArg.getType(), ArgType.BOOLEAN);
}
return false;
}

public boolean isTrue() {
if (isLiteral()) {
LiteralArg litArg = (LiteralArg) this;
return litArg.getLiteral() == 1 && Objects.equals(litArg.getType(), ArgType.BOOLEAN);
}
return false;
}

public boolean isThis() {
return contains(AFlag.THIS);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,37 @@

public final class LiteralArg extends InsnArg {

public static final LiteralArg TRUE = new LiteralArg(1, ArgType.BOOLEAN);
public static final LiteralArg FALSE = new LiteralArg(0, ArgType.BOOLEAN);
public static LiteralArg make(long value, ArgType type) {
return new LiteralArg(value, type);
}

public static LiteralArg makeWithFixedType(long value, ArgType type) {
return new LiteralArg(value, fixLiteralType(value, type));
}

private static ArgType fixLiteralType(long value, ArgType type) {
if (value == 0 || type.isTypeKnown() || type.contains(PrimitiveType.LONG) || type.contains(PrimitiveType.DOUBLE)) {
return type;
}
if (value == 1) {
return ArgType.NARROW_NUMBERS;
}
return ArgType.NARROW_NUMBERS_NO_BOOL;
}

public static LiteralArg litFalse() {
return new LiteralArg(0, ArgType.BOOLEAN);
}

public static LiteralArg litTrue() {
return new LiteralArg(1, ArgType.BOOLEAN);
}

private final long literal;

public LiteralArg(long value, ArgType type) {
if (value != 0) {
if (type.isObject()) {
throw new JadxRuntimeException("Wrong literal type: " + type + " for value: " + value);
} else if (!type.isTypeKnown()
&& !type.contains(PrimitiveType.LONG)
&& !type.contains(PrimitiveType.DOUBLE)) {
if (value != 1) {
type = ArgType.NARROW_NUMBERS_NO_BOOL;
} else {
type = ArgType.NARROW_NUMBERS;
}
}
private LiteralArg(long value, ArgType type) {
if (value != 0 && type.isObject()) {
throw new JadxRuntimeException("Wrong literal type: " + type + " for value: " + value);
}
this.literal = value;
this.type = type;
Expand All @@ -33,6 +46,11 @@ public long getLiteral() {
return literal;
}

@Override
public void setType(ArgType type) {
super.setType(type);
}

@Override
public boolean isLiteral() {
return true;
Expand All @@ -49,9 +67,7 @@ public boolean isInteger() {

@Override
public InsnArg duplicate() {
LiteralArg copy = new LiteralArg(literal, getType());
copy.type = type;
return copyCommonParams(copy);
return copyCommonParams(new LiteralArg(literal, type));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.args.InsnArg;
import jadx.core.dex.instructions.args.LiteralArg;
import jadx.core.dex.instructions.args.RegisterArg;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.regions.conditions.IfCondition;
Expand All @@ -18,7 +17,7 @@ public TernaryInsn(IfCondition condition, RegisterArg result, InsnArg th, InsnAr
this();
setResult(result);

if (th.equals(LiteralArg.FALSE) && els.equals(LiteralArg.TRUE)) {
if (th.isFalse() && els.isTrue()) {
// inverted
this.condition = IfCondition.invert(condition);
addArg(els);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ public Compare invert() {
* Change 'a != false' to 'a == true'
*/
public void normalize() {
if (getOp() == IfOp.NE && getB().isLiteral() && getB().equals(LiteralArg.FALSE)) {
insn.changeCondition(IfOp.EQ, getA(), LiteralArg.TRUE);
if (getOp() == IfOp.NE && getB().isFalse()) {
insn.changeCondition(IfOp.EQ, getA(), LiteralArg.litTrue());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public static IfCondition simplify(IfCondition cond) {
if (i != null) {
return i;
}
if (c.getOp() == IfOp.EQ && c.getB().isLiteral() && c.getB().equals(LiteralArg.FALSE)) {
if (c.getOp() == IfOp.EQ && c.getB().isFalse()) {
cond = not(new IfCondition(c.invert()));
} else {
c.normalize();
Expand Down Expand Up @@ -234,8 +234,8 @@ private static IfCondition simplifyCmpOp(Compare c) {
Mode mode = isTrue && arithOp == ArithOp.OR
|| !isTrue && arithOp == ArithOp.AND ? Mode.OR : Mode.AND;

IfNode if1 = new IfNode(op, -1, wrapInsn.getArg(0), LiteralArg.FALSE);
IfNode if2 = new IfNode(op, -1, wrapInsn.getArg(1), LiteralArg.FALSE);
IfNode if1 = new IfNode(op, -1, wrapInsn.getArg(0), LiteralArg.litFalse());
IfNode if2 = new IfNode(op, -1, wrapInsn.getArg(1), LiteralArg.litFalse());
return new IfCondition(mode,
Arrays.asList(new IfCondition(new Compare(if1)),
new IfCondition(new Compare(if2))));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,16 +253,16 @@ private static void fixPrimitiveCast(MethodNode mth, BlockNode block, int i, Ins
}

public static TernaryInsn makeBooleanConvertInsn(RegisterArg result, InsnArg castArg, ArgType type) {
InsnArg zero = new LiteralArg(0, type);
InsnArg zero = LiteralArg.make(0, type);
long litVal = 1;
if (type == ArgType.DOUBLE) {
litVal = DOUBLE_TO_BITS;
} else if (type == ArgType.FLOAT) {
litVal = FLOAT_TO_BITS;
}
InsnArg one = new LiteralArg(litVal, type);
InsnArg one = LiteralArg.make(litVal, type);

IfNode ifNode = new IfNode(IfOp.EQ, -1, castArg, LiteralArg.TRUE);
IfNode ifNode = new IfNode(IfOp.EQ, -1, castArg, LiteralArg.litTrue());
IfCondition condition = IfCondition.fromIfNode(ifNode);
return new TernaryInsn(condition, result, one, zero);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public static Object convertToConstValue(RootNode root, EncodedValue encodedValu
case ENCODED_NULL:
return InsnArg.lit(0, ArgType.OBJECT);
case ENCODED_BOOLEAN:
return Boolean.TRUE.equals(value) ? LiteralArg.TRUE : LiteralArg.FALSE;
return Boolean.TRUE.equals(value) ? LiteralArg.litTrue() : LiteralArg.litFalse();
case ENCODED_BYTE:
return InsnArg.lit((Byte) value, ArgType.BYTE);
case ENCODED_SHORT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import jadx.core.dex.regions.conditions.Compare;
import jadx.core.dex.regions.conditions.IfCondition;

import static jadx.core.dex.instructions.args.LiteralArg.TRUE;
import static jadx.core.dex.regions.conditions.IfCondition.Mode;
import static jadx.core.dex.regions.conditions.IfCondition.Mode.AND;
import static jadx.core.dex.regions.conditions.IfCondition.Mode.COMPARE;
Expand All @@ -29,11 +28,11 @@ private static IfCondition makeCondition(IfOp op, InsnArg a, InsnArg b) {
}

private static IfCondition makeSimpleCondition() {
return makeCondition(IfOp.EQ, mockArg(), LiteralArg.TRUE);
return makeCondition(IfOp.EQ, mockArg(), LiteralArg.litTrue());
}

private static IfCondition makeNegCondition() {
return makeCondition(IfOp.NE, mockArg(), LiteralArg.TRUE);
return makeCondition(IfOp.NE, mockArg(), LiteralArg.litTrue());
}

private static InsnArg mockArg() {
Expand All @@ -44,13 +43,13 @@ private static InsnArg mockArg() {
public void testNormalize() {
// 'a != false' => 'a == true'
InsnArg a = mockArg();
IfCondition c = makeCondition(IfOp.NE, a, LiteralArg.FALSE);
IfCondition c = makeCondition(IfOp.NE, a, LiteralArg.litFalse());
IfCondition simp = simplify(c);

assertThat(simp.getMode(), is(COMPARE));
Compare compare = simp.getCompare();
assertThat(compare.getA(), is(a));
assertThat(compare.getB(), is(TRUE));
assertThat(compare.getB(), is(LiteralArg.litTrue()));
}

@Test
Expand Down

0 comments on commit 8ca3cd3

Please sign in to comment.