Transform incorrect invokevirtual ops to invokedirect.
bug: 12370565
Change-Id: I75de311912d3e620f95fbac712e799e64622e4ab
diff --git a/dx/src/com/android/dx/cf/code/Ropper.java b/dx/src/com/android/dx/cf/code/Ropper.java
index c318b4c..6a20f27 100644
--- a/dx/src/com/android/dx/cf/code/Ropper.java
+++ b/dx/src/com/android/dx/cf/code/Ropper.java
@@ -16,6 +16,7 @@
package com.android.dx.cf.code;
+import com.android.dx.cf.iface.MethodList;
import com.android.dx.rop.code.AccessFlags;
import com.android.dx.rop.code.BasicBlock;
import com.android.dx.rop.code.BasicBlockList;
@@ -257,12 +258,14 @@
*
* @param method {@code non-null;} method to convert
* @param advice {@code non-null;} translation advice to use
+ * @param methods {@code non-null;} list of methods defined by the class
+ * that defines {@code method}.
* @return {@code non-null;} the converted instance
*/
public static RopMethod convert(ConcreteMethod method,
- TranslationAdvice advice) {
+ TranslationAdvice advice, MethodList methods) {
try {
- Ropper r = new Ropper(method, advice);
+ Ropper r = new Ropper(method, advice, methods);
r.doit();
return r.getRopMethod();
} catch (SimException ex) {
@@ -278,8 +281,10 @@
*
* @param method {@code non-null;} method to convert
* @param advice {@code non-null;} translation advice to use
+ * @param methods {@code non-null;} list of methods defined by the class
+ * that defines {@code method}.
*/
- private Ropper(ConcreteMethod method, TranslationAdvice advice) {
+ private Ropper(ConcreteMethod method, TranslationAdvice advice, MethodList methods) {
if (method == null) {
throw new NullPointerException("method == null");
}
@@ -292,7 +297,7 @@
this.blocks = BasicBlocker.identifyBlocks(method);
this.maxLabel = blocks.getMaxLabel();
this.maxLocals = method.getMaxLocals();
- this.machine = new RopperMachine(this, method, advice);
+ this.machine = new RopperMachine(this, method, advice, methods);
this.sim = new Simulator(machine, method);
this.startFrames = new Frame[maxLabel];
this.subroutines = new Subroutine[maxLabel];
diff --git a/dx/src/com/android/dx/cf/code/RopperMachine.java b/dx/src/com/android/dx/cf/code/RopperMachine.java
index f1b8db4..ea7d6ff 100644
--- a/dx/src/com/android/dx/cf/code/RopperMachine.java
+++ b/dx/src/com/android/dx/cf/code/RopperMachine.java
@@ -16,6 +16,9 @@
package com.android.dx.cf.code;
+import com.android.dx.cf.iface.Method;
+import com.android.dx.cf.iface.MethodList;
+import com.android.dx.rop.code.AccessFlags;
import com.android.dx.rop.code.FillArrayDataInsn;
import com.android.dx.rop.code.Insn;
import com.android.dx.rop.code.PlainCstInsn;
@@ -67,6 +70,9 @@
/** {@code non-null;} method being converted */
private final ConcreteMethod method;
+ /** {@code non-null:} list of methods from the class whose method is being converted */
+ private final MethodList methods;
+
/** {@code non-null;} translation advice */
private final TranslationAdvice advice;
@@ -122,11 +128,17 @@
* @param ropper {@code non-null;} ropper controlling this instance
* @param method {@code non-null;} method being converted
* @param advice {@code non-null;} translation advice to use
+ * @param methods {@code non-null;} list of methods defined by the class
+ * that defines {@code method}.
*/
public RopperMachine(Ropper ropper, ConcreteMethod method,
- TranslationAdvice advice) {
+ TranslationAdvice advice, MethodList methods) {
super(method.getEffectiveDescriptor());
+ if (methods == null) {
+ throw new NullPointerException("methods == null");
+ }
+
if (ropper == null) {
throw new NullPointerException("ropper == null");
}
@@ -137,6 +149,7 @@
this.ropper = ropper;
this.method = method;
+ this.methods = methods;
this.advice = advice;
this.maxLocals = method.getMaxLocals();
this.insns = new ArrayList<Insn>(25);
@@ -907,6 +920,35 @@
return RegOps.PUT_FIELD;
}
case ByteOps.INVOKEVIRTUAL: {
+ CstMethodRef ref = (CstMethodRef) cst;
+ // The java bytecode specification does not explicitly disallow
+ // invokevirtual calls to any instance method, though it
+ // specifies that instance methods and private methods "should" be
+ // called using "invokespecial" instead of "invokevirtual".
+ // Several bytecode tools generate "invokevirtual" instructions for
+ // invocation of private methods.
+ //
+ // The dalvik opcode specification on the other hand allows
+ // invoke-virtual to be used only with "normal" virtual methods,
+ // i.e, ones that are not private, static, final or constructors.
+ // We therefore need to transform invoke-virtual calls to private
+ // instance methods to invoke-direct opcodes.
+ //
+ // Note that it assumes that all methods for a given class are
+ // defined in the same dex file.
+ //
+ // NOTE: This is a slow O(n) loop, and can be replaced with a
+ // faster implementation (at the cost of higher memory usage)
+ // if it proves to be a hot area of code.
+ if (ref.getDefiningClass() == method.getDefiningClass()) {
+ for (int i = 0; i < methods.size(); ++i) {
+ final Method m = methods.get(i);
+ if (AccessFlags.isPrivate(m.getAccessFlags()) &&
+ ref.getNat().equals(m.getNat())) {
+ return RegOps.INVOKE_DIRECT;
+ }
+ }
+ }
return RegOps.INVOKE_VIRTUAL;
}
case ByteOps.INVOKESPECIAL: {
diff --git a/dx/src/com/android/dx/command/dump/BlockDumper.java b/dx/src/com/android/dx/command/dump/BlockDumper.java
index b670cd7..8d19c45 100644
--- a/dx/src/com/android/dx/command/dump/BlockDumper.java
+++ b/dx/src/com/android/dx/command/dump/BlockDumper.java
@@ -285,7 +285,7 @@
TranslationAdvice advice = DexTranslationAdvice.THE_ONE;
BytecodeArray code = meth.getCode();
ByteArray bytes = code.getBytes();
- RopMethod rmeth = Ropper.convert(meth, advice);
+ RopMethod rmeth = Ropper.convert(meth, advice, classFile.getMethods());
StringBuffer sb = new StringBuffer(2000);
if (optimize) {
diff --git a/dx/src/com/android/dx/command/dump/DotDumper.java b/dx/src/com/android/dx/command/dump/DotDumper.java
index 1c4c74b..eb9f00d 100644
--- a/dx/src/com/android/dx/command/dump/DotDumper.java
+++ b/dx/src/com/android/dx/command/dump/DotDumper.java
@@ -115,7 +115,7 @@
TranslationAdvice advice = DexTranslationAdvice.THE_ONE;
RopMethod rmeth =
- Ropper.convert(meth, advice);
+ Ropper.convert(meth, advice, classFile.getMethods());
if (optimize) {
boolean isStatic = AccessFlags.isStatic(meth.getAccessFlags());
diff --git a/dx/src/com/android/dx/command/dump/SsaDumper.java b/dx/src/com/android/dx/command/dump/SsaDumper.java
index 80953c1..76f919c 100644
--- a/dx/src/com/android/dx/command/dump/SsaDumper.java
+++ b/dx/src/com/android/dx/command/dump/SsaDumper.java
@@ -90,7 +90,7 @@
ConcreteMethod meth =
new ConcreteMethod((Method) member, classFile, true, true);
TranslationAdvice advice = DexTranslationAdvice.THE_ONE;
- RopMethod rmeth = Ropper.convert(meth, advice);
+ RopMethod rmeth = Ropper.convert(meth, advice, classFile.getMethods());
SsaMethod ssaMeth = null;
boolean isStatic = AccessFlags.isStatic(meth.getAccessFlags());
int paramWidth = computeParamWidth(meth, isStatic);
diff --git a/dx/src/com/android/dx/dex/cf/CfTranslator.java b/dx/src/com/android/dx/dex/cf/CfTranslator.java
index 92ea0f7..1d2542d 100644
--- a/dx/src/com/android/dx/dex/cf/CfTranslator.java
+++ b/dx/src/com/android/dx/dex/cf/CfTranslator.java
@@ -279,7 +279,7 @@
advice = DexTranslationAdvice.THE_ONE;
- RopMethod rmeth = Ropper.convert(concrete, advice);
+ RopMethod rmeth = Ropper.convert(concrete, advice, methods);
RopMethod nonOptRmeth = null;
int paramSize;
diff --git a/dx/tests/123-dex-transform-invalid-virtual-to-direct/Zorch.class b/dx/tests/123-dex-transform-invalid-virtual-to-direct/Zorch.class
new file mode 100644
index 0000000..cd60b9a
--- /dev/null
+++ b/dx/tests/123-dex-transform-invalid-virtual-to-direct/Zorch.class
Binary files differ
diff --git a/dx/tests/123-dex-transform-invalid-virtual-to-direct/Zorch.java b/dx/tests/123-dex-transform-invalid-virtual-to-direct/Zorch.java
new file mode 100644
index 0000000..a163cb0
--- /dev/null
+++ b/dx/tests/123-dex-transform-invalid-virtual-to-direct/Zorch.java
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Zorch
+{
+ public void zorch1() {
+ zorch2(0);
+ }
+
+ private int zorch2(int x) {
+ return 0;
+ }
+}
diff --git a/dx/tests/123-dex-transform-invalid-virtual-to-direct/expected.txt b/dx/tests/123-dex-transform-invalid-virtual-to-direct/expected.txt
new file mode 100644
index 0000000..8c39333
--- /dev/null
+++ b/dx/tests/123-dex-transform-invalid-virtual-to-direct/expected.txt
@@ -0,0 +1,8 @@
+Zorch.zorch1:()V:
+regs: 0004; ins: 0001; outs: 0002
+ 0000: move-object v0, v3
+ 0001: move-object v1, v0
+ 0002: const/4 v2, #int 0 // #0
+ 0003: invoke-direct {v1, v2}, Zorch.zorch2:(I)I
+ 0006: move-result v1
+ 0007: return-void
diff --git a/dx/tests/123-dex-transform-invalid-virtual-to-direct/info.txt b/dx/tests/123-dex-transform-invalid-virtual-to-direct/info.txt
new file mode 100644
index 0000000..4b882cf
--- /dev/null
+++ b/dx/tests/123-dex-transform-invalid-virtual-to-direct/info.txt
@@ -0,0 +1,10 @@
+This test checks that we convert invoke-virtual calls to private
+methods with invoke-direct calls.
+
+The Zorch.class file is manually created by compiling Zorch.java
+with java6 and manually editing the file to replace the opcode at
+position 0xE9 with 0xB6 (invokevirtual) instead of 0xB7 (invokespecial).
+
+This test compares emitted code against a known-good (via eyeballing)
+version, so it is possible for this test to spuriously fail if other
+aspects of conversion end up altering the output in innocuous ways.
diff --git a/dx/tests/123-dex-transform-invalid-virtual-to-direct/run b/dx/tests/123-dex-transform-invalid-virtual-to-direct/run
new file mode 100644
index 0000000..71ff62f
--- /dev/null
+++ b/dx/tests/123-dex-transform-invalid-virtual-to-direct/run
@@ -0,0 +1,18 @@
+#!/bin/bash
+#
+# Copyright (C) 2014 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+dx --debug --dex --no-optimize --positions=none --no-locals \
+ --dump-method=Zorch.zorch1 Zorch.class
