Revert "Disable opaque JNI Ids for now."
We were incorrectly using a potentially out-of-date pointer to access
class object fields. This could cause segvs if the class object moves
during certain allocations relating to JNI id allocation.
This reverts commit 1be9c72dd49e7620dd76a8b9cef23d7cde0a6b01.
Reason for revert: Fixed issue with gcstress
Test: ./test.py --host --debuggable --gcstress
Bug: 134162467
Change-Id: I6ee7fc4485bbae6f0f1d4f4af0e8c2bc88bf4075
diff --git a/runtime/jni/jni_id_manager.cc b/runtime/jni/jni_id_manager.cc
index 16f4d5f..c28d813 100644
--- a/runtime/jni/jni_id_manager.cc
+++ b/runtime/jni/jni_id_manager.cc
@@ -69,11 +69,14 @@
ArtField* field,
/*out*/bool* allocation_failure) {
ScopedExceptionStorage ses(self);
+ StackHandleScope<1> hs(self);
+ Handle<mirror::Class> h_k(hs.NewHandle(k));
ObjPtr<mirror::PointerArray> res;
if (Locks::mutator_lock_->IsExclusiveHeld(self)) {
- res = field->IsStatic() ? k->GetStaticFieldIds() : k->GetInstanceFieldIds();
+ res = field->IsStatic() ? h_k->GetStaticFieldIds() : h_k->GetInstanceFieldIds();
} else {
- res = field->IsStatic() ? k->GetOrCreateStaticFieldIds() : k->GetOrCreateInstanceFieldIds();
+ res = field->IsStatic() ? mirror::Class::GetOrCreateStaticFieldIds(h_k)
+ : mirror::Class::GetOrCreateInstanceFieldIds(h_k);
}
if (self->IsExceptionPending()) {
self->AssertPendingOOMException();
@@ -98,11 +101,13 @@
*allocation_failure = false;
return nullptr;
}
+ StackHandleScope<1> hs(self);
+ Handle<mirror::Class> h_k(hs.NewHandle(k));
ObjPtr<mirror::PointerArray> res;
if (Locks::mutator_lock_->IsExclusiveHeld(self) || !Locks::mutator_lock_->IsSharedHeld(self)) {
- res = k->GetMethodIds();
+ res = h_k->GetMethodIds();
} else {
- res = k->GetOrCreateMethodIds();
+ res = mirror::Class::GetOrCreateMethodIds(h_k);
}
if (self->IsExceptionPending()) {
self->AssertPendingOOMException();
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index c0a950d..23df946 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -1558,17 +1558,15 @@
return ext->GetJMethodIDs();
}
}
-ObjPtr<PointerArray> Class::GetOrCreateMethodIds() {
+ObjPtr<PointerArray> Class::GetOrCreateMethodIds(Handle<Class> h_this) {
DCHECK(Runtime::Current()->JniIdsAreIndices()) << "JNI Ids are pointers!";
Thread* self = Thread::Current();
- StackHandleScope<1> hs(self);
- Handle<Class> h_this(hs.NewHandle(this));
ObjPtr<ClassExt> ext(EnsureExtDataPresent(h_this, self));
if (ext.IsNull()) {
self->AssertPendingOOMException();
return nullptr;
}
- return ext->EnsureJMethodIDsArrayPresent(NumMethods());
+ return ext->EnsureJMethodIDsArrayPresent(h_this->NumMethods());
}
ObjPtr<PointerArray> Class::GetStaticFieldIds() {
@@ -1579,17 +1577,15 @@
return ext->GetStaticJFieldIDs();
}
}
-ObjPtr<PointerArray> Class::GetOrCreateStaticFieldIds() {
+ObjPtr<PointerArray> Class::GetOrCreateStaticFieldIds(Handle<Class> h_this) {
DCHECK(Runtime::Current()->JniIdsAreIndices()) << "JNI Ids are pointers!";
Thread* self = Thread::Current();
- StackHandleScope<1> hs(self);
- Handle<Class> h_this(hs.NewHandle(this));
ObjPtr<ClassExt> ext(EnsureExtDataPresent(h_this, self));
if (ext.IsNull()) {
self->AssertPendingOOMException();
return nullptr;
}
- return ext->EnsureStaticJFieldIDsArrayPresent(NumStaticFields());
+ return ext->EnsureStaticJFieldIDsArrayPresent(h_this->NumStaticFields());
}
ObjPtr<PointerArray> Class::GetInstanceFieldIds() {
ObjPtr<ClassExt> ext(GetExtData());
@@ -1599,17 +1595,15 @@
return ext->GetInstanceJFieldIDs();
}
}
-ObjPtr<PointerArray> Class::GetOrCreateInstanceFieldIds() {
+ObjPtr<PointerArray> Class::GetOrCreateInstanceFieldIds(Handle<Class> h_this) {
DCHECK(Runtime::Current()->JniIdsAreIndices()) << "JNI Ids are pointers!";
Thread* self = Thread::Current();
- StackHandleScope<1> hs(self);
- Handle<Class> h_this(hs.NewHandle(this));
ObjPtr<ClassExt> ext(EnsureExtDataPresent(h_this, self));
if (ext.IsNull()) {
self->AssertPendingOOMException();
return nullptr;
}
- return ext->EnsureInstanceJFieldIDsArrayPresent(NumInstanceFields());
+ return ext->EnsureInstanceJFieldIDsArrayPresent(h_this->NumInstanceFields());
}
size_t Class::GetStaticFieldIdOffset(ArtField* field) {
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 144350f..63d6b60 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -1230,11 +1230,14 @@
REQUIRES_SHARED(Locks::mutator_lock_);
// Get or create the various jni id arrays in a lock-less thread safe manner.
- ObjPtr<PointerArray> GetOrCreateMethodIds() REQUIRES_SHARED(Locks::mutator_lock_);
+ static ObjPtr<PointerArray> GetOrCreateMethodIds(Handle<Class> h_this)
+ REQUIRES_SHARED(Locks::mutator_lock_);
ObjPtr<PointerArray> GetMethodIds() REQUIRES_SHARED(Locks::mutator_lock_);
- ObjPtr<PointerArray> GetOrCreateStaticFieldIds() REQUIRES_SHARED(Locks::mutator_lock_);
+ static ObjPtr<PointerArray> GetOrCreateStaticFieldIds(Handle<Class> h_this)
+ REQUIRES_SHARED(Locks::mutator_lock_);
ObjPtr<PointerArray> GetStaticFieldIds() REQUIRES_SHARED(Locks::mutator_lock_);
- ObjPtr<PointerArray> GetOrCreateInstanceFieldIds() REQUIRES_SHARED(Locks::mutator_lock_);
+ static ObjPtr<PointerArray> GetOrCreateInstanceFieldIds(Handle<Class> h_this)
+ REQUIRES_SHARED(Locks::mutator_lock_);
ObjPtr<PointerArray> GetInstanceFieldIds() REQUIRES_SHARED(Locks::mutator_lock_);
// Calculate the index in the ifields_, methods_ or sfields_ arrays a method is located at. This
diff --git a/test/testrunner/testrunner.py b/test/testrunner/testrunner.py
index 4872226..379b2c6 100755
--- a/test/testrunner/testrunner.py
+++ b/test/testrunner/testrunner.py
@@ -435,7 +435,7 @@
options_test += ' --no-image'
if debuggable == 'debuggable':
- options_test += ' --debuggable'
+ options_test += ' --debuggable --runtime-option -Xopaque-jni-ids:true'
if jvmti == 'jvmti-stress':
options_test += ' --jvmti-trace-stress --jvmti-redefine-stress --jvmti-field-stress'