Skip to content

Commit

Permalink
fix: forbid to change types for methods arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Feb 11, 2019
1 parent 89563b6 commit b689efc
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

public class CodeVar {
private String name;
private ArgType type;
private ArgType type; // nullable before type inference, set only for immutable types
private List<SSAVar> ssaVars = new ArrayList<>(3);

private boolean isFinal;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ public CodeVar getCodeVar() {

public void setCodeVar(@NotNull CodeVar codeVar) {
this.codeVar = codeVar;
if (codeVar.getType() != null && !typeInfo.getType().equals(codeVar.getType())) {
throw new JadxRuntimeException("Unmached types for SSA and Code variables: " + this + " and " + codeVar);
}
codeVar.addSsaVar(this);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
package jadx.core.dex.visitors;

import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.instructions.PhiInsn;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.CodeVar;
import jadx.core.dex.instructions.args.RegisterArg;
import jadx.core.dex.instructions.args.SSAVar;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.visitors.ssa.SSATransform;
import jadx.core.utils.exceptions.JadxException;
import jadx.core.utils.exceptions.JadxRuntimeException;

@JadxVisitor(
name = "InitCodeVariables",
Expand Down Expand Up @@ -41,7 +45,6 @@ public static void initCodeVar(SSAVar ssaVar) {
return;
}
CodeVar codeVar = new CodeVar();
codeVar.setType(ssaVar.getTypeInfo().getType());
RegisterArg assignArg = ssaVar.getAssign();
if (assignArg.contains(AFlag.THIS)) {
codeVar.setName(RegisterArg.THIS_ARG_NAME);
Expand All @@ -55,17 +58,36 @@ public static void initCodeVar(SSAVar ssaVar) {
}

private static void setCodeVar(SSAVar ssaVar, CodeVar codeVar) {
ssaVar.setCodeVar(codeVar);
PhiInsn usedInPhi = ssaVar.getUsedInPhi();
if (usedInPhi != null) {
Set<SSAVar> vars = new HashSet<>();
Set<SSAVar> vars = new LinkedHashSet<>();
vars.add(ssaVar);
collectConnectedVars(usedInPhi, vars);
setCodeVarType(codeVar, vars);
vars.forEach(var -> {
if (var.isCodeVarSet()) {
codeVar.mergeFlagsFrom(var.getCodeVar());
}
var.setCodeVar(codeVar);
});
} else {
ssaVar.setCodeVar(codeVar);
}
}

private static void setCodeVarType(CodeVar codeVar, Set<SSAVar> vars) {
if (vars.size() > 1) {
List<ArgType> imTypes = vars.stream()
.filter(var -> var.contains(AFlag.METHOD_ARGUMENT))
.map(var -> var.getTypeInfo().getType())
.distinct()
.collect(Collectors.toList());
int imCount = imTypes.size();
if (imCount == 1) {
codeVar.setType(imTypes.get(0));
} else if (imCount > 1) {
throw new JadxRuntimeException("Several immutable types in one variable: " + imTypes + ", vars: " + vars);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,51 +102,58 @@ private void runMultiVariableSearch(MethodNode mth) {

private boolean setBestType(SSAVar ssaVar) {
try {
RegisterArg assignArg = ssaVar.getAssign();
if (!assignArg.isTypeImmutable()) {
return calculateFromBounds(ssaVar);
ArgType codeVarType = ssaVar.getCodeVar().getType();
if (codeVarType != null) {
return applyImmutableType(ssaVar, codeVarType);
}
ArgType initType = assignArg.getInitType();
TypeUpdateResult result = typeUpdate.apply(ssaVar, initType);
if (result == TypeUpdateResult.REJECT) {
if (Consts.DEBUG) {
LOG.info("Initial immutable type set rejected: {} -> {}", ssaVar, initType);
}
return false;
RegisterArg assignArg = ssaVar.getAssign();
if (assignArg.isTypeImmutable()) {
return applyImmutableType(ssaVar, assignArg.getInitType());
}
return true;
return calculateFromBounds(ssaVar);
} catch (Exception e) {
LOG.error("Failed to calculate best type for var: {}", ssaVar);
return false;
}
}

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);
}
return false;
}
return result == TypeUpdateResult.CHANGED;
}

private boolean calculateFromBounds(SSAVar ssaVar) {
TypeInfo typeInfo = ssaVar.getTypeInfo();
Set<ITypeBound> bounds = typeInfo.getBounds();
Optional<ArgType> bestTypeOpt = selectBestTypeFromBounds(bounds);
if (bestTypeOpt.isPresent()) {
ArgType candidateType = bestTypeOpt.get();
TypeUpdateResult result = typeUpdate.apply(ssaVar, candidateType);
if (result == TypeUpdateResult.REJECT) {
if (Consts.DEBUG) {
if (ssaVar.getTypeInfo().getType().equals(candidateType)) {
LOG.info("Same type rejected: {} -> {}, bounds: {}", ssaVar, candidateType, bounds);
} else if (candidateType.isTypeKnown()) {
LOG.debug("Type set rejected: {} -> {}, bounds: {}", ssaVar, candidateType, bounds);
}
if (!bestTypeOpt.isPresent()) {
if (Consts.DEBUG) {
LOG.warn("Failed to select best type from bounds, count={} : ", bounds.size());
for (ITypeBound bound : bounds) {
LOG.warn(" {}", bound);
}
return false;
}
return result == TypeUpdateResult.CHANGED;
return false;
}
if (Consts.DEBUG) {
LOG.warn("Failed to select best type from bounds, count={} : ", bounds.size());
for (ITypeBound bound : bounds) {
LOG.warn(" {}", bound);
ArgType candidateType = bestTypeOpt.get();
TypeUpdateResult result = typeUpdate.apply(ssaVar, candidateType);
if (result == TypeUpdateResult.REJECT) {
if (Consts.DEBUG) {
if (ssaVar.getTypeInfo().getType().equals(candidateType)) {
LOG.info("Same type rejected: {} -> {}, bounds: {}", ssaVar, candidateType, bounds);
} else if (candidateType.isTypeKnown()) {
LOG.debug("Type set rejected: {} -> {}, bounds: {}", ssaVar, candidateType, bounds);
}
}
return false;
}
return false;
return result == TypeUpdateResult.CHANGED;
}

private Optional<ArgType> selectBestTypeFromBounds(Set<ITypeBound> bounds) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package jadx.core.dex.visitors.typeinference;

import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Set;

import org.jetbrains.annotations.NotNull;
Expand All @@ -10,7 +10,7 @@
public class TypeInfo {
private ArgType type = ArgType.UNKNOWN;

private final Set<ITypeBound> bounds = new HashSet<>();
private final Set<ITypeBound> bounds = new LinkedHashSet<>();

@NotNull
public ArgType getType() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public TypeUpdateResult apply(SSAVar ssaVar, ArgType candidateType) {
if (candidateType == null) {
return REJECT;
}
if (!candidateType.isTypeKnown() && ssaVar.getTypeInfo().getType().isTypeKnown()) {
if (!candidateType.isTypeKnown()/* && ssaVar.getTypeInfo().getType().isTypeKnown()*/) {
return REJECT;
}

Expand Down Expand Up @@ -86,14 +86,14 @@ private TypeUpdateResult updateTypeChecked(TypeUpdateInfo updateInfo, InsnArg ar
return SAME;
}
TypeCompareEnum compareResult = comparator.compareTypes(candidateType, currentType);
if (compareResult == TypeCompareEnum.CONFLICT) {
if (Consts.DEBUG) {
LOG.debug("Type rejected for {} due to conflict: candidate={}, current={}", arg, candidateType, currentType);
}
return REJECT;
}
if (arg.isTypeImmutable() && currentType != ArgType.UNKNOWN) {
// don't changed type, conflict already rejected
// don't changed type
if (compareResult == TypeCompareEnum.CONFLICT) {
if (Consts.DEBUG) {
LOG.debug("Type rejected for {} due to conflict: candidate={}, current={}", arg, candidateType, currentType);
}
return REJECT;
}
return SAME;
}
if (compareResult.isWider()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ public void testNoDebug() {
String code = cls.getCode().toString();

assertThat(code, containsLines(2,
"FileInputStream fileInputStream = null;",
"InputStream inputStream = null;",
"try {",
indent() + "call();",
indent() + "fileInputStream = new FileInputStream(\"1.txt\");",
indent() + "inputStream = new FileInputStream(\"1.txt\");",
"} finally {",
indent() + "if (fileInputStream != null) {",
indent() + indent() + "fileInputStream.close();",
indent() + "if (inputStream != null) {",
indent() + indent() + "inputStream.close();",
indent() + "}",
"}"
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static <T> j a(i<? super T> iVar, c<T> cVar) {
@Test
public void test() {
disableCompilation();
ClassNode cls = getClassNodeFromSmaliWithPath("variables", "TestVariablesGeneric");
ClassNode cls = getClassNodeFromSmaliWithPkg("variables", "TestVariablesGeneric");
String code = cls.getCode().toString();

assertThat(code, not(containsString("iVar2")));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.class public LTestVariablesGeneric;
.class public Lvariables/TestVariablesGeneric;
.super Ljava/lang/Object;
.source "SourceFile"

Expand Down

0 comments on commit b689efc

Please sign in to comment.