OSDN Git Service

Make InvokeInterfaceTrampoline check methods
authorAlex Light <allight@google.com>
Thu, 31 Mar 2016 21:34:33 +0000 (14:34 -0700)
committerAlex Light <allight@google.com>
Fri, 1 Apr 2016 16:46:37 +0000 (09:46 -0700)
InvokeInterfaceTrampoline was causing problems by looking into the
ImtConflictTable of non-imt-conflict-methods. This makes it check for
that before doing so.

Bug: 27931085

Change-Id: I993178a371f8f46535a752e5c4d46d74777cefaf

runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
test/146-bad-interface/build [new file with mode: 0755]
test/146-bad-interface/expected.txt [new file with mode: 0644]
test/146-bad-interface/info.txt [new file with mode: 0644]
test/146-bad-interface/smali/invoke_inf.smali [new file with mode: 0644]
test/146-bad-interface/src-ex/A.java [new file with mode: 0644]
test/146-bad-interface/src-ex/Iface.java [new file with mode: 0644]
test/146-bad-interface/src/Main.java [new file with mode: 0644]

index 35f2102..7375656 100644 (file)
@@ -2166,9 +2166,13 @@ extern "C" TwoWordReturn artInvokeInterfaceTrampoline(uint32_t deadbeef ATTRIBUT
     uint32_t imt_index = interface_method->GetDexMethodIndex();
     ArtMethod* conflict_method = cls->GetEmbeddedImTableEntry(
         imt_index % mirror::Class::kImtSize, sizeof(void*));
-    DCHECK(conflict_method->IsRuntimeMethod()) << PrettyMethod(conflict_method);
-    ImtConflictTable* current_table = conflict_method->GetImtConflictTable(sizeof(void*));
-    method = current_table->Lookup(interface_method);
+    if (LIKELY(conflict_method->IsRuntimeMethod())) {
+      ImtConflictTable* current_table = conflict_method->GetImtConflictTable(sizeof(void*));
+      method = current_table->Lookup(interface_method);
+    } else {
+      // It seems we aren't really a conflict method!
+      method = cls->FindVirtualMethodForInterface(interface_method, sizeof(void*));
+    }
     if (method != nullptr) {
       return GetTwoWordSuccessValue(
           reinterpret_cast<uintptr_t>(method->GetEntryPointFromQuickCompiledCode()),
@@ -2214,39 +2218,41 @@ extern "C" TwoWordReturn artInvokeInterfaceTrampoline(uint32_t deadbeef ATTRIBUT
   uint32_t imt_index = interface_method->GetDexMethodIndex();
   ArtMethod* conflict_method = cls->GetEmbeddedImTableEntry(
       imt_index % mirror::Class::kImtSize, sizeof(void*));
-  ImtConflictTable* current_table = conflict_method->GetImtConflictTable(sizeof(void*));
-  Runtime* runtime = Runtime::Current();
-  LinearAlloc* linear_alloc = (cls->GetClassLoader() == nullptr)
-      ? runtime->GetLinearAlloc()
-      : cls->GetClassLoader()->GetAllocator();
-  bool is_new_entry = (conflict_method == runtime->GetImtConflictMethod());
-
-  // Create a new entry if the existing one is the shared conflict method.
-  ArtMethod* new_conflict_method = is_new_entry
-      ? runtime->CreateImtConflictMethod(linear_alloc)
-      : conflict_method;
-
-  // Allocate a new table. Note that we will leak this table at the next conflict,
-  // but that's a tradeoff compared to making the table fixed size.
-  void* data = linear_alloc->Alloc(
-      self, ImtConflictTable::ComputeSizeWithOneMoreEntry(current_table));
-  CHECK(data != nullptr) << "Out of memory";
-  ImtConflictTable* new_table = new (data) ImtConflictTable(
-      current_table, interface_method, method);
-
-  // Do a fence to ensure threads see the data in the table before it is assigned
-  // to the conlict method.
-  // Note that there is a race in the presence of multiple threads and we may leak
-  // memory from the LinearAlloc, but that's a tradeoff compared to using
-  // atomic operations.
-  QuasiAtomic::ThreadFenceRelease();
-  new_conflict_method->SetImtConflictTable(new_table);
-  if (is_new_entry) {
-    // Update the IMT if we create a new conflict method. No fence needed here, as the
-    // data is consistent.
-    cls->SetEmbeddedImTableEntry(imt_index % mirror::Class::kImtSize,
-                                 new_conflict_method,
-                                 sizeof(void*));
+  if (conflict_method->IsRuntimeMethod()) {
+    ImtConflictTable* current_table = conflict_method->GetImtConflictTable(sizeof(void*));
+    Runtime* runtime = Runtime::Current();
+    LinearAlloc* linear_alloc = (cls->GetClassLoader() == nullptr)
+        ? runtime->GetLinearAlloc()
+        : cls->GetClassLoader()->GetAllocator();
+    bool is_new_entry = (conflict_method == runtime->GetImtConflictMethod());
+
+    // Create a new entry if the existing one is the shared conflict method.
+    ArtMethod* new_conflict_method = is_new_entry
+        ? runtime->CreateImtConflictMethod(linear_alloc)
+        : conflict_method;
+
+    // Allocate a new table. Note that we will leak this table at the next conflict,
+    // but that's a tradeoff compared to making the table fixed size.
+    void* data = linear_alloc->Alloc(
+        self, ImtConflictTable::ComputeSizeWithOneMoreEntry(current_table));
+    CHECK(data != nullptr) << "Out of memory";
+    ImtConflictTable* new_table = new (data) ImtConflictTable(
+        current_table, interface_method, method);
+
+    // Do a fence to ensure threads see the data in the table before it is assigned
+    // to the conlict method.
+    // Note that there is a race in the presence of multiple threads and we may leak
+    // memory from the LinearAlloc, but that's a tradeoff compared to using
+    // atomic operations.
+    QuasiAtomic::ThreadFenceRelease();
+    new_conflict_method->SetImtConflictTable(new_table);
+    if (is_new_entry) {
+      // Update the IMT if we create a new conflict method. No fence needed here, as the
+      // data is consistent.
+      cls->SetEmbeddedImTableEntry(imt_index % mirror::Class::kImtSize,
+                                  new_conflict_method,
+                                  sizeof(void*));
+    }
   }
 
   const void* code = method->GetEntryPointFromQuickCompiledCode();
diff --git a/test/146-bad-interface/build b/test/146-bad-interface/build
new file mode 100755 (executable)
index 0000000..0dd8573
--- /dev/null
@@ -0,0 +1,27 @@
+#!/bin/bash
+#
+# Copyright 2015 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.
+
+# make us exit on a failure
+set -e
+
+if [[ $@ != *"--jvm"* ]]; then
+  # Don't do anything with jvm
+  # Hard-wired use of experimental jack.
+  # TODO: fix this temporary work-around for default-methods, see b/19467889
+  export USE_JACK=true
+fi
+
+./default-build "$@" --experimental default-methods
diff --git a/test/146-bad-interface/expected.txt b/test/146-bad-interface/expected.txt
new file mode 100644 (file)
index 0000000..3441966
--- /dev/null
@@ -0,0 +1 @@
+running invoke
diff --git a/test/146-bad-interface/info.txt b/test/146-bad-interface/info.txt
new file mode 100644 (file)
index 0000000..38f188e
--- /dev/null
@@ -0,0 +1 @@
+Check whether a duplicate class can invoke-interface on an unresolved method.
diff --git a/test/146-bad-interface/smali/invoke_inf.smali b/test/146-bad-interface/smali/invoke_inf.smali
new file mode 100644 (file)
index 0000000..c5101e0
--- /dev/null
@@ -0,0 +1,24 @@
+# Copyright (C) 2016 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.
+
+
+.class public LInvokeInf;
+.super Ljava/lang/Object;
+
+.method public static doInvoke(LIface;)V
+.locals 0
+    invoke-interface {p0}, LIface;->invoke()V
+    return-void
+.end method
+
diff --git a/test/146-bad-interface/src-ex/A.java b/test/146-bad-interface/src-ex/A.java
new file mode 100644 (file)
index 0000000..a30a5f2
--- /dev/null
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2015 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 A implements Iface {
+}
diff --git a/test/146-bad-interface/src-ex/Iface.java b/test/146-bad-interface/src-ex/Iface.java
new file mode 100644 (file)
index 0000000..921e25c
--- /dev/null
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2016 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 interface Iface {
+  public default void aPadding() {}
+  public default void bPadding() {}
+  public default void cPadding() {}
+  public default void dPadding() {}
+  public default void invoke() {
+    System.out.println("running invoke");
+  }
+  public default void wPadding() {}
+  public default void xPadding() {}
+  public default void yPadding() {}
+  public default void zPadding() {}
+}
diff --git a/test/146-bad-interface/src/Main.java b/test/146-bad-interface/src/Main.java
new file mode 100644 (file)
index 0000000..5534bb4
--- /dev/null
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+import java.lang.reflect.Method;
+import dalvik.system.PathClassLoader;
+
+/**
+ * Structural hazard test.
+ */
+public class Main {
+  static final String DEX_LOCATION = System.getenv("DEX_LOCATION");
+  static final String DEX_FILES =
+      DEX_LOCATION + "/146-bad-interface-ex.jar" + ":" +
+      DEX_LOCATION + "/146-bad-interface.jar";
+  public static void main(String[] args) {
+    try {
+      PathClassLoader p = new PathClassLoader(DEX_FILES, Main.class.getClassLoader());
+      Class<?> c = Class.forName("A", true, p);
+      Object o = c.newInstance();
+      Class<?> runner = Class.forName("InvokeInf", true, p);
+      Class<?> arg = Class.forName("Iface", true, p);
+      Method r = runner.getDeclaredMethod("doInvoke", arg);
+      r.invoke(null, o);
+    } catch (Throwable t) {
+      System.out.println("Error occurred");
+      System.out.println(t);
+      t.printStackTrace();
+    }
+  }
+}