Merge "Take into account catch handlers when adding item-based string."
diff --git a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
index 5e9e0f2..997a8c9 100644
--- a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
@@ -12,7 +12,7 @@
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import java.util.List;
-abstract class FieldInstruction extends Instruction {
+public abstract class FieldInstruction extends Instruction {
protected final MemberType type;
protected final DexField field;
@@ -41,6 +41,16 @@
return field;
}
+ @Override
+ public boolean isFieldInstruction() {
+ return true;
+ }
+
+ @Override
+ public FieldInstruction asFieldInstruction() {
+ return this;
+ }
+
/**
* Returns the target of this field instruction, if such target is known, or null.
* <p>
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index 68253c6..cdca04e 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -582,6 +582,14 @@
return null;
}
+ public boolean isFieldInstruction() {
+ return false;
+ }
+
+ public FieldInstruction asFieldInstruction() {
+ return null;
+ }
+
public boolean isGoto() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
index c90f1cf..f6e087b 100644
--- a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
+++ b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
@@ -17,9 +17,11 @@
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DexValue.DexValueString;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.ConstString;
+import com.android.tools.r8.ir.code.FieldInstruction;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
@@ -32,6 +34,7 @@
import com.google.common.collect.Streams;
import java.util.Arrays;
import java.util.List;
+import java.util.ListIterator;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
@@ -69,130 +72,192 @@
}
public void decoupleIdentifierNameStringsInMethod(DexEncodedMethod encodedMethod, IRCode code) {
- for (BasicBlock block : code.blocks) {
+ ListIterator<BasicBlock> blocks = code.listIterator();
+ while (blocks.hasNext()) {
+ BasicBlock block = blocks.next();
InstructionListIterator iterator = block.listIterator();
while (iterator.hasNext()) {
Instruction instruction = iterator.next();
- if (instruction.isStaticPut()) {
- StaticPut staticPut = instruction.asStaticPut();
- DexField field = staticPut.getField();
- if (identifierNameStrings.contains(field)) {
- Value in = staticPut.inValue();
- Value newIn = decoupleIdentifierIfNecessary(code, iterator, staticPut, in);
- if (newIn != in) {
- iterator.replaceCurrentInstruction(
- new StaticPut(staticPut.getType(), newIn, field));
- encodedMethod.markUseIdentifierNameString();
- }
+ // v_n <- "x.y.z" // in.definition
+ // ...
+ // ... <- ... v_n ..
+ // ...
+ // this.fld <- v_n // fieldPut
+ //
+ // ~>
+ //
+ // ...
+ // v_n' <- DexItemBasedString("Lx/y/z;") // decoupled
+ // this.fld <- v_n' // fieldPut
+ if (instruction.isStaticPut() || instruction.isInstancePut()) {
+ FieldInstruction fieldPut = instruction.asFieldInstruction();
+ DexField field = fieldPut.getField();
+ if (!identifierNameStrings.contains(field)) {
+ continue;
}
- } else if (instruction.isInstancePut()) {
- InstancePut instancePut = instruction.asInstancePut();
- DexField field = instancePut.getField();
- if (identifierNameStrings.contains(field)) {
- Value in = instancePut.value();
- Value newIn = decoupleIdentifierIfNecessary(code, iterator, instancePut, in);
- if (newIn != in) {
- iterator.replaceCurrentInstruction(
- new InstancePut(instancePut.getType(), field, instancePut.object(), newIn));
- encodedMethod.markUseIdentifierNameString();
- }
+ Value in = instruction.isStaticPut()
+ ? instruction.asStaticPut().inValue()
+ : instruction.asInstancePut().value();
+ if (!in.isConstString()) {
+ continue;
}
+ DexString original = in.getConstInstruction().asConstString().getValue();
+ DexItemBasedString itemBasedString = inferMemberOrTypeFromNameString(appInfo, original);
+ if (itemBasedString == null) {
+ continue;
+ }
+ // Move the cursor back to $fieldPut
+ assert iterator.peekPrevious() == fieldPut;
+ iterator.previous();
+ // Prepare $decoupled just before $fieldPut
+ Value newIn = code.createValue(in.outType(), in.getLocalInfo());
+ ConstString decoupled = new ConstString(newIn, itemBasedString);
+ decoupled.setPosition(fieldPut.getPosition());
+ // If the current block has catch handler, split into two blocks.
+ // Because const-string we're about to add is also a throwing instr, we need to split
+ // before adding it.
+ BasicBlock blockWithFieldInstruction =
+ block.hasCatchHandlers() ? iterator.split(code, blocks) : block;
+ if (blockWithFieldInstruction != block) {
+ // If we split, add const-string at the end of the currently visiting block.
+ iterator = block.listIterator(block.getInstructions().size());
+ iterator.previous();
+ iterator.add(decoupled);
+ // Restore the cursor and block.
+ iterator = blockWithFieldInstruction.listIterator();
+ assert iterator.peekNext() == fieldPut;
+ iterator.next();
+ block = blockWithFieldInstruction;
+ } else {
+ // Otherwise, just add it to the current block at the position of the iterator.
+ iterator.add(decoupled);
+ // Restore the cursor.
+ assert iterator.peekNext() == fieldPut;
+ iterator.next();
+ }
+ if (instruction.isStaticPut()) {
+ StaticPut staticPut = instruction.asStaticPut();
+ iterator.replaceCurrentInstruction(
+ new StaticPut(staticPut.getType(), newIn, field));
+ } else {
+ assert instruction.isInstancePut();
+ InstancePut instancePut = instruction.asInstancePut();
+ iterator.replaceCurrentInstruction(
+ new InstancePut(instancePut.getType(), field, instancePut.object(), newIn));
+ }
+ encodedMethod.markUseIdentifierNameString();
} else if (instruction.isInvokeMethod()) {
InvokeMethod invoke = instruction.asInvokeMethod();
DexMethod invokedMethod = invoke.getInvokedMethod();
- if (identifierNameStrings.contains(invokedMethod)) {
- List<Value> ins = invoke.arguments();
- Value[] changes = new Value [ins.size()];
- if (isReflectionMethod(dexItemFactory, invokedMethod)) {
- decoupleReflectiveMemberIdentifier(code, iterator, invoke, changes);
+ if (!identifierNameStrings.contains(invokedMethod)) {
+ continue;
+ }
+ List<Value> ins = invoke.arguments();
+ Value[] changes = new Value [ins.size()];
+ if (isReflectionMethod(dexItemFactory, invokedMethod)) {
+ DexItemBasedString itemBasedString = identifyIdentiferNameString(appInfo, invoke);
+ if (itemBasedString == null) {
+ continue;
+ }
+ DexType returnType = invoke.getReturnType();
+ boolean isClassForName =
+ returnType.descriptor == dexItemFactory.classDescriptor;
+ boolean isReferenceFieldUpdater =
+ returnType.descriptor == dexItemFactory.referenceFieldUpdaterDescriptor;
+ int positionOfIdentifier = isClassForName ? 0 : (isReferenceFieldUpdater ? 2 : 1);
+ Value in = invoke.arguments().get(positionOfIdentifier);
+ // Move the cursor back to $invoke
+ assert iterator.peekPrevious() == invoke;
+ iterator.previous();
+ // Prepare $decoupled just before $invoke
+ Value newIn = code.createValue(in.outType(), in.getLocalInfo());
+ ConstString decoupled = new ConstString(newIn, itemBasedString);
+ decoupled.setPosition(invoke.getPosition());
+ changes[positionOfIdentifier] = newIn;
+ // If the current block has catch handler, split into two blocks.
+ // Because const-string we're about to add is also a throwing instr, we need to split
+ // before adding it.
+ BasicBlock blockWithInvoke =
+ block.hasCatchHandlers() ? iterator.split(code, blocks) : block;
+ if (blockWithInvoke != block) {
+ // If we split, add const-string at the end of the currently visiting block.
+ iterator = block.listIterator(block.getInstructions().size());
+ iterator.previous();
+ iterator.add(decoupled);
+ // Restore the cursor and block.
+ iterator = blockWithInvoke.listIterator();
+ assert iterator.peekNext() == invoke;
+ iterator.next();
+ block = blockWithInvoke;
} else {
- for (int i = 0; i < ins.size(); i++) {
- Value in = ins.get(i);
- Value newIn = decoupleIdentifierIfNecessary(code, iterator, invoke, in);
- if (newIn != in) {
- changes[i] = newIn;
- }
+ // Otherwise, just add it to the current block at the position of the iterator.
+ iterator.add(decoupled);
+ // Restore the cursor.
+ assert iterator.peekNext() == invoke;
+ iterator.next();
+ }
+ } else {
+ // For general invoke. Multiple arguments can be string literals to be renamed.
+ for (int i = 0; i < ins.size(); i++) {
+ Value in = ins.get(i);
+ if (!in.isConstString()) {
+ continue;
+ }
+ DexString original = in.getConstInstruction().asConstString().getValue();
+ DexItemBasedString itemBasedString =
+ inferMemberOrTypeFromNameString(appInfo, original);
+ if (itemBasedString == null) {
+ continue;
+ }
+ // Move the cursor back to $invoke
+ assert iterator.peekPrevious() == invoke;
+ iterator.previous();
+ // Prepare $decoupled just before $invoke
+ Value newIn = code.createValue(in.outType(), in.getLocalInfo());
+ ConstString decoupled = new ConstString(newIn, itemBasedString);
+ decoupled.setPosition(invoke.getPosition());
+ changes[i] = newIn;
+ // If the current block has catch handler, split into two blocks.
+ // Because const-string we're about to add is also a throwing instr, we need to split
+ // before adding it.
+ BasicBlock blockWithInvoke =
+ block.hasCatchHandlers() ? iterator.split(code, blocks) : block;
+ if (blockWithInvoke != block) {
+ // If we split, add const-string at the end of the currently visiting block.
+ iterator = block.listIterator(block.getInstructions().size());
+ iterator.previous();
+ iterator.add(decoupled);
+ // Restore the cursor and block.
+ iterator = blockWithInvoke.listIterator();
+ assert iterator.peekNext() == invoke;
+ iterator.next();
+ block = blockWithInvoke;
+ } else {
+ // Otherwise, just add it to the current block at the position of the iterator.
+ iterator.add(decoupled);
+ // Restore the cursor.
+ assert iterator.peekNext() == invoke;
+ iterator.next();
}
}
- if (!Arrays.stream(changes).allMatch(Objects::isNull)) {
- List<Value> newIns =
- Streams.mapWithIndex(
- ins.stream(),
- (in, index) -> changes[(int) index] != null ? changes[(int) index] : in)
- .collect(Collectors.toList());
- iterator.replaceCurrentInstruction(
- Invoke.create(
- invoke.getType(),
- invokedMethod,
- invokedMethod.proto,
- invoke.outValue(),
- newIns));
- encodedMethod.markUseIdentifierNameString();
- }
+ }
+ if (!Arrays.stream(changes).allMatch(Objects::isNull)) {
+ List<Value> newIns =
+ Streams.mapWithIndex(
+ ins.stream(),
+ (in, index) -> changes[(int) index] != null ? changes[(int) index] : in)
+ .collect(Collectors.toList());
+ iterator.replaceCurrentInstruction(
+ Invoke.create(
+ invoke.getType(),
+ invokedMethod,
+ invokedMethod.proto,
+ invoke.outValue(),
+ newIns));
+ encodedMethod.markUseIdentifierNameString();
}
}
}
}
}
-
- private Value decoupleIdentifierIfNecessary(
- IRCode code, InstructionListIterator iterator, Instruction base, Value in) {
- if (!in.isConstString()) {
- return in;
- }
- DexString original = in.getConstInstruction().asConstString().getValue();
- DexItemBasedString itemBasedString = inferMemberOrTypeFromNameString(appInfo, original);
- if (itemBasedString == null) {
- return in;
- }
- return insertItemBasedString(code, iterator, base, in, itemBasedString);
- }
-
- private void decoupleReflectiveMemberIdentifier(
- IRCode code, InstructionListIterator iterator, InvokeMethod invoke, Value[] changes) {
- DexItemBasedString itemBasedString = identifyIdentiferNameString(appInfo, invoke);
- if (itemBasedString == null) {
- return;
- }
- boolean isClassForName = invoke.getReturnType().descriptor == dexItemFactory.classDescriptor;
- boolean isReferenceFieldUpdater =
- invoke.getReturnType().descriptor == dexItemFactory.referenceFieldUpdaterDescriptor;
- int positionOfIdentifier =
- isClassForName ? 0 : (isReferenceFieldUpdater ? 2 : 1);
- Value in = invoke.arguments().get(positionOfIdentifier);
- Value newIn = insertItemBasedString(code, iterator, invoke, in, itemBasedString);
- if (newIn != in) {
- changes[positionOfIdentifier] = newIn;
- }
- }
-
- private Value insertItemBasedString(
- IRCode code,
- InstructionListIterator iterator,
- Instruction base,
- Value in,
- DexItemBasedString itemBasedString) {
- // v_n <- "x.y.z" // in.definition
- // ...
- // ... <- ... v_n ..
- // ...
- // this.fld <- v_n // base
- //
- // ~>
- //
- // ...
- // v_n' <- DexItemBasedString("Lx/y/z;") // decoupled
- // this.fld <- v_n' // base
- //
- // 1) Move the cursor back to $base
- iterator.previous();
- // 2) Add $decoupled just before $base
- Value newIn = code.createValue(in.outType(), in.getLocalInfo());
- ConstString decoupled = new ConstString(newIn, itemBasedString);
- decoupled.setPosition(base.getPosition());
- iterator.add(decoupled);
- // 3) Restore the cursor
- iterator.next();
- return newIn;
- }
}
diff --git a/src/test/examples/identifiernamestring/Main.java b/src/test/examples/identifiernamestring/Main.java
index e29f4d1..6f502ea 100644
--- a/src/test/examples/identifiernamestring/Main.java
+++ b/src/test/examples/identifiernamestring/Main.java
@@ -10,8 +10,12 @@
public static void main(String[] args) throws Exception {
A ax = new A();
assert ax.boo.equals(A.TYPE_B);
- // Should be renamed
- ax.bar("identifiernamestring.B");
+ try {
+ // Should be renamed
+ ax.bar("identifiernamestring.B");
+ } catch (NullPointerException e) {
+ System.err.println(e.getMessage());
+ }
Class a = Class.forName(A.TYPE_A);
Class bByA = Class.forName(B.TYPO_A);
