Make JNI work correctly with default methods.
Also adds some tests for JNI and DefaultMethods.
Bug: 27259142
Bug: 24618811
(cherry picked from commit 3612149aee482ab7a17da68b0ef5fef3879729a2)
Change-Id: I31222e3e41059d803be1dbb0f40e1144ac4bf457
diff --git a/runtime/art_method.h b/runtime/art_method.h
index f3e8d6b..ec00a7b 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -132,9 +132,12 @@
return (GetAccessFlags() & kAccFinal) != 0;
}
- // Returns true if this method might be copied from another class.
- bool MightBeCopied() {
- return IsMiranda() || IsDefault() || IsDefaultConflicting();
+ bool IsCopied() {
+ const bool copied = (GetAccessFlags() & kAccCopied) != 0;
+ // (IsMiranda() || IsDefaultConflicting()) implies copied
+ DCHECK(!(IsMiranda() || IsDefaultConflicting()) || copied)
+ << "Miranda or default-conflict methods must always be copied.";
+ return copied;
}
bool IsMiranda() {
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 936c988..9ea0827 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -759,7 +759,7 @@
SHARED_REQUIRES(Locks::mutator_lock_) {
if (m->IsRuntimeMethod()) {
CHECK(m->GetDeclaringClass() == nullptr) << PrettyMethod(m);
- } else if (m->MightBeCopied()) {
+ } else if (m->IsCopied()) {
CHECK(m->GetDeclaringClass() != nullptr) << PrettyMethod(m);
} else if (expected_class != nullptr) {
CHECK_EQ(m->GetDeclaringClassUnchecked(), expected_class) << PrettyMethod(m);
@@ -1137,18 +1137,18 @@
virtual void Visit(ArtMethod* method) SHARED_REQUIRES(Locks::mutator_lock_) {
GcRoot<mirror::Class>* resolved_types = method->GetDexCacheResolvedTypes(sizeof(void*));
- const bool maybe_copied = method->MightBeCopied();
+ const bool is_copied = method->IsCopied();
if (resolved_types != nullptr) {
bool in_image_space = false;
- if (kIsDebugBuild || maybe_copied) {
+ if (kIsDebugBuild || is_copied) {
in_image_space = header_.GetImageSection(ImageHeader::kSectionDexCacheArrays).Contains(
reinterpret_cast<const uint8_t*>(resolved_types) - header_.GetImageBegin());
}
// Must be in image space for non-miranda method.
- DCHECK(maybe_copied || in_image_space)
+ DCHECK(is_copied || in_image_space)
<< resolved_types << " is not in image starting at "
<< reinterpret_cast<void*>(header_.GetImageBegin());
- if (!maybe_copied || in_image_space) {
+ if (!is_copied || in_image_space) {
// Go through the array so that we don't need to do a slow map lookup.
method->SetDexCacheResolvedTypes(*reinterpret_cast<GcRoot<mirror::Class>**>(resolved_types),
sizeof(void*));
@@ -1157,15 +1157,15 @@
ArtMethod** resolved_methods = method->GetDexCacheResolvedMethods(sizeof(void*));
if (resolved_methods != nullptr) {
bool in_image_space = false;
- if (kIsDebugBuild || maybe_copied) {
+ if (kIsDebugBuild || is_copied) {
in_image_space = header_.GetImageSection(ImageHeader::kSectionDexCacheArrays).Contains(
reinterpret_cast<const uint8_t*>(resolved_methods) - header_.GetImageBegin());
}
// Must be in image space for non-miranda method.
- DCHECK(maybe_copied || in_image_space)
+ DCHECK(is_copied || in_image_space)
<< resolved_methods << " is not in image starting at "
<< reinterpret_cast<void*>(header_.GetImageBegin());
- if (!maybe_copied || in_image_space) {
+ if (!is_copied || in_image_space) {
// Go through the array so that we don't need to do a slow map lookup.
method->SetDexCacheResolvedMethods(*reinterpret_cast<ArtMethod***>(resolved_methods),
sizeof(void*));
@@ -6459,7 +6459,7 @@
for (ArtMethod* mir_method : miranda_methods) {
ArtMethod& new_method = *out;
new_method.CopyFrom(mir_method, image_pointer_size_);
- new_method.SetAccessFlags(new_method.GetAccessFlags() | kAccMiranda);
+ new_method.SetAccessFlags(new_method.GetAccessFlags() | kAccMiranda | kAccCopied);
DCHECK_NE(new_method.GetAccessFlags() & kAccAbstract, 0u)
<< "Miranda method should be abstract!";
move_table.emplace(mir_method, &new_method);
@@ -6478,7 +6478,9 @@
// yet it shouldn't have methods that are skipping access checks.
// TODO This is rather arbitrary. We should maybe support classes where only some of its
// methods are skip_access_checks.
- new_method.SetAccessFlags((new_method.GetAccessFlags() | kAccDefault) & ~kAccSkipAccessChecks);
+ constexpr uint32_t kSetFlags = kAccDefault | kAccCopied;
+ constexpr uint32_t kMaskFlags = ~kAccSkipAccessChecks;
+ new_method.SetAccessFlags((new_method.GetAccessFlags() | kSetFlags) & kMaskFlags);
move_table.emplace(def_method, &new_method);
++out;
}
@@ -6489,7 +6491,7 @@
// this as a default, non-abstract method, since thats what it is. Also clear the
// kAccSkipAccessChecks bit since this class hasn't been verified yet it shouldn't have
// methods that are skipping access checks.
- constexpr uint32_t kSetFlags = kAccDefault | kAccDefaultConflict;
+ constexpr uint32_t kSetFlags = kAccDefault | kAccDefaultConflict | kAccCopied;
constexpr uint32_t kMaskFlags = ~(kAccAbstract | kAccSkipAccessChecks);
new_method.SetAccessFlags((new_method.GetAccessFlags() | kSetFlags) & kMaskFlags);
DCHECK(new_method.IsDefaultConflicting());
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index 5c3029a..488826b 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -263,7 +263,7 @@
for (ArtMethod& method : klass->GetCopiedMethods(sizeof(void*))) {
AssertMethod(&method);
EXPECT_FALSE(method.IsDirect());
- EXPECT_TRUE(method.MightBeCopied());
+ EXPECT_TRUE(method.IsCopied());
EXPECT_TRUE(method.GetDeclaringClass()->IsInterface())
<< "declaring class: " << PrettyClass(method.GetDeclaringClass());
EXPECT_TRUE(method.GetDeclaringClass()->IsAssignableFrom(klass.Get()))
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 3f806d3..19584ed 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -502,7 +502,7 @@
if (method->IsDirect()) {
return method;
}
- if (method->GetDeclaringClass()->IsInterface() && !method->IsMiranda()) {
+ if (method->GetDeclaringClass()->IsInterface() && !method->IsCopied()) {
return FindVirtualMethodForInterface(method, pointer_size);
}
return FindVirtualMethodForVirtual(method, pointer_size);
diff --git a/runtime/modifiers.h b/runtime/modifiers.h
index ed4c5fc..c31b22e 100644
--- a/runtime/modifiers.h
+++ b/runtime/modifiers.h
@@ -50,6 +50,11 @@
// Used by a class to denote that the verifier has attempted to check it at least once.
static constexpr uint32_t kAccVerificationAttempted = 0x00080000; // class (runtime)
static constexpr uint32_t kAccFastNative = 0x00080000; // method (dex only)
+// This is set by the class linker during LinkInterfaceMethods. It is used by a method to represent
+// that it was copied from its declaring class into another class. All methods marked kAccMiranda
+// and kAccDefaultConflict will have this bit set. Any kAccDefault method contained in the methods_
+// array of a concrete class will also have this bit set.
+static constexpr uint32_t kAccCopied = 0x00100000; // method (runtime)
static constexpr uint32_t kAccMiranda = 0x00200000; // method (dex only)
static constexpr uint32_t kAccDefault = 0x00400000; // method (runtime)
// This is set by the class linker during LinkInterfaceMethods. Prior to that point we do not know