commit bc19b7559e887310664a4c86ad3eb3b47749b087
author Alex Light <allight@google.com> Mon Dec 02 18:54:13 2019 +0000
committer Treehugger Robot <treehugger-gerrit@google.com> Mon Dec 02 20:35:34 2019 +0000
tree 820961050c6904f89d9bfe67f8b936f2a5ec9d9c
parent 0a19e212e56fc6fe2809b58072ddcf4acfdc568d

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

openjdkjvmti/ti_redefine.cc

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(