diff options
-rw-r--r-- | runtime/class_linker.cc | 61 | ||||
-rw-r--r-- | test/182-method-linking/expected-stderr.txt | 0 | ||||
-rw-r--r-- | test/182-method-linking/expected-stdout.txt | 34 | ||||
-rw-r--r-- | test/182-method-linking/info.txt | 1 | ||||
-rw-r--r-- | test/182-method-linking/src/Main.java | 67 | ||||
-rw-r--r-- | test/182-method-linking/src/pkg1/A.java | 28 | ||||
-rw-r--r-- | test/182-method-linking/src/pkg1/C.java | 30 | ||||
-rw-r--r-- | test/182-method-linking/src/pkg1/C2.java | 31 | ||||
-rw-r--r-- | test/182-method-linking/src/pkg2/B.java | 30 | ||||
-rw-r--r-- | test/182-method-linking/src/pkg2/D.java | 30 | ||||
-rw-r--r-- | test/182-method-linking/src/pkg2/D2.java | 31 |
11 files changed, 330 insertions, 13 deletions
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 7f2d92686c..d1de4ba9cb 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -7533,6 +7533,13 @@ class ClassLinker::LinkMethodsHelper { ArenaStack stack_; ScopedArenaAllocator allocator_; + // If there are multiple methods with the same signature in the superclass vtable + // (which can happen with a new virtual method having the same signature as an + // inaccessible package-private method from another package in the superclass), + // we keep singly-linked lists in this single array that maps vtable index to the + // next vtable index in the list, `dex::kDexNoIndex` denotes the end of a list. + ArrayRef<uint32_t> same_signature_vtable_lists_; + // Avoid large allocation for a few copied method records. // Keep the initial buffer on the stack to avoid arena allocations // if there are no special cases (the first arena allocation is costly). @@ -7971,6 +7978,7 @@ size_t ClassLinker::LinkMethodsHelper<kPointerSize>::AssignVTableIndexes( same_signature_vtable_lists = ArrayRef<uint32_t>( allocator_.AllocArray<uint32_t>(super_vtable_length), super_vtable_length); std::fill_n(same_signature_vtable_lists.data(), super_vtable_length, dex::kDexNoIndex); + same_signature_vtable_lists_ = same_signature_vtable_lists; } DCHECK_LT(*it, i); same_signature_vtable_lists[i] = *it; @@ -8008,9 +8016,18 @@ size_t ClassLinker::LinkMethodsHelper<kPointerSize>::AssignVTableIndexes( // superclass method would have been incorrectly overridden. bool overrides = klass->CanAccessMember(super_method->GetDeclaringClass(), super_method->GetAccessFlags()); + if (overrides && super_method->IsFinal()) { + sants.reset(); + ThrowLinkageError(klass, "Method %s overrides final method in class %s", + virtual_method->PrettyMethod().c_str(), + super_method->GetDeclaringClassDescriptor()); + return 0u; + } if (UNLIKELY(!same_signature_vtable_lists.empty())) { - // We override only the first accessible virtual method from superclass. - // TODO: Override all methods that need to be overridden according to JLS. b/211854716 + // We may override more than one method according to JLS, see b/211854716 . + // We record the highest overridden vtable index here so that we can walk + // the list to find other overridden methods when constructing the vtable. + // However, we walk all the methods to check for final method overriding. size_t current_index = super_index; while (same_signature_vtable_lists[current_index] != dex::kDexNoIndex) { DCHECK_LT(same_signature_vtable_lists[current_index], current_index); @@ -8018,20 +8035,22 @@ size_t ClassLinker::LinkMethodsHelper<kPointerSize>::AssignVTableIndexes( ArtMethod* current_method = super_vtable_accessor.GetVTableEntry(current_index); if (klass->CanAccessMember(current_method->GetDeclaringClass(), current_method->GetAccessFlags())) { - overrides = true; - super_index = current_index; - super_method = current_method; + if (current_method->IsFinal()) { + sants.reset(); + ThrowLinkageError(klass, "Method %s overrides final method in class %s", + virtual_method->PrettyMethod().c_str(), + current_method->GetDeclaringClassDescriptor()); + return 0u; + } + if (!overrides) { + overrides = true; + super_index = current_index; + super_method = current_method; + } } } } if (overrides) { - if (super_method->IsFinal()) { - sants.reset(); - ThrowLinkageError(klass, "Method %s overrides final method in class %s", - virtual_method->PrettyMethod().c_str(), - super_method->GetDeclaringClassDescriptor()); - return 0u; - } virtual_method->SetMethodIndex(super_index); continue; } @@ -8426,9 +8445,25 @@ bool ClassLinker::LinkMethodsHelper<kPointerSize>::LinkMethods( } // Store new virtual methods in the new vtable. + ArrayRef<uint32_t> same_signature_vtable_lists = same_signature_vtable_lists_; for (ArtMethod& virtual_method : klass->GetVirtualMethodsSliceUnchecked(kPointerSize)) { - int32_t vtable_index = virtual_method.GetMethodIndexDuringLinking(); + uint32_t vtable_index = virtual_method.GetMethodIndexDuringLinking(); vtable->SetElementPtrSize(vtable_index, &virtual_method, kPointerSize); + if (UNLIKELY(vtable_index < same_signature_vtable_lists.size())) { + // We may override more than one method according to JLS, see b/211854716 . + // If we do, arbitrarily update the method index to the lowest overridden vtable index. + while (same_signature_vtable_lists[vtable_index] != dex::kDexNoIndex) { + DCHECK_LT(same_signature_vtable_lists[vtable_index], vtable_index); + vtable_index = same_signature_vtable_lists[vtable_index]; + ArtMethod* current_method = super_class->GetVTableEntry(vtable_index, kPointerSize); + if (klass->CanAccessMember(current_method->GetDeclaringClass(), + current_method->GetAccessFlags())) { + DCHECK(!current_method->IsFinal()); + vtable->SetElementPtrSize(vtable_index, &virtual_method, kPointerSize); + virtual_method.SetMethodIndex(vtable_index); + } + } + } } // For non-overridden vtable slots, copy a method from `super_class`. diff --git a/test/182-method-linking/expected-stderr.txt b/test/182-method-linking/expected-stderr.txt new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/test/182-method-linking/expected-stderr.txt diff --git a/test/182-method-linking/expected-stdout.txt b/test/182-method-linking/expected-stdout.txt new file mode 100644 index 0000000000..712b0d7738 --- /dev/null +++ b/test/182-method-linking/expected-stdout.txt @@ -0,0 +1,34 @@ +Calling pkg1.A.foo on pkg1.A +pkg1.A.foo +Calling pkg1.A.foo on pkg2.B +pkg1.A.foo +Calling pkg2.B.foo on pkg2.B +pkg2.B.foo +Calling pkg1.A.foo on pkg1.C +pkg1.C.foo +Calling pkg2.B.foo on pkg1.C +pkg2.B.foo +Calling pkg1.C.foo on pkg1.C +pkg1.C.foo +Calling pkg1.A.foo on pkg2.D +pkg1.C.foo +Calling pkg2.B.foo on pkg2.D +pkg2.D.foo +Calling pkg1.C.foo on pkg2.D +pkg1.C.foo +Calling pkg2.D.foo on pkg2.D +pkg2.D.foo +Calling pkg1.A.foo on pkg1.C2 +pkg1.C2.foo +Calling pkg2.B.foo on pkg1.C2 +pkg2.B.foo +Calling pkg1.C2.foo on pkg1.C2 +pkg1.C2.foo +Calling pkg1.A.foo on pkg2.D2 +pkg2.D2.foo +Calling pkg2.B.foo on pkg2.D2 +pkg2.D2.foo +Calling pkg1.C2.foo on pkg2.D2 +pkg2.D2.foo +Calling pkg2.D2.foo on pkg2.D2 +pkg2.D2.foo diff --git a/test/182-method-linking/info.txt b/test/182-method-linking/info.txt new file mode 100644 index 0000000000..3772294fc3 --- /dev/null +++ b/test/182-method-linking/info.txt @@ -0,0 +1 @@ +Test some edge cases for method linking. diff --git a/test/182-method-linking/src/Main.java b/test/182-method-linking/src/Main.java new file mode 100644 index 0000000000..50102c13b7 --- /dev/null +++ b/test/182-method-linking/src/Main.java @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2022 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 pkg1.A; +import pkg1.C; +import pkg1.C2; +import pkg2.B; +import pkg2.D; +import pkg2.D2; + +public class Main { + public static void main(String args[]) { + // A single method signature can result in multiple vtable entries + // when package-private methods from different packages are involved. + // All classes here define the method `void foo()` but classes + // class pkg1.A { ... } + // class pkg2.B extends pkg1.A { ... } + // class pkg1.C extends pkg2.B { ... } + // class pkg2.D extends pkg1.C { ... } + // define it as package-private and classes + // class pkg1.C2 extends pkg2.B { ... } + // class pkg2.D2 extends pkg1.C2 { ... } + // define it as public, so that we can test different cases of overriding. + + A a = new A(); + a.callAFoo(); // pkg1.A.foo + + B b = new B(); + b.callAFoo(); // pkg1.A.foo (not overridden by pkg2.B.foo) + b.callBFoo(); // pkg2.B.foo + + C c = new C(); + c.callAFoo(); // pkg1.C.foo (overriddes pkg1.A.foo) + c.callBFoo(); // pkg2.B.foo (not overridden by pkg1.C.foo) + c.callCFoo(); // pkg1.C.foo + + D d = new D(); + d.callAFoo(); // pkg1.C.foo (not overridden by pkg2.D.foo) + d.callBFoo(); // pkg2.D.foo (overrides pkg2.B.foo) + d.callCFoo(); // pkg1.C.foo (not overridden by pkg2.D.foo) + d.callDFoo(); // pkg2.D.foo + + C2 c2 = new C2(); + c2.callAFoo(); // pkg1.C2.foo (overriddes pkg1.A.foo) + c2.callBFoo(); // pkg2.B.foo (not overridden by pkg1.C2.foo) + c2.callC2Foo(); // pkg1.C2.foo + + D2 d2 = new D2(); + d2.callAFoo(); // pkg2.D2.foo (overrides public pkg2.C2.foo which overrides pkg1.A.foo) + d2.callBFoo(); // pkg2.D2.foo (overrides package-private pkg2.B.foo in the same package) + d2.callC2Foo(); // pkg2.D2.foo (overrides public pkg2.C2.foo) + d2.callD2Foo(); // pkg2.D2.foo + } +} diff --git a/test/182-method-linking/src/pkg1/A.java b/test/182-method-linking/src/pkg1/A.java new file mode 100644 index 0000000000..c0e4fd457c --- /dev/null +++ b/test/182-method-linking/src/pkg1/A.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2022 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. + */ + +package pkg1; + +public class A { + public void callAFoo() { + System.out.println("Calling pkg1.A.foo on " + getClass().getName()); + foo(); + }; + + /*package-private*/ void foo() { + System.out.println("pkg1.A.foo"); + } +} diff --git a/test/182-method-linking/src/pkg1/C.java b/test/182-method-linking/src/pkg1/C.java new file mode 100644 index 0000000000..004d342171 --- /dev/null +++ b/test/182-method-linking/src/pkg1/C.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2022 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. + */ + +package pkg1; + +import pkg2.B; + +public class C extends B { + public void callCFoo() { + System.out.println("Calling pkg1.C.foo on " + getClass().getName()); + foo(); + }; + + /*package-private*/ void foo() { + System.out.println("pkg1.C.foo"); + } +} diff --git a/test/182-method-linking/src/pkg1/C2.java b/test/182-method-linking/src/pkg1/C2.java new file mode 100644 index 0000000000..16fea07659 --- /dev/null +++ b/test/182-method-linking/src/pkg1/C2.java @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2022 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. + */ + +package pkg1; + +import pkg2.B; + +public class C2 extends B { + public void callC2Foo() { + System.out.println("Calling pkg1.C2.foo on " + getClass().getName()); + foo(); + }; + + // This overrides package-private method as public. + public void foo() { + System.out.println("pkg1.C2.foo"); + } +} diff --git a/test/182-method-linking/src/pkg2/B.java b/test/182-method-linking/src/pkg2/B.java new file mode 100644 index 0000000000..3e3a4ccf6a --- /dev/null +++ b/test/182-method-linking/src/pkg2/B.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2022 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. + */ + +package pkg2; + +import pkg1.A; + +public class B extends A { + public void callBFoo() { + System.out.println("Calling pkg2.B.foo on " + getClass().getName()); + foo(); + }; + + /*package-private*/ void foo() { + System.out.println("pkg2.B.foo"); + } +} diff --git a/test/182-method-linking/src/pkg2/D.java b/test/182-method-linking/src/pkg2/D.java new file mode 100644 index 0000000000..d79a769048 --- /dev/null +++ b/test/182-method-linking/src/pkg2/D.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2022 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. + */ + +package pkg2; + +import pkg1.C; + +public class D extends C { + public void callDFoo() { + System.out.println("Calling pkg2.D.foo on " + getClass().getName()); + foo(); + }; + + /*package-private*/ void foo() { + System.out.println("pkg2.D.foo"); + } +} diff --git a/test/182-method-linking/src/pkg2/D2.java b/test/182-method-linking/src/pkg2/D2.java new file mode 100644 index 0000000000..d1642ca350 --- /dev/null +++ b/test/182-method-linking/src/pkg2/D2.java @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2022 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. + */ + +package pkg2; + +import pkg1.C2; + +public class D2 extends C2 { + public void callD2Foo() { + System.out.println("Calling pkg2.D2.foo on " + getClass().getName()); + foo(); + }; + + // This overrides both public pkg1.C2.foo() and package-private pkg2.B.foo(). + public void foo() { + System.out.println("pkg2.D2.foo"); + } +} |