diff options
| author | 2016-03-31 14:34:33 -0700 | |
|---|---|---|
| committer | 2016-04-01 09:46:37 -0700 | |
| commit | 9fc547ac3936fe88e9592f4a47afd7b134cb607c (patch) | |
| tree | 1f604cb34a00418694515e7790657be8306650e1 | |
| parent | 87ad82eb1e085ccc6ed3ec54945937582334dbbc (diff) | |
Make InvokeInterfaceTrampoline check methods
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
| -rw-r--r-- | runtime/entrypoints/quick/quick_trampoline_entrypoints.cc | 78 | ||||
| -rwxr-xr-x | test/146-bad-interface/build | 27 | ||||
| -rw-r--r-- | test/146-bad-interface/expected.txt | 1 | ||||
| -rw-r--r-- | test/146-bad-interface/info.txt | 1 | ||||
| -rw-r--r-- | test/146-bad-interface/smali/invoke_inf.smali | 24 | ||||
| -rw-r--r-- | test/146-bad-interface/src-ex/A.java | 18 | ||||
| -rw-r--r-- | test/146-bad-interface/src-ex/Iface.java | 29 | ||||
| -rw-r--r-- | test/146-bad-interface/src/Main.java | 43 |
8 files changed, 185 insertions, 36 deletions
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc index 35f2102b25..7375656b35 100644 --- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc +++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc @@ -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 index 0000000000..0dd8573f54 --- /dev/null +++ b/test/146-bad-interface/build @@ -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 index 0000000000..344196674f --- /dev/null +++ b/test/146-bad-interface/expected.txt @@ -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 index 0000000000..38f188e728 --- /dev/null +++ b/test/146-bad-interface/info.txt @@ -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 index 0000000000..c5101e020a --- /dev/null +++ b/test/146-bad-interface/smali/invoke_inf.smali @@ -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 index 0000000000..a30a5f23ec --- /dev/null +++ b/test/146-bad-interface/src-ex/A.java @@ -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 index 0000000000..921e25c6bf --- /dev/null +++ b/test/146-bad-interface/src-ex/Iface.java @@ -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 index 0000000000..5534bb4bba --- /dev/null +++ b/test/146-bad-interface/src/Main.java @@ -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(); + } + } +} |