Ensure that overrides work in presence of package-private methods.
It was possible that methods with the same signature & name of
package-private methods could fail to be correctly overridden causing
surprising behavior and DCHECK failures.
Bug: 32193118
Test: mma test-art-host
Test: ART_TEST_RUN_TEST_NDEBUG=true ART_TEST_RUN_TEST_NO_PREBUILD=true mma test-art-host-run-test-300-package-override
Change-Id: I8f53a830cd8d4210f60e9827e525c779a0696e04
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index a7d8b13..cea8377 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -5643,27 +5643,29 @@
for (size_t j = 0; j < super_vtable_length; ++j) {
// Search the hash table to see if we are overridden by any method.
ArtMethod* super_method = vtable->GetElementPtrSize<ArtMethod*>(j, image_pointer_size_);
+ if (!klass->CanAccessMember(super_method->GetDeclaringClass(),
+ super_method->GetAccessFlags())) {
+ // Continue on to the next method since this one is package private and canot be overridden.
+ // Before Android 4.1, the package-private method super_method might have been incorrectly
+ // overridden.
+ continue;
+ }
MethodNameAndSignatureComparator super_method_name_comparator(
super_method->GetInterfaceMethodIfProxy(image_pointer_size_));
+ // We remove the method so that subsequent lookups will be faster by making the hash-map
+ // smaller as we go on.
uint32_t hash_index = hash_table.FindAndRemove(&super_method_name_comparator);
if (hash_index != hash_table.GetNotFoundIndex()) {
ArtMethod* virtual_method = klass->GetVirtualMethodDuringLinking(
hash_index, image_pointer_size_);
- if (klass->CanAccessMember(super_method->GetDeclaringClass(),
- super_method->GetAccessFlags())) {
- if (super_method->IsFinal()) {
- ThrowLinkageError(klass.Get(), "Method %s overrides final method in class %s",
- ArtMethod::PrettyMethod(virtual_method).c_str(),
- super_method->GetDeclaringClassDescriptor());
- return false;
- }
- vtable->SetElementPtrSize(j, virtual_method, image_pointer_size_);
- virtual_method->SetMethodIndex(j);
- } else {
- LOG(WARNING) << "Before Android 4.1, method " << ArtMethod::PrettyMethod(virtual_method)
- << " would have incorrectly overridden the package-private method in "
- << PrettyDescriptor(super_method->GetDeclaringClassDescriptor());
+ if (super_method->IsFinal()) {
+ ThrowLinkageError(klass.Get(), "Method %s overrides final method in class %s",
+ virtual_method->PrettyMethod().c_str(),
+ super_method->GetDeclaringClassDescriptor());
+ return false;
}
+ vtable->SetElementPtrSize(j, virtual_method, image_pointer_size_);
+ virtual_method->SetMethodIndex(j);
} else if (super_method->IsOverridableByDefaultMethod()) {
// We didn't directly override this method but we might through default methods...
// Check for default method update.
@@ -6458,15 +6460,20 @@
}
MethodNameAndSignatureComparator name_comparator(
vtable_entry->GetInterfaceMethodIfProxy(pointer_size));
- for (int32_t j = i+1; j < num_entries; j++) {
+ for (int32_t j = i + 1; j < num_entries; j++) {
ArtMethod* other_entry = vtable->GetElementPtrSize<ArtMethod*>(j, pointer_size);
+ if (!klass->CanAccessMember(other_entry->GetDeclaringClass(),
+ other_entry->GetAccessFlags())) {
+ continue;
+ }
CHECK(vtable_entry != other_entry &&
!name_comparator.HasSameNameAndSignature(
other_entry->GetInterfaceMethodIfProxy(pointer_size)))
<< "vtable entries " << i << " and " << j << " are identical for "
- << klass->PrettyClass() << " in method "
- << vtable_entry->PrettyMethod()
- << " and " << other_entry->PrettyMethod();
+ << klass->PrettyClass() << " in method " << vtable_entry->PrettyMethod() << " (0x"
+ << std::hex << reinterpret_cast<uintptr_t>(vtable_entry) << ") and "
+ << other_entry->PrettyMethod() << " (0x" << std::hex
+ << reinterpret_cast<uintptr_t>(other_entry) << ")";
}
}
}
diff --git a/test/300-package-override/expected.txt b/test/300-package-override/expected.txt
index b0aad4d..a2c3f20 100644
--- a/test/300-package-override/expected.txt
+++ b/test/300-package-override/expected.txt
@@ -1 +1,4 @@
passed
+This should be visible!
+This should override!
+This should override!
diff --git a/test/300-package-override/src/Main.java b/test/300-package-override/src/Main.java
index ad7eaaf..a9319e3 100644
--- a/test/300-package-override/src/Main.java
+++ b/test/300-package-override/src/Main.java
@@ -18,5 +18,11 @@
public static void main(String args[]) throws Exception {
p1.BaseClass c = new p2.DerivedClass();
c.run();
+ p2.DerivedClass d = new p2.DerivedClass();
+ d.bar();
+ p2.DerivedClass d2 = new p2.DerivedClass2();
+ d2.bar();
+ p2.DerivedClass2 d3 = new p2.DerivedClass2();
+ d3.bar();
}
}
diff --git a/test/300-package-override/src/p1/BaseClass.java b/test/300-package-override/src/p1/BaseClass.java
index 1c048ac..eea35ec 100644
--- a/test/300-package-override/src/p1/BaseClass.java
+++ b/test/300-package-override/src/p1/BaseClass.java
@@ -19,4 +19,5 @@
public class BaseClass {
public void run() { foo(); }
void foo() { System.out.println("passed"); } // It should not be possible to override this.
+ void bar() { System.out.println("FAILED: This should not be called!"); }
}
diff --git a/test/300-package-override/src/p2/DerivedClass.java b/test/300-package-override/src/p2/DerivedClass.java
index 860f50c..76f6200 100644
--- a/test/300-package-override/src/p2/DerivedClass.java
+++ b/test/300-package-override/src/p2/DerivedClass.java
@@ -18,4 +18,5 @@
public class DerivedClass extends p1.BaseClass {
void foo() { System.out.println("DerivedClass overrode package-private method!"); } // This should not override BaseClass.foo.
+ public void bar() { System.out.println("This should be visible!"); }
}
diff --git a/test/300-package-override/src/p2/DerivedClass2.java b/test/300-package-override/src/p2/DerivedClass2.java
new file mode 100644
index 0000000..ab55799
--- /dev/null
+++ b/test/300-package-override/src/p2/DerivedClass2.java
@@ -0,0 +1,22 @@
+/*
+ * 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.
+ */
+
+package p2;
+
+// Regression test for b/32193118
+public class DerivedClass2 extends p2.DerivedClass {
+ public void bar() { System.out.println("This should override!"); }
+}