Revert^2 "Make opaque-jni-ids:swapable more efficient"
This reverts commit 42c52f53b3c85b5e3c984bca8eaec2e03893dd2e.
This un-reverts commit 4e7dd70e3ee7a25089bed791df8865966cb4b837.
We were missing some null-checks that caused a null-pointer
dereference to occur if one tried to get a jmethodID for an obsolete
method when we start with index-ids.
Reason for revert: Fixes issue causing failure in art-jit config.
Test: Manual
Test: count-fields.py
Bug: 134162467
Change-Id: Ie36c7a4edb70f96655b3b85e5f5eaa6bf47f5cc7
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index 6314d42..53bf4df 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -469,30 +469,9 @@
// need to check any subtypes too.
art::ObjPtr<art::mirror::ClassExt> ext(klass->GetExtData());
if (!ext.IsNull()) {
- bool non_index_id = false;
- ext->VisitJFieldIDs([&](jfieldID id, uint32_t idx, bool is_static)
- REQUIRES_SHARED(art::Locks::mutator_lock_) {
- if (!art::jni::JniIdManager::IsIndexId(id)) {
- non_index_id = true;
- *error_msg =
- StringPrintf("%s Field %d (%s) has non-index jni-ids.",
- (is_static ? "static" : "non-static"),
- idx,
- (is_static ? klass->GetStaticField(idx)
- : klass->GetInstanceField(idx))->PrettyField().c_str());
- }
- });
- ext->VisitJMethodIDs([&](jmethodID id, uint32_t idx)
- REQUIRES_SHARED(art::Locks::mutator_lock_) {
- if (!art::jni::JniIdManager::IsIndexId(id)) {
- non_index_id = true;
- *error_msg = StringPrintf(
- "method %d (%s) has non-index jni-ids.",
- idx,
- klass->GetDeclaredMethodsSlice(art::kRuntimePointerSize)[idx].PrettyMethod().c_str());
- }
- });
- if (non_index_id) {
+ if (ext->HasInstanceFieldPointerIdMarker() ||
+ ext->HasMethodPointerIdMarker() ||
+ ext->HasStaticFieldPointerIdMarker()) {
return ERR(UNMODIFIABLE_CLASS);
}
}
@@ -1988,9 +1967,9 @@
// Make sure we have ext-data space for method & field ids. We won't know if we need them until
// it's too late to create them.
// TODO We might want to remove these arrays if they're not needed.
- if (art::mirror::Class::GetOrCreateInstanceFieldIds(linked_class).IsNull() ||
- art::mirror::Class::GetOrCreateStaticFieldIds(linked_class).IsNull() ||
- art::mirror::Class::GetOrCreateMethodIds(linked_class).IsNull()) {
+ if (!art::mirror::Class::EnsureInstanceFieldIds(linked_class) ||
+ !art::mirror::Class::EnsureStaticFieldIds(linked_class) ||
+ !art::mirror::Class::EnsureMethodIds(linked_class)) {
driver_->self_->AssertPendingOOMException();
driver_->self_->ClearException();
RecordFailure(
diff --git a/runtime/jni/jni_id_manager.cc b/runtime/jni/jni_id_manager.cc
index 4b6335b..8070505 100644
--- a/runtime/jni/jni_id_manager.cc
+++ b/runtime/jni/jni_id_manager.cc
@@ -33,6 +33,7 @@
#include "jni_id_type.h"
#include "mirror/array-inl.h"
#include "mirror/array.h"
+#include "mirror/class-alloc-inl.h"
#include "mirror/class-inl.h"
#include "mirror/class.h"
#include "mirror/class_ext-inl.h"
@@ -65,13 +66,49 @@
template <typename ArtType>
ObjPtr<mirror::PointerArray> GetIds(ObjPtr<mirror::Class> k, ArtType* t)
REQUIRES_SHARED(Locks::mutator_lock_) {
+ ObjPtr<mirror::Object> ret;
if constexpr (std::is_same_v<ArtType, ArtField>) {
- return t->IsStatic() ? k->GetStaticFieldIds() : k->GetInstanceFieldIds();
+ ret = t->IsStatic() ? k->GetStaticFieldIds() : k->GetInstanceFieldIds();
} else {
- return t->IsObsolete() ? nullptr : k->GetMethodIds();
+ ret = t->IsObsolete() ? nullptr : k->GetMethodIds();
}
+ DCHECK(ret.IsNull() || ret->IsArrayInstance()) << "Should have bailed out early!";
+ if (kIsDebugBuild && !ret.IsNull()) {
+ if (kRuntimePointerSize == PointerSize::k32) {
+ CHECK(ret->IsIntArray());
+ } else {
+ CHECK(ret->IsLongArray());
+ }
+ }
+ return down_cast<mirror::PointerArray*>(ret.Ptr());
}
+template <typename ArtType>
+bool ShouldReturnPointer(ObjPtr<mirror::Class> klass, ArtType* t)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
+template <>
+bool ShouldReturnPointer(ObjPtr<mirror::Class> klass, ArtMethod* t ATTRIBUTE_UNUSED) {
+ ObjPtr<mirror::ClassExt> ext(klass->GetExtData());
+ if (ext.IsNull()) {
+ return true;
+ }
+ ObjPtr<mirror::Object> arr = ext->GetJMethodIDs();
+ return arr.IsNull() || !arr->IsArrayInstance();
+}
+
+template<>
+bool ShouldReturnPointer(ObjPtr<mirror::Class> klass, ArtField* t) {
+ ObjPtr<mirror::ClassExt> ext(klass->GetExtData());
+ if (ext.IsNull()) {
+ return true;
+ }
+ ObjPtr<mirror::Object> arr = t->IsStatic() ? ext->GetStaticJFieldIDs()
+ : ext->GetInstanceJFieldIDs();
+ return arr.IsNull() || !arr->IsArrayInstance();
+}
+
+
// Forces the appropriate id array to be present if possible. Returns true if allocation was
// attempted but failed.
template <typename ArtType>
@@ -87,8 +124,8 @@
return false;
} else {
// NB This modifies the class to allocate the ClassExt and the ids array.
- field->IsStatic() ? mirror::Class::GetOrCreateStaticFieldIds(h_k)
- : mirror::Class::GetOrCreateInstanceFieldIds(h_k);
+ field->IsStatic() ? mirror::Class::EnsureStaticFieldIds(h_k)
+ : mirror::Class::EnsureInstanceFieldIds(h_k);
}
if (self->IsExceptionPending()) {
self->AssertPendingOOMException();
@@ -113,7 +150,7 @@
return false;
} else {
// NB This modifies the class to allocate the ClassExt and the ids array.
- mirror::Class::GetOrCreateMethodIds(h_k);
+ mirror::Class::EnsureMethodIds(h_k);
}
if (self->IsExceptionPending()) {
self->AssertPendingOOMException();
@@ -187,29 +224,21 @@
// and not a pointer. This gives us 2**31 unique methods that can be addressed on 32-bit art, which
// should be more than enough.
template <>
-uintptr_t JniIdManager::GetNextId<ArtField>(JniIdType type, ReflectiveHandle<ArtField> f) {
- if (LIKELY(type == JniIdType::kIndices)) {
- uintptr_t res = next_field_id_;
- next_field_id_ += 2;
- CHECK_GT(next_field_id_, res) << "jfieldID Overflow";
- return res;
- } else {
- DCHECK_EQ(type, JniIdType::kSwapablePointer);
- return reinterpret_cast<uintptr_t>(f.Get());
- }
+uintptr_t JniIdManager::GetNextId<ArtField>(JniIdType type) {
+ DCHECK_EQ(type, JniIdType::kIndices);
+ uintptr_t res = next_field_id_;
+ next_field_id_ += 2;
+ CHECK_GT(next_field_id_, res) << "jfieldID Overflow";
+ return res;
}
template <>
-uintptr_t JniIdManager::GetNextId<ArtMethod>(JniIdType type, ReflectiveHandle<ArtMethod> m) {
- if (LIKELY(type == JniIdType::kIndices)) {
- uintptr_t res = next_method_id_;
- next_method_id_ += 2;
- CHECK_GT(next_method_id_, res) << "jmethodID Overflow";
- return res;
- } else {
- DCHECK_EQ(type, JniIdType::kSwapablePointer);
- return reinterpret_cast<uintptr_t>(m.Get());
- }
+uintptr_t JniIdManager::GetNextId<ArtMethod>(JniIdType type) {
+ DCHECK_EQ(type, JniIdType::kIndices);
+ uintptr_t res = next_method_id_;
+ next_method_id_ += 2;
+ CHECK_GT(next_method_id_, res) << "jmethodID Overflow";
+ return res;
}
template <>
std::vector<ArtField*>& JniIdManager::GetGenericMap<ArtField>() {
@@ -247,18 +276,19 @@
}
Thread* self = Thread::Current();
ScopedExceptionStorage ses(self);
- ObjPtr<mirror::Class> klass = t->GetDeclaringClass();
- DCHECK(!klass.IsNull()) << "Null declaring class " << PrettyGeneric(t);
- size_t off = GetIdOffset(klass, Canonicalize(t), kRuntimePointerSize);
+ DCHECK(!t->GetDeclaringClass().IsNull()) << "Null declaring class " << PrettyGeneric(t);
+ size_t off = GetIdOffset(t->GetDeclaringClass(), Canonicalize(t), kRuntimePointerSize);
// Here is the earliest point we can suspend.
- bool allocation_failure = EnsureIdsArray(self, klass, t.Get());
- klass = t->GetDeclaringClass();
- ObjPtr<mirror::PointerArray> ids(GetIds(klass, t.Get()));
+ bool allocation_failure = EnsureIdsArray(self, t->GetDeclaringClass(), t.Get());
if (allocation_failure) {
self->AssertPendingOOMException();
ses.SuppressOldException("OOM exception while trying to allocate JNI ids.");
return 0u;
+ } else if (ShouldReturnPointer(t->GetDeclaringClass(), t.Get())) {
+ return reinterpret_cast<uintptr_t>(t.Get());
}
+ ObjPtr<mirror::Class> klass = t->GetDeclaringClass();
+ ObjPtr<mirror::PointerArray> ids(GetIds(klass, t.Get()));
uintptr_t cur_id = 0;
if (!ids.IsNull()) {
DCHECK_GT(ids->GetLength(), static_cast<int32_t>(off)) << " is " << PrettyGeneric(t);
@@ -315,18 +345,13 @@
return IndexToId(index);
}
}
- cur_id = GetNextId(id_type, t);
- if (UNLIKELY(id_type == JniIdType::kIndices)) {
- DCHECK_EQ(cur_id % 2, 1u);
- size_t cur_index = IdToIndex(cur_id);
- std::vector<ArtType*>& vec = GetGenericMap<ArtType>();
- vec.reserve(cur_index + 1);
- vec.resize(std::max(vec.size(), cur_index + 1), nullptr);
- vec[cur_index] = t.Get();
- } else {
- DCHECK_EQ(cur_id % 2, 0u);
- DCHECK_EQ(cur_id, reinterpret_cast<uintptr_t>(t.Get()));
- }
+ cur_id = GetNextId<ArtType>(id_type);
+ DCHECK_EQ(cur_id % 2, 1u);
+ size_t cur_index = IdToIndex(cur_id);
+ std::vector<ArtType*>& vec = GetGenericMap<ArtType>();
+ vec.reserve(cur_index + 1);
+ vec.resize(std::max(vec.size(), cur_index + 1), nullptr);
+ vec[cur_index] = t.Get();
if (ids.IsNull()) {
if (kIsDebugBuild && !IsObsolete(t)) {
CHECK_NE(deferred_allocation_refcount_, 0u)
@@ -365,6 +390,29 @@
return res;
}
+void JniIdManager::VisitRoots(RootVisitor *visitor) {
+ pointer_marker_.VisitRootIfNonNull(visitor, RootInfo(kRootVMInternal));
+}
+
+void JniIdManager::Init(Thread* self) {
+ // When compiling we don't want to have anything to do with any of this, which is fine since JNI
+ // ids won't be created during AOT compilation. This also means we don't need to do any
+ // complicated stuff with the image-writer.
+ if (!Runtime::Current()->IsAotCompiler()) {
+ // Allocate the marker
+ StackHandleScope<3> hs(self);
+ Handle<mirror::Object> marker_obj(
+ hs.NewHandle(GetClassRoot<mirror::Object>()->AllocObject(self)));
+ CHECK(!marker_obj.IsNull());
+ pointer_marker_ = GcRoot<mirror::Object>(marker_obj.Get());
+ // Manually mark class-ext as having all pointer-ids to avoid any annoying loops.
+ Handle<mirror::Class> class_ext_class(hs.NewHandle(GetClassRoot<mirror::ClassExt>()));
+ mirror::Class::EnsureExtDataPresent(class_ext_class, self);
+ Handle<mirror::ClassExt> class_ext_ext(hs.NewHandle(class_ext_class->GetExtData()));
+ class_ext_ext->SetIdsArraysForClassExtExtData(marker_obj.Get());
+ }
+}
+
void JniIdManager::VisitReflectiveTargets(ReflectiveValueVisitor* rvv) {
art::WriterMutexLock mu(Thread::Current(), *Locks::jni_id_lock_);
for (auto it = field_id_map_.begin(); it != field_id_map_.end(); ++it) {
@@ -379,32 +427,40 @@
ObjPtr<mirror::ClassExt> old_ext_data(old_class->GetExtData());
ObjPtr<mirror::ClassExt> new_ext_data(new_class->GetExtData());
if (!old_ext_data.IsNull()) {
+ CHECK(!old_ext_data->HasInstanceFieldPointerIdMarker() &&
+ !old_ext_data->HasStaticFieldPointerIdMarker())
+ << old_class->PrettyClass();
// Clear the old field mapping.
if (old_field->IsStatic()) {
size_t old_off = ArraySlice<ArtField>(old_class->GetSFieldsPtr()).OffsetOf(old_field);
- ObjPtr<mirror::PointerArray> old_statics(old_ext_data->GetStaticJFieldIDs());
+ ObjPtr<mirror::PointerArray> old_statics(old_ext_data->GetStaticJFieldIDsPointerArray());
if (!old_statics.IsNull()) {
old_statics->SetElementPtrSize(old_off, 0, kRuntimePointerSize);
}
} else {
size_t old_off = ArraySlice<ArtField>(old_class->GetIFieldsPtr()).OffsetOf(old_field);
- ObjPtr<mirror::PointerArray> old_instances(old_ext_data->GetInstanceJFieldIDs());
+ ObjPtr<mirror::PointerArray> old_instances(
+ old_ext_data->GetInstanceJFieldIDsPointerArray());
if (!old_instances.IsNull()) {
old_instances->SetElementPtrSize(old_off, 0, kRuntimePointerSize);
}
}
}
if (!new_ext_data.IsNull()) {
+ CHECK(!new_ext_data->HasInstanceFieldPointerIdMarker() &&
+ !new_ext_data->HasStaticFieldPointerIdMarker())
+ << new_class->PrettyClass();
// Set the new field mapping.
if (new_field->IsStatic()) {
size_t new_off = ArraySlice<ArtField>(new_class->GetSFieldsPtr()).OffsetOf(new_field);
- ObjPtr<mirror::PointerArray> new_statics(new_ext_data->GetStaticJFieldIDs());
+ ObjPtr<mirror::PointerArray> new_statics(new_ext_data->GetStaticJFieldIDsPointerArray());
if (!new_statics.IsNull()) {
new_statics->SetElementPtrSize(new_off, id, kRuntimePointerSize);
}
} else {
size_t new_off = ArraySlice<ArtField>(new_class->GetIFieldsPtr()).OffsetOf(new_field);
- ObjPtr<mirror::PointerArray> new_instances(new_ext_data->GetInstanceJFieldIDs());
+ ObjPtr<mirror::PointerArray> new_instances(
+ new_ext_data->GetInstanceJFieldIDsPointerArray());
if (!new_instances.IsNull()) {
new_instances->SetElementPtrSize(new_off, id, kRuntimePointerSize);
}
@@ -424,17 +480,19 @@
ObjPtr<mirror::ClassExt> old_ext_data(old_class->GetExtData());
ObjPtr<mirror::ClassExt> new_ext_data(new_class->GetExtData());
if (!old_ext_data.IsNull()) {
+ CHECK(!old_ext_data->HasMethodPointerIdMarker()) << old_class->PrettyClass();
// Clear the old method mapping.
size_t old_off = ArraySlice<ArtMethod>(old_class->GetMethodsPtr()).OffsetOf(old_method);
- ObjPtr<mirror::PointerArray> old_methods(old_ext_data->GetJMethodIDs());
+ ObjPtr<mirror::PointerArray> old_methods(old_ext_data->GetJMethodIDsPointerArray());
if (!old_methods.IsNull()) {
old_methods->SetElementPtrSize(old_off, 0, kRuntimePointerSize);
}
}
if (!new_ext_data.IsNull()) {
+ CHECK(!new_ext_data->HasMethodPointerIdMarker()) << new_class->PrettyClass();
// Set the new method mapping.
size_t new_off = ArraySlice<ArtMethod>(new_class->GetMethodsPtr()).OffsetOf(new_method);
- ObjPtr<mirror::PointerArray> new_methods(new_ext_data->GetJMethodIDs());
+ ObjPtr<mirror::PointerArray> new_methods(new_ext_data->GetJMethodIDsPointerArray());
if (!new_methods.IsNull()) {
new_methods->SetElementPtrSize(new_off, id, kRuntimePointerSize);
}
@@ -463,6 +521,10 @@
return DecodeGenericId<ArtField>(reinterpret_cast<uintptr_t>(field));
}
+ObjPtr<mirror::Object> JniIdManager::GetPointerMarker() {
+ return pointer_marker_.Read();
+}
+
// This whole defer system is an annoying requirement to allow us to generate IDs during heap-walks
// such as those required for instrumentation tooling.
//
diff --git a/runtime/jni/jni_id_manager.h b/runtime/jni/jni_id_manager.h
index 1cfcefb..c8ebfc3 100644
--- a/runtime/jni/jni_id_manager.h
+++ b/runtime/jni/jni_id_manager.h
@@ -25,10 +25,15 @@
#include "art_field.h"
#include "art_method.h"
#include "base/mutex.h"
+#include "gc_root.h"
#include "jni_id_type.h"
#include "reflective_value_visitor.h"
namespace art {
+namespace mirror {
+class Object;
+class ClassExt;
+} // namespace mirror
template<typename RT> class ReflectiveHandle;
namespace jni {
@@ -43,6 +48,8 @@
return val == nullptr || reinterpret_cast<uintptr_t>(val) % 2 == 1;
}
+ void Init(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_);
+
ArtMethod* DecodeMethodId(jmethodID method) REQUIRES(!Locks::jni_id_lock_);
ArtField* DecodeFieldId(jfieldID field) REQUIRES(!Locks::jni_id_lock_);
jmethodID EncodeMethodId(ReflectiveHandle<ArtMethod> method) REQUIRES(!Locks::jni_id_lock_)
@@ -57,6 +64,10 @@
void VisitReflectiveTargets(ReflectiveValueVisitor* rvv)
REQUIRES(Locks::mutator_lock_, !Locks::jni_id_lock_);
+ void VisitRoots(RootVisitor* visitor) REQUIRES_SHARED(Locks::mutator_lock_);
+
+ ObjPtr<mirror::Object> GetPointerMarker() REQUIRES_SHARED(Locks::mutator_lock_);
+
private:
template <typename ArtType>
uintptr_t EncodeGenericId(ReflectiveHandle<ArtType> t) REQUIRES(!Locks::jni_id_lock_)
@@ -65,8 +76,7 @@
ArtType* DecodeGenericId(uintptr_t input) REQUIRES(!Locks::jni_id_lock_);
template <typename ArtType> std::vector<ArtType*>& GetGenericMap()
REQUIRES(Locks::jni_id_lock_);
- template <typename ArtType>
- uintptr_t GetNextId(JniIdType id, ReflectiveHandle<ArtType> t)
+ template <typename ArtType> uintptr_t GetNextId(JniIdType id)
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(Locks::jni_id_lock_);
template <typename ArtType>
@@ -92,7 +102,11 @@
// min jfieldID that might not have it's field->id mapping filled in.
uintptr_t deferred_allocation_field_id_start_ GUARDED_BY(Locks::jni_id_lock_) = 0u;
+ GcRoot<mirror::Object> pointer_marker_;
+
friend class ScopedEnableSuspendAllJniIdQueries;
+ // For GetPointerMarker
+ friend class mirror::ClassExt;
};
// A scope that will enable using the Encode/Decode JNI id functions with all threads suspended.
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index 70621ff..e57cc98 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -1708,7 +1708,7 @@
(new_access_flags & kAccVerificationAttempted) != 0);
}
-ObjPtr<PointerArray> Class::GetMethodIds() {
+ObjPtr<Object> Class::GetMethodIds() {
ObjPtr<ClassExt> ext(GetExtData());
if (ext.IsNull()) {
return nullptr;
@@ -1716,18 +1716,18 @@
return ext->GetJMethodIDs();
}
}
-ObjPtr<PointerArray> Class::GetOrCreateMethodIds(Handle<Class> h_this) {
+bool Class::EnsureMethodIds(Handle<Class> h_this) {
DCHECK_NE(Runtime::Current()->GetJniIdType(), JniIdType::kPointer) << "JNI Ids are pointers!";
Thread* self = Thread::Current();
ObjPtr<ClassExt> ext(EnsureExtDataPresent(h_this, self));
if (ext.IsNull()) {
self->AssertPendingOOMException();
- return nullptr;
+ return false;
}
return ext->EnsureJMethodIDsArrayPresent(h_this->NumMethods());
}
-ObjPtr<PointerArray> Class::GetStaticFieldIds() {
+ObjPtr<Object> Class::GetStaticFieldIds() {
ObjPtr<ClassExt> ext(GetExtData());
if (ext.IsNull()) {
return nullptr;
@@ -1735,17 +1735,17 @@
return ext->GetStaticJFieldIDs();
}
}
-ObjPtr<PointerArray> Class::GetOrCreateStaticFieldIds(Handle<Class> h_this) {
+bool Class::EnsureStaticFieldIds(Handle<Class> h_this) {
DCHECK_NE(Runtime::Current()->GetJniIdType(), JniIdType::kPointer) << "JNI Ids are pointers!";
Thread* self = Thread::Current();
ObjPtr<ClassExt> ext(EnsureExtDataPresent(h_this, self));
if (ext.IsNull()) {
self->AssertPendingOOMException();
- return nullptr;
+ return false;
}
return ext->EnsureStaticJFieldIDsArrayPresent(h_this->NumStaticFields());
}
-ObjPtr<PointerArray> Class::GetInstanceFieldIds() {
+ObjPtr<Object> Class::GetInstanceFieldIds() {
ObjPtr<ClassExt> ext(GetExtData());
if (ext.IsNull()) {
return nullptr;
@@ -1753,13 +1753,13 @@
return ext->GetInstanceJFieldIDs();
}
}
-ObjPtr<PointerArray> Class::GetOrCreateInstanceFieldIds(Handle<Class> h_this) {
+bool Class::EnsureInstanceFieldIds(Handle<Class> h_this) {
DCHECK_NE(Runtime::Current()->GetJniIdType(), JniIdType::kPointer) << "JNI Ids are pointers!";
Thread* self = Thread::Current();
ObjPtr<ClassExt> ext(EnsureExtDataPresent(h_this, self));
if (ext.IsNull()) {
self->AssertPendingOOMException();
- return nullptr;
+ return false;
}
return ext->EnsureInstanceJFieldIDsArrayPresent(h_this->NumInstanceFields());
}
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index d37aa35..f5c2614 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -1286,15 +1286,15 @@
REQUIRES_SHARED(Locks::mutator_lock_);
// Get or create the various jni id arrays in a lock-less thread safe manner.
- static ObjPtr<PointerArray> GetOrCreateMethodIds(Handle<Class> h_this)
+ static bool EnsureMethodIds(Handle<Class> h_this)
REQUIRES_SHARED(Locks::mutator_lock_);
- ObjPtr<PointerArray> GetMethodIds() REQUIRES_SHARED(Locks::mutator_lock_);
- static ObjPtr<PointerArray> GetOrCreateStaticFieldIds(Handle<Class> h_this)
+ ObjPtr<Object> GetMethodIds() REQUIRES_SHARED(Locks::mutator_lock_);
+ static bool EnsureStaticFieldIds(Handle<Class> h_this)
REQUIRES_SHARED(Locks::mutator_lock_);
- ObjPtr<PointerArray> GetStaticFieldIds() REQUIRES_SHARED(Locks::mutator_lock_);
- static ObjPtr<PointerArray> GetOrCreateInstanceFieldIds(Handle<Class> h_this)
+ ObjPtr<Object> GetStaticFieldIds() REQUIRES_SHARED(Locks::mutator_lock_);
+ static bool EnsureInstanceFieldIds(Handle<Class> h_this)
REQUIRES_SHARED(Locks::mutator_lock_);
- ObjPtr<PointerArray> GetInstanceFieldIds() REQUIRES_SHARED(Locks::mutator_lock_);
+ ObjPtr<Object> GetInstanceFieldIds() REQUIRES_SHARED(Locks::mutator_lock_);
// Calculate the index in the ifields_, methods_ or sfields_ arrays a method is located at. This
// is to be used with the above Get{,OrCreate}...Ids functions.
diff --git a/runtime/mirror/class_ext-inl.h b/runtime/mirror/class_ext-inl.h
index fd81a2a..99f7f49 100644
--- a/runtime/mirror/class_ext-inl.h
+++ b/runtime/mirror/class_ext-inl.h
@@ -22,30 +22,40 @@
#include "array-inl.h"
#include "art_method-inl.h"
#include "base/enums.h"
+#include "base/globals.h"
+#include "class_root.h"
#include "handle_scope.h"
+#include "jni/jni_internal.h"
+#include "jni_id_type.h"
+#include "mirror/array.h"
#include "mirror/object.h"
#include "object-inl.h"
#include "verify_object.h"
+#include "well_known_classes.h"
namespace art {
namespace mirror {
template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
-inline ObjPtr<PointerArray> ClassExt::EnsureJniIdsArrayPresent(MemberOffset off, size_t count) {
- ObjPtr<PointerArray> existing(
- GetFieldObject<PointerArray, kVerifyFlags, kReadBarrierOption>(off));
+inline bool ClassExt::EnsureJniIdsArrayPresent(MemberOffset off, size_t count) {
+ ObjPtr<Object> existing(
+ GetFieldObject<Object, kVerifyFlags, kReadBarrierOption>(off));
if (!existing.IsNull()) {
- return existing;
+ return true;
}
Thread* self = Thread::Current();
StackHandleScope<2> hs(self);
Handle<ClassExt> h_this(hs.NewHandle(this));
- Handle<PointerArray> new_arr(
- hs.NewHandle(Runtime::Current()->GetClassLinker()->AllocPointerArray(self, count)));
+ MutableHandle<Object> new_arr(hs.NewHandle<Object>(nullptr));
+ if (UNLIKELY(Runtime::Current()->GetJniIdType() == JniIdType::kSwapablePointer)) {
+ new_arr.Assign(Runtime::Current()->GetJniIdManager()->GetPointerMarker());
+ } else {
+ new_arr.Assign(Runtime::Current()->GetClassLinker()->AllocPointerArray(self, count));
+ }
if (new_arr.IsNull()) {
// Fail.
self->AssertPendingOOMException();
- return nullptr;
+ return false;
}
bool set;
// Set the ext_data_ field using CAS semantics.
@@ -56,40 +66,62 @@
set = h_this->CasFieldObject<false>(
off, nullptr, new_arr.Get(), CASMode::kStrong, std::memory_order_seq_cst);
}
- ObjPtr<PointerArray> ret(
- set ? new_arr.Get()
- : h_this->GetFieldObject<PointerArray, kVerifyFlags, kReadBarrierOption>(off));
- CHECK(!ret.IsNull());
- return ret;
+ if (kIsDebugBuild) {
+ ObjPtr<Object> ret(
+ set ? new_arr.Get()
+ : h_this->GetFieldObject<PointerArray, kVerifyFlags, kReadBarrierOption>(off));
+ CHECK(!ret.IsNull());
+ }
+ return true;
}
template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
-inline ObjPtr<PointerArray> ClassExt::EnsureJMethodIDsArrayPresent(size_t count) {
+inline bool ClassExt::EnsureJMethodIDsArrayPresent(size_t count) {
return EnsureJniIdsArrayPresent<kVerifyFlags, kReadBarrierOption>(
MemberOffset(OFFSET_OF_OBJECT_MEMBER(ClassExt, jmethod_ids_)), count);
}
template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
-inline ObjPtr<PointerArray> ClassExt::EnsureStaticJFieldIDsArrayPresent(size_t count) {
+inline bool ClassExt::EnsureStaticJFieldIDsArrayPresent(size_t count) {
return EnsureJniIdsArrayPresent<kVerifyFlags, kReadBarrierOption>(
MemberOffset(OFFSET_OF_OBJECT_MEMBER(ClassExt, static_jfield_ids_)), count);
}
template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
-inline ObjPtr<PointerArray> ClassExt::EnsureInstanceJFieldIDsArrayPresent(size_t count) {
+inline bool ClassExt::EnsureInstanceJFieldIDsArrayPresent(size_t count) {
return EnsureJniIdsArrayPresent<kVerifyFlags, kReadBarrierOption>(
MemberOffset(OFFSET_OF_OBJECT_MEMBER(ClassExt, instance_jfield_ids_)), count);
}
template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
-inline ObjPtr<PointerArray> ClassExt::GetInstanceJFieldIDs() {
- return GetFieldObject<PointerArray, kVerifyFlags, kReadBarrierOption>(
+inline ObjPtr<Object> ClassExt::GetInstanceJFieldIDs() {
+ return GetFieldObject<Object, kVerifyFlags, kReadBarrierOption>(
OFFSET_OF_OBJECT_MEMBER(ClassExt, instance_jfield_ids_));
}
+template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+inline bool ClassExt::HasInstanceFieldPointerIdMarker() {
+ ObjPtr<Object> arr(GetInstanceJFieldIDs<kVerifyFlags, kReadBarrierOption>());
+ return !arr.IsNull() && !arr->IsArrayInstance();
+}
+template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+inline ObjPtr<PointerArray> ClassExt::GetInstanceJFieldIDsPointerArray() {
+ DCHECK(!HasInstanceFieldPointerIdMarker());
+ return down_cast<PointerArray*>(GetInstanceJFieldIDs<kVerifyFlags, kReadBarrierOption>().Ptr());
+}
template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
-inline ObjPtr<PointerArray> ClassExt::GetStaticJFieldIDs() {
- return GetFieldObject<PointerArray, kVerifyFlags, kReadBarrierOption>(
+inline ObjPtr<Object> ClassExt::GetStaticJFieldIDs() {
+ return GetFieldObject<Object, kVerifyFlags, kReadBarrierOption>(
OFFSET_OF_OBJECT_MEMBER(ClassExt, static_jfield_ids_));
}
+template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+inline ObjPtr<PointerArray> ClassExt::GetStaticJFieldIDsPointerArray() {
+ DCHECK(!HasStaticFieldPointerIdMarker());
+ return down_cast<PointerArray*>(GetStaticJFieldIDs<kVerifyFlags, kReadBarrierOption>().Ptr());
+}
+template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+inline bool ClassExt::HasStaticFieldPointerIdMarker() {
+ ObjPtr<Object> arr(GetStaticJFieldIDs<kVerifyFlags, kReadBarrierOption>());
+ return !arr.IsNull() && !arr->IsArrayInstance();
+}
template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
inline ObjPtr<Class> ClassExt::GetObsoleteClass() {
@@ -98,10 +130,21 @@
}
template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
-inline ObjPtr<PointerArray> ClassExt::GetJMethodIDs() {
- return GetFieldObject<PointerArray, kVerifyFlags, kReadBarrierOption>(
+inline ObjPtr<Object> ClassExt::GetJMethodIDs() {
+ return GetFieldObject<Object, kVerifyFlags, kReadBarrierOption>(
OFFSET_OF_OBJECT_MEMBER(ClassExt, jmethod_ids_));
}
+template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+inline ObjPtr<PointerArray> ClassExt::GetJMethodIDsPointerArray() {
+ DCHECK(!HasMethodPointerIdMarker());
+ return down_cast<PointerArray*>(GetJMethodIDs<kVerifyFlags, kReadBarrierOption>().Ptr());
+}
+template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+inline bool ClassExt::HasMethodPointerIdMarker() {
+ ObjPtr<Object> arr(GetJMethodIDs<kVerifyFlags, kReadBarrierOption>());
+ return !arr.IsNull() && !arr->IsArrayInstance();
+}
+
inline ObjPtr<Object> ClassExt::GetVerifyError() {
return GetFieldObject<ClassExt>(OFFSET_OF_OBJECT_MEMBER(ClassExt, verify_error_));
@@ -146,8 +189,9 @@
template<ReadBarrierOption kReadBarrierOption, class Visitor>
void ClassExt::VisitJMethodIDs(Visitor v) {
- ObjPtr<PointerArray> marr(GetJMethodIDs<kDefaultVerifyFlags, kReadBarrierOption>());
- if (!marr.IsNull()) {
+ ObjPtr<Object> arr(GetJMethodIDs<kDefaultVerifyFlags, kReadBarrierOption>());
+ if (!arr.IsNull() && arr->IsArrayInstance()) {
+ ObjPtr<PointerArray> marr(down_cast<PointerArray*>(arr.Ptr()));
int32_t len = marr->GetLength();
for (int32_t i = 0; i < len; i++) {
jmethodID id = marr->GetElementPtrSize<jmethodID>(i, kRuntimePointerSize);
@@ -159,8 +203,9 @@
}
template<ReadBarrierOption kReadBarrierOption, class Visitor>
void ClassExt::VisitJFieldIDs(Visitor v) {
- ObjPtr<PointerArray> sarr(GetStaticJFieldIDs<kDefaultVerifyFlags, kReadBarrierOption>());
- if (!sarr.IsNull()) {
+ ObjPtr<Object> sarr_obj(GetStaticJFieldIDs<kDefaultVerifyFlags, kReadBarrierOption>());
+ if (!sarr_obj.IsNull() && sarr_obj->IsArrayInstance()) {
+ ObjPtr<PointerArray> sarr(down_cast<PointerArray*>(sarr_obj->AsArray().Ptr()));
int32_t len = sarr->GetLength();
for (int32_t i = 0; i < len; i++) {
jfieldID id = sarr->GetElementPtrSize<jfieldID>(i, kRuntimePointerSize);
@@ -169,8 +214,9 @@
}
}
}
- ObjPtr<PointerArray> iarr(GetInstanceJFieldIDs<kDefaultVerifyFlags, kReadBarrierOption>());
- if (!iarr.IsNull()) {
+ ObjPtr<PointerArray> iarr_obj(GetInstanceJFieldIDs<kDefaultVerifyFlags, kReadBarrierOption>());
+ if (!iarr_obj.IsNull() && iarr_obj->IsArrayInstance()) {
+ ObjPtr<PointerArray> iarr(down_cast<PointerArray*>(iarr_obj->AsArray().Ptr()));
int32_t len = iarr->GetLength();
for (int32_t i = 0; i < len; i++) {
jfieldID id = iarr->GetElementPtrSize<jfieldID>(i, kRuntimePointerSize);
diff --git a/runtime/mirror/class_ext.cc b/runtime/mirror/class_ext.cc
index 27dcea8..ba1ae5f 100644
--- a/runtime/mirror/class_ext.cc
+++ b/runtime/mirror/class_ext.cc
@@ -51,6 +51,13 @@
SetFieldObject<false>(obsolete_methods_off, methods);
}
+void ClassExt::SetIdsArraysForClassExtExtData(ObjPtr<Object> marker) {
+ CHECK(!marker.IsNull());
+ SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(ClassExt, instance_jfield_ids_), marker);
+ SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(ClassExt, static_jfield_ids_), marker);
+ SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(ClassExt, jmethod_ids_), marker);
+}
+
// We really need to be careful how we update this. If we ever in the future make it so that
// these arrays are written into without all threads being suspended we have a race condition! This
// race could cause obsolete methods to be missed.
diff --git a/runtime/mirror/class_ext.h b/runtime/mirror/class_ext.h
index eb4047b..fa4e87a 100644
--- a/runtime/mirror/class_ext.h
+++ b/runtime/mirror/class_ext.h
@@ -48,30 +48,48 @@
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ObjPtr<PointerArray> EnsureInstanceJFieldIDsArrayPresent(size_t count)
+ bool EnsureInstanceJFieldIDsArrayPresent(size_t count)
REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ObjPtr<PointerArray> GetInstanceJFieldIDs() REQUIRES_SHARED(Locks::mutator_lock_);
+ ObjPtr<PointerArray> GetInstanceJFieldIDsPointerArray() REQUIRES_SHARED(Locks::mutator_lock_);
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
+ ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ ObjPtr<Object> GetInstanceJFieldIDs() REQUIRES_SHARED(Locks::mutator_lock_);
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
+ ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ bool HasInstanceFieldPointerIdMarker() REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ObjPtr<PointerArray> EnsureStaticJFieldIDsArrayPresent(size_t count)
+ bool EnsureStaticJFieldIDsArrayPresent(size_t count)
REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ObjPtr<PointerArray> GetStaticJFieldIDs() REQUIRES_SHARED(Locks::mutator_lock_);
+ ObjPtr<PointerArray> GetStaticJFieldIDsPointerArray() REQUIRES_SHARED(Locks::mutator_lock_);
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
+ ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ ObjPtr<Object> GetStaticJFieldIDs() REQUIRES_SHARED(Locks::mutator_lock_);
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
+ ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ bool HasStaticFieldPointerIdMarker() REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ObjPtr<PointerArray> EnsureJMethodIDsArrayPresent(size_t count)
+ bool EnsureJMethodIDsArrayPresent(size_t count)
REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ObjPtr<PointerArray> GetJMethodIDs() REQUIRES_SHARED(Locks::mutator_lock_);
+ ObjPtr<Object> GetJMethodIDs() REQUIRES_SHARED(Locks::mutator_lock_);
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
+ ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ ObjPtr<PointerArray> GetJMethodIDsPointerArray() REQUIRES_SHARED(Locks::mutator_lock_);
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
+ ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ bool HasMethodPointerIdMarker() REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
@@ -79,6 +97,10 @@
ObjPtr<Object> GetOriginalDexFile() REQUIRES_SHARED(Locks::mutator_lock_);
+ // Used to manually initialize the ext-ids arrays for the ClassExt associated
+ // with the Class<ClassExt>. This simplifies the id allocation path.
+ void SetIdsArraysForClassExtExtData(ObjPtr<Object> marker) REQUIRES_SHARED(Locks::mutator_lock_);
+
void SetOriginalDexFile(ObjPtr<Object> bytes) REQUIRES_SHARED(Locks::mutator_lock_);
uint16_t GetPreRedefineClassDefIndex() REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -129,7 +151,7 @@
private:
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ObjPtr<PointerArray> EnsureJniIdsArrayPresent(MemberOffset off, size_t count)
+ bool EnsureJniIdsArrayPresent(MemberOffset off, size_t count)
REQUIRES_SHARED(Locks::mutator_lock_);
// Field order required by test "ValidateFieldOrderOfJavaCppUnionClasses".
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 135ee63..3c6e437 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1685,6 +1685,9 @@
"no stack trace available");
}
+ // Class-roots are setup, we can now finish initializing the JniIdManager.
+ GetJniIdManager()->Init(self);
+
// Runtime initialization is largely done now.
// We load plugins first since that can modify the runtime state slightly.
// Load all plugins
@@ -2168,6 +2171,7 @@
void Runtime::VisitConcurrentRoots(RootVisitor* visitor, VisitRootFlags flags) {
intern_table_->VisitRoots(visitor, flags);
class_linker_->VisitRoots(visitor, flags);
+ jni_id_manager_->VisitRoots(visitor);
heap_->VisitAllocationRecords(visitor);
if ((flags & kVisitRootFlagNewRoots) == 0) {
// Guaranteed to have no new roots in the constant roots.
diff --git a/test/1972-jni-id-swap-indices/src/Main.java b/test/1972-jni-id-swap-indices/src/Main.java
index f2bb1ab..eaf4126 100644
--- a/test/1972-jni-id-swap-indices/src/Main.java
+++ b/test/1972-jni-id-swap-indices/src/Main.java
@@ -19,9 +19,13 @@
public class Main {
public static final boolean PRINT = false;
- public static void doNothingPtr() {}
+ public static class PtrCls {
+ public static void doNothingPtr() {}
+ }
- public static void doNothingIdx() {}
+ public static class IdxCls {
+ public static void doNothingIdx() {}
+ }
public static void DbgPrint(String str) {
if (PRINT) {
@@ -29,14 +33,14 @@
}
}
- public static long GetId(String name) {
- return GetMethodId(true, Main.class, name, "()V");
+ public static long GetId(Class<?> k, String name) {
+ return GetMethodId(true, k, name, "()V");
}
public static void main(String[] args) {
System.loadLibrary(args[0]);
System.out.println("JNI Type is: " + GetJniType());
- long expect_ptr_id = GetId("doNothingPtr");
+ long expect_ptr_id = GetId(PtrCls.class, "doNothingPtr");
DbgPrint(String.format("expected_ptr_id is 0x%x", expect_ptr_id));
if (expect_ptr_id % 4 != 0) {
throw new Error("ID " + expect_ptr_id + " is not aligned!");
@@ -45,14 +49,14 @@
}
SetToIndexIds();
System.out.println("JNI Type is: " + GetJniType());
- long expect_idx_id = GetId("doNothingIdx");
+ long expect_idx_id = GetId(IdxCls.class, "doNothingIdx");
DbgPrint(String.format("expected_idx_id is 0x%x", expect_idx_id));
if (expect_idx_id % 2 != 1) {
throw new Error("ID " + expect_ptr_id + " is not odd!");
} else {
System.out.println("index ID looks like an index!");
}
- long again_ptr_id = GetId("doNothingPtr");
+ long again_ptr_id = GetId(PtrCls.class, "doNothingPtr");
if (expect_ptr_id != again_ptr_id) {
throw new Error(
"Got different id values for same method. " + expect_ptr_id + " vs " + again_ptr_id);
diff --git a/test/1973-jni-id-swap-pointer/src/Main.java b/test/1973-jni-id-swap-pointer/src/Main.java
index 755fbd5..654cfec 100644
--- a/test/1973-jni-id-swap-pointer/src/Main.java
+++ b/test/1973-jni-id-swap-pointer/src/Main.java
@@ -17,9 +17,13 @@
public class Main {
public static final boolean PRINT = false;
- public static void doNothingPtr() {}
+ public static class PtrCls {
+ public static void doNothingPtr() {}
+ }
- public static void doNothingIdx() {}
+ public static class IdxCls {
+ public static void doNothingIdx() {}
+ }
public static void DbgPrint(String str) {
if (PRINT) {
@@ -27,14 +31,14 @@
}
}
- public static long GetId(String name) {
- return GetMethodId(true, Main.class, name, "()V");
+ public static long GetId(Class<?> c, String name) {
+ return GetMethodId(true, c, name, "()V");
}
public static void main(String[] args) {
System.loadLibrary(args[0]);
System.out.println("JNI Type is: " + GetJniType());
- long expect_ptr_id = GetId("doNothingPtr");
+ long expect_ptr_id = GetId(PtrCls.class, "doNothingPtr");
DbgPrint(String.format("expected_ptr_id is 0x%x", expect_ptr_id));
if (expect_ptr_id % 4 != 0) {
throw new Error("ID " + expect_ptr_id + " is not aligned!");
@@ -43,14 +47,14 @@
}
SetToPointerIds();
System.out.println("JNI Type is: " + GetJniType());
- long expect_ptr_id2 = GetId("doNothingIdx");
+ long expect_ptr_id2 = GetId(IdxCls.class, "doNothingIdx");
DbgPrint(String.format("expected_ptr_id2 is 0x%x", expect_ptr_id2));
if (expect_ptr_id2 % 4 != 0) {
throw new Error("ID " + expect_ptr_id + " is not aligned!");
} else {
System.out.println("pointer2 ID looks like a pointer!");
}
- long again_ptr_id = GetId("doNothingPtr");
+ long again_ptr_id = GetId(PtrCls.class, "doNothingPtr");
if (expect_ptr_id != again_ptr_id) {
throw new Error(
"Got different id values for same method. " + expect_ptr_id + " vs " + again_ptr_id);