Skip to content

Commit

Permalink
Propagate sideways conversions after an instance-of + if-eq/if-eqz
Browse files Browse the repository at this point in the history
Previously, we only propagated the conversion if it was a narrowing
conversion, to avoid problems that can occur with member access with
widening conversions.

However, it should be safe to do the conversion for a "sideways"
conversion - one that is neither widening or narrowing.

This can happen if we don't yet have full knowledge of the register types,
or, less likely, if the "true" branch is impossible to reach.

In the first case, we should get better type info as we continue to analyze
the method, and we'll revisit the conversion once we have better type info.

Or, if it really is an impossible conversion, we still want to propagate
the type from the instance-of to the true branch.
  • Loading branch information
JesusFreke committed Jan 8, 2017
1 parent adb1235 commit 1a83d5a
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,8 @@ public boolean setsRegister(int registerNumber) {
if (previousInstruction != null &&
previousInstruction.instruction.getOpcode() == Opcode.INSTANCE_OF &&
registerNumber == ((Instruction22c)previousInstruction.instruction).getRegisterB() &&
MethodAnalyzer.canNarrowAfterInstanceOf(previousInstruction, this, methodAnalyzer.getClassPath())) {
MethodAnalyzer.canPropogateTypeAfterInstanceOf(
previousInstruction, this, methodAnalyzer.getClassPath())) {
return true;
}
}
Expand Down
22 changes: 14 additions & 8 deletions dexlib2/src/main/java/org/jf/dexlib2/analysis/MethodAnalyzer.java
Original file line number Diff line number Diff line change
Expand Up @@ -1167,20 +1167,26 @@ private void analyzeCheckCast(@Nonnull AnalyzedInstruction analyzedInstruction)
setDestinationRegisterTypeAndPropagateChanges(analyzedInstruction, castRegisterType);
}

private static boolean isNarrowingConversion(RegisterType originalType, RegisterType newType) {
private static boolean isNotWideningConversion(RegisterType originalType, RegisterType newType) {
if (originalType.type == null || newType.type == null) {
return false;
return true;
}
if (originalType.type.isInterface()) {
return newType.type.implementsInterface(originalType.type.getType());
} else {
TypeProto commonSuperclass = newType.type.getCommonSuperclass(originalType.type);
return commonSuperclass.getType().equals(originalType.type.getType());
if (commonSuperclass.getType().equals(originalType.type.getType())) {
return true;
}
if (commonSuperclass.getType().equals(newType.type.getType())) {
return false;
}
}
return true;
}

static boolean canNarrowAfterInstanceOf(AnalyzedInstruction analyzedInstanceOfInstruction,
AnalyzedInstruction analyzedIfInstruction, ClassPath classPath) {
static boolean canPropogateTypeAfterInstanceOf(AnalyzedInstruction analyzedInstanceOfInstruction,
AnalyzedInstruction analyzedIfInstruction, ClassPath classPath) {
if (!classPath.isArt()) {
return false;
}
Expand All @@ -1197,7 +1203,7 @@ static boolean canNarrowAfterInstanceOf(AnalyzedInstruction analyzedInstanceOfIn

RegisterType originalType = analyzedIfInstruction.getPreInstructionRegisterType(objectRegister);

return isNarrowingConversion(originalType, registerType);
return isNotWideningConversion(originalType, registerType);
}
} catch (UnresolvedClassException ex) {
return false;
Expand All @@ -1220,7 +1226,7 @@ private void analyzeIfEqzNez(@Nonnull AnalyzedInstruction analyzedInstruction) {
AnalyzedInstruction prevAnalyzedInstruction = analyzedInstruction.getPredecessors().first();
if (prevAnalyzedInstruction.instruction.getOpcode() == Opcode.INSTANCE_OF) {
Instruction22c instanceOfInstruction = (Instruction22c)prevAnalyzedInstruction.instruction;
if (canNarrowAfterInstanceOf(prevAnalyzedInstruction, analyzedInstruction, classPath)) {
if (canPropogateTypeAfterInstanceOf(prevAnalyzedInstruction, analyzedInstruction, classPath)) {
List<Integer> narrowingRegisters = Lists.newArrayList();

RegisterType newType = RegisterType.getRegisterType(classPath,
Expand Down Expand Up @@ -1252,7 +1258,7 @@ private void analyzeIfEqzNez(@Nonnull AnalyzedInstruction analyzedInstruction) {
additionalNarrowingRegister = -1;
break;
}
if (isNarrowingConversion(originalType, newType)) {
if (isNotWideningConversion(originalType, newType)) {
if (additionalNarrowingRegister != -1) {
if (additionalNarrowingRegister != moveInstruction.getRegisterB()) {
additionalNarrowingRegister = -1;
Expand Down

0 comments on commit 1a83d5a

Please sign in to comment.