Change indirect reference table to use ObjPtr

Bug: 31113334

Test: test-art-host

Change-Id: I340fdf430897ebd790ea4e35f94bcee776e98445

17 files changed, 121 insertions, 86 deletions

diff --git a/benchmark/jobject-benchmark/jobject_benchmark.cc b/benchmark/jobject-benchmark/jobject_benchmark.cc
index de43f73800..7e0a5362c9 100644
--- a/benchmark/jobject-benchmark/jobject_benchmark.cc
+++ b/benchmark/jobject-benchmark/jobject_benchmark.cc
@@ -29,7 +29,7 @@ extern "C" JNIEXPORT void JNICALL Java_JObjectBenchmark_timeAddRemoveLocal(
   ObjPtr<mirror::Object> obj = soa.Decode<mirror::Object>(jobj);
   CHECK(obj != nullptr);
   for (jint i = 0; i < reps; ++i) {
-    jobject ref = soa.Env()->AddLocalReference<jobject>(obj.Ptr());
+    jobject ref = soa.Env()->AddLocalReference<jobject>(obj);
     soa.Env()->DeleteLocalRef(ref);
   }
 }
@@ -39,7 +39,7 @@ extern "C" JNIEXPORT void JNICALL Java_JObjectBenchmark_timeDecodeLocal(
   ScopedObjectAccess soa(env);
   ObjPtr<mirror::Object> obj = soa.Decode<mirror::Object>(jobj);
   CHECK(obj != nullptr);
-  jobject ref = soa.Env()->AddLocalReference<jobject>(obj.Ptr());
+  jobject ref = soa.Env()->AddLocalReference<jobject>(obj);
   for (jint i = 0; i < reps; ++i) {
     CHECK_EQ(soa.Decode<mirror::Object>(ref), obj);
   }
diff --git a/runtime/base/dumpable-inl.h b/runtime/base/dumpable-inl.h
new file mode 100644
index 0000000000..2cdf083f01
--- /dev/null
+++ b/runtime/base/dumpable-inl.h
@@ -0,0 +1,35 @@
+/*
+ * 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.
+ */
+
+#ifndef ART_RUNTIME_BASE_DUMPABLE_INL_H_
+#define ART_RUNTIME_BASE_DUMPABLE_INL_H_
+
+#include "base/dumpable.h"
+#include "base/mutex.h"
+#include "thread-inl.h"
+
+namespace art {
+
+template<typename T>
+inline std::ostream& operator<<(std::ostream& os, const MutatorLockedDumpable<T>& rhs) {
+  Locks::mutator_lock_->AssertSharedHeld(Thread::Current());
+  rhs.Dump(os);
+  return os;
+}
+
+}  // namespace art
+
+#endif  // ART_RUNTIME_BASE_DUMPABLE_INL_H_
diff --git a/runtime/base/dumpable.h b/runtime/base/dumpable.h
index 9bc4089163..9ef8d69b48 100644
--- a/runtime/base/dumpable.h
+++ b/runtime/base/dumpable.h
@@ -20,6 +20,7 @@
 #include <ostream>
 
 #include "base/macros.h"
+#include "base/mutex.h"
 
 namespace art {
 
@@ -50,6 +51,27 @@ std::ostream& operator<<(std::ostream& os, const Dumpable<T>& rhs) {
   return os;
 }
 
+template<typename T>
+class MutatorLockedDumpable {
+ public:
+  explicit MutatorLockedDumpable(T& value) REQUIRES_SHARED(Locks::mutator_lock_) : value_(value) {}
+
+  void Dump(std::ostream& os) const REQUIRES_SHARED(Locks::mutator_lock_) {
+    value_.Dump(os);
+  }
+
+ private:
+  const T& value_;
+
+  DISALLOW_COPY_AND_ASSIGN(MutatorLockedDumpable);
+};
+
+template<typename T>
+std::ostream& operator<<(std::ostream& os, const MutatorLockedDumpable<T>& rhs)
+  // TODO: should be REQUIRES_SHARED(Locks::mutator_lock_) however annotalysis
+  //       currently fails for this.
+    NO_THREAD_SAFETY_ANALYSIS;
+
 }  // namespace art
 
 #endif  // ART_RUNTIME_BASE_DUMPABLE_H_
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index bb62f2d55f..7f50eb8273 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -8270,7 +8270,7 @@ jobject ClassLinker::CreatePathClassLoader(Thread* self,
 
   // Make it a global ref and return.
   ScopedLocalRef<jobject> local_ref(
-      soa.Env(), soa.Env()->AddLocalReference<jobject>(h_path_class_loader.Get()));
+      soa.Env(), soa.Env()->AddLocalReference<jobject>(MakeObjPtr(h_path_class_loader.Get())));
   return soa.Env()->NewGlobalRef(local_ref.get());
 }
 
diff --git a/runtime/indirect_reference_table-inl.h b/runtime/indirect_reference_table-inl.h
index 5cc1de209d..e05f8f307c 100644
--- a/runtime/indirect_reference_table-inl.h
+++ b/runtime/indirect_reference_table-inl.h
@@ -19,7 +19,9 @@
 
 #include "indirect_reference_table.h"
 
+#include "base/dumpable.h"
 #include "gc_root-inl.h"
+#include "obj_ptr-inl.h"
 #include "runtime-inl.h"
 #include "verify_object-inl.h"
 
@@ -82,17 +84,17 @@ inline bool IndirectReferenceTable::CheckEntry(const char* what, IndirectRef ire
 }
 
 template<ReadBarrierOption kReadBarrierOption>
-inline mirror::Object* IndirectReferenceTable::Get(IndirectRef iref) const {
+inline ObjPtr<mirror::Object> IndirectReferenceTable::Get(IndirectRef iref) const {
   if (!GetChecked(iref)) {
     return nullptr;
   }
   uint32_t idx = ExtractIndex(iref);
-  mirror::Object* obj = table_[idx].GetReference()->Read<kReadBarrierOption>();
-  VerifyObject(obj);
+  ObjPtr<mirror::Object> obj = table_[idx].GetReference()->Read<kReadBarrierOption>();
+  VerifyObject(obj.Ptr());
   return obj;
 }
 
-inline void IndirectReferenceTable::Update(IndirectRef iref, mirror::Object* obj) {
+inline void IndirectReferenceTable::Update(IndirectRef iref, ObjPtr<mirror::Object> obj) {
   if (!GetChecked(iref)) {
     LOG(WARNING) << "IndirectReferenceTable Update failed to find reference " << iref;
     return;
@@ -101,6 +103,19 @@ inline void IndirectReferenceTable::Update(IndirectRef iref, mirror::Object* obj
   table_[idx].SetReference(obj);
 }
 
+inline void IrtEntry::Add(ObjPtr<mirror::Object> obj) {
+  ++serial_;
+  if (serial_ == kIRTPrevCount) {
+    serial_ = 0;
+  }
+  references_[serial_] = GcRoot<mirror::Object>(obj);
+}
+
+inline void IrtEntry::SetReference(ObjPtr<mirror::Object> obj) {
+  DCHECK_LT(serial_, kIRTPrevCount);
+  references_[serial_] = GcRoot<mirror::Object>(obj);
+}
+
 }  // namespace art
 
 #endif  // ART_RUNTIME_INDIRECT_REFERENCE_TABLE_INL_H_
diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc
index b742ccc4df..d59bb39ccf 100644
--- a/runtime/indirect_reference_table.cc
+++ b/runtime/indirect_reference_table.cc
@@ -16,6 +16,7 @@
 
 #include "indirect_reference_table-inl.h"
 
+#include "base/dumpable-inl.h"
 #include "base/systrace.h"
 #include "jni_internal.h"
 #include "nth_caller_visitor.h"
@@ -46,32 +47,6 @@ const char* GetIndirectRefKindString(const IndirectRefKind& kind) {
   return "IndirectRefKind Error";
 }
 
-template<typename T>
-class MutatorLockedDumpable {
- public:
-  explicit MutatorLockedDumpable(T& value)
-      REQUIRES_SHARED(Locks::mutator_lock_) : value_(value) {
-  }
-
-  void Dump(std::ostream& os) const REQUIRES_SHARED(Locks::mutator_lock_) {
-    value_.Dump(os);
-  }
-
- private:
-  T& value_;
-
-  DISALLOW_COPY_AND_ASSIGN(MutatorLockedDumpable);
-};
-
-template<typename T>
-std::ostream& operator<<(std::ostream& os, const MutatorLockedDumpable<T>& rhs)
-// TODO: should be REQUIRES_SHARED(Locks::mutator_lock_) however annotalysis
-//       currently fails for this.
-    NO_THREAD_SAFETY_ANALYSIS {
-  rhs.Dump(os);
-  return os;
-}
-
 void IndirectReferenceTable::AbortIfNoCheckJNI(const std::string& msg) {
   // If -Xcheck:jni is on, it'll give a more detailed error before aborting.
   JavaVMExt* vm = Runtime::Current()->GetJavaVM();
@@ -118,13 +93,13 @@ bool IndirectReferenceTable::IsValid() const {
   return table_mem_map_.get() != nullptr;
 }
 
-IndirectRef IndirectReferenceTable::Add(uint32_t cookie, mirror::Object* obj) {
+IndirectRef IndirectReferenceTable::Add(uint32_t cookie, ObjPtr<mirror::Object> obj) {
   IRTSegmentState prevState;
   prevState.all = cookie;
   size_t topIndex = segment_state_.parts.topIndex;
 
   CHECK(obj != nullptr);
-  VerifyObject(obj);
+  VerifyObject(obj.Ptr());
   DCHECK(table_ != nullptr);
   DCHECK_GE(segment_state_.parts.numHoles, prevState.parts.numHoles);
 
@@ -171,9 +146,9 @@ IndirectRef IndirectReferenceTable::Add(uint32_t cookie, mirror::Object* obj) {
 void IndirectReferenceTable::AssertEmpty() {
   for (size_t i = 0; i < Capacity(); ++i) {
     if (!table_[i].GetReference()->IsNull()) {
-      ScopedObjectAccess soa(Thread::Current());
       LOG(FATAL) << "Internal Error: non-empty local reference table\n"
                  << MutatorLockedDumpable<IndirectReferenceTable>(*this);
+      UNREACHABLE();
     }
   }
 }
@@ -299,7 +274,7 @@ void IndirectReferenceTable::Dump(std::ostream& os) const {
   os << kind_ << " table dump:\n";
   ReferenceTable::Table entries;
   for (size_t i = 0; i < Capacity(); ++i) {
-    mirror::Object* obj = table_[i].GetReference()->Read<kWithoutReadBarrier>();
+    ObjPtr<mirror::Object> obj = table_[i].GetReference()->Read<kWithoutReadBarrier>();
     if (obj != nullptr) {
       obj = table_[i].GetReference()->Read();
       entries.push_back(GcRoot<mirror::Object>(obj));
diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h
index e194f792b5..64de7a86ef 100644
--- a/runtime/indirect_reference_table.h
+++ b/runtime/indirect_reference_table.h
@@ -25,6 +25,7 @@
 #include "base/logging.h"
 #include "base/mutex.h"
 #include "gc_root.h"
+#include "obj_ptr.h"
 #include "object_callbacks.h"
 #include "offsets.h"
 #include "read_barrier_option.h"
@@ -200,24 +201,18 @@ union IRTSegmentState {
 static const size_t kIRTPrevCount = kIsDebugBuild ? 7 : 3;
 class IrtEntry {
  public:
-  void Add(mirror::Object* obj) REQUIRES_SHARED(Locks::mutator_lock_) {
-    ++serial_;
-    if (serial_ == kIRTPrevCount) {
-      serial_ = 0;
-    }
-    references_[serial_] = GcRoot<mirror::Object>(obj);
-  }
+  void Add(ObjPtr<mirror::Object> obj) REQUIRES_SHARED(Locks::mutator_lock_);
+
   GcRoot<mirror::Object>* GetReference() {
     DCHECK_LT(serial_, kIRTPrevCount);
     return &references_[serial_];
   }
+
   uint32_t GetSerial() const {
     return serial_;
   }
-  void SetReference(mirror::Object* obj) {
-    DCHECK_LT(serial_, kIRTPrevCount);
-    references_[serial_] = GcRoot<mirror::Object>(obj);
-  }
+
+  void SetReference(ObjPtr<mirror::Object> obj) REQUIRES_SHARED(Locks::mutator_lock_);
 
  private:
   uint32_t serial_;
@@ -237,7 +232,7 @@ class IrtIterator {
     return *this;
   }
 
-  GcRoot<mirror::Object>* operator*() {
+  GcRoot<mirror::Object>* operator*() REQUIRES_SHARED(Locks::mutator_lock_) {
     // This does not have a read barrier as this is used to visit roots.
     return table_[i_].GetReference();
   }
@@ -277,7 +272,7 @@ class IndirectReferenceTable {
    * Returns nullptr if the table is full (max entries reached, or alloc
    * failed during expansion).
    */
-  IndirectRef Add(uint32_t cookie, mirror::Object* obj)
+  IndirectRef Add(uint32_t cookie, ObjPtr<mirror::Object> obj)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
   /*
@@ -286,12 +281,13 @@ class IndirectReferenceTable {
    * Returns kInvalidIndirectRefObject if iref is invalid.
    */
   template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
-  mirror::Object* Get(IndirectRef iref) const REQUIRES_SHARED(Locks::mutator_lock_)
+  ObjPtr<mirror::Object> Get(IndirectRef iref) const REQUIRES_SHARED(Locks::mutator_lock_)
       ALWAYS_INLINE;
 
   // Synchronized get which reads a reference, acquiring a lock if necessary.
   template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
-  mirror::Object* SynchronizedGet(IndirectRef iref) const REQUIRES_SHARED(Locks::mutator_lock_) {
+  ObjPtr<mirror::Object> SynchronizedGet(IndirectRef iref) const
+      REQUIRES_SHARED(Locks::mutator_lock_) {
     return Get<kReadBarrierOption>(iref);
   }
 
@@ -300,7 +296,7 @@ class IndirectReferenceTable {
    *
    * Updates an existing indirect reference to point to a new object.
    */
-  void Update(IndirectRef iref, mirror::Object* obj) REQUIRES_SHARED(Locks::mutator_lock_);
+  void Update(IndirectRef iref, ObjPtr<mirror::Object> obj) REQUIRES_SHARED(Locks::mutator_lock_);
 
   /*
    * Remove an existing entry.
@@ -313,7 +309,7 @@ class IndirectReferenceTable {
    */
   bool Remove(uint32_t cookie, IndirectRef iref);
 
-  void AssertEmpty();
+  void AssertEmpty() REQUIRES_SHARED(Locks::mutator_lock_);
 
   void Dump(std::ostream& os) const REQUIRES_SHARED(Locks::mutator_lock_);
 
@@ -377,7 +373,7 @@ class IndirectReferenceTable {
   static void AbortIfNoCheckJNI(const std::string& msg);
 
   /* extra debugging checks */
-  bool GetChecked(IndirectRef) const;
+  bool GetChecked(IndirectRef) const REQUIRES_SHARED(Locks::mutator_lock_);
   bool CheckEntry(const char*, IndirectRef, int) const;
 
   /* semi-public - read/write by jni down calls */
diff --git a/runtime/indirect_reference_table_test.cc b/runtime/indirect_reference_table_test.cc
index 58d487d14a..0380f3ee7c 100644
--- a/runtime/indirect_reference_table_test.cc
+++ b/runtime/indirect_reference_table_test.cc
@@ -81,9 +81,9 @@ TEST_F(IndirectReferenceTableTest, BasicTest) {
   EXPECT_TRUE(iref2 != nullptr);
   CheckDump(&irt, 3, 3);
 
-  EXPECT_EQ(obj0, irt.Get(iref0));
-  EXPECT_EQ(obj1, irt.Get(iref1));
-  EXPECT_EQ(obj2, irt.Get(iref2));
+  EXPECT_OBJ_PTR_EQ(obj0, irt.Get(iref0));
+  EXPECT_OBJ_PTR_EQ(obj1, irt.Get(iref1));
+  EXPECT_OBJ_PTR_EQ(obj2, irt.Get(iref2));
 
   EXPECT_TRUE(irt.Remove(cookie, iref0));
   CheckDump(&irt, 2, 2);
diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc
index 89cbbe6c0e..ac5401f5d9 100644
--- a/runtime/interpreter/unstarted_runtime.cc
+++ b/runtime/interpreter/unstarted_runtime.cc
@@ -1441,15 +1441,15 @@ void UnstartedRuntime::UnstartedMethodInvoke(
   JNIEnvExt* env = self->GetJniEnv();
   ScopedObjectAccessUnchecked soa(self);
 
-  mirror::Object* java_method_obj = shadow_frame->GetVRegReference(arg_offset);
+  ObjPtr<mirror::Object> java_method_obj = shadow_frame->GetVRegReference(arg_offset);
   ScopedLocalRef<jobject> java_method(env,
       java_method_obj == nullptr ? nullptr :env->AddLocalReference<jobject>(java_method_obj));
 
-  mirror::Object* java_receiver_obj = shadow_frame->GetVRegReference(arg_offset + 1);
+  ObjPtr<mirror::Object> java_receiver_obj = shadow_frame->GetVRegReference(arg_offset + 1);
   ScopedLocalRef<jobject> java_receiver(env,
       java_receiver_obj == nullptr ? nullptr : env->AddLocalReference<jobject>(java_receiver_obj));
 
-  mirror::Object* java_args_obj = shadow_frame->GetVRegReference(arg_offset + 2);
+  ObjPtr<mirror::Object> java_args_obj = shadow_frame->GetVRegReference(arg_offset + 2);
   ScopedLocalRef<jobject> java_args(env,
       java_args_obj == nullptr ? nullptr : env->AddLocalReference<jobject>(java_args_obj));
 
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index 215f2b3ddf..ecd6b524df 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -641,7 +641,7 @@ void JavaVMExt::BroadcastForNewWeakGlobals() {
 }
 
 mirror::Object* JavaVMExt::DecodeGlobal(IndirectRef ref) {
-  return globals_.SynchronizedGet(ref);
+  return globals_.SynchronizedGet(ref).Ptr();
 }
 
 void JavaVMExt::UpdateGlobal(Thread* self, IndirectRef ref, mirror::Object* result) {
@@ -669,7 +669,7 @@ mirror::Object* JavaVMExt::DecodeWeakGlobal(Thread* self, IndirectRef ref) {
   // if MayAccessWeakGlobals is false.
   DCHECK_EQ(GetIndirectRefKind(ref), kWeakGlobal);
   if (LIKELY(MayAccessWeakGlobalsUnlocked(self))) {
-    return weak_globals_.SynchronizedGet(ref);
+    return weak_globals_.SynchronizedGet(ref).Ptr();
   }
   MutexLock mu(self, weak_globals_lock_);
   return DecodeWeakGlobalLocked(self, ref);
@@ -682,7 +682,7 @@ mirror::Object* JavaVMExt::DecodeWeakGlobalLocked(Thread* self, IndirectRef ref)
   while (UNLIKELY(!MayAccessWeakGlobals(self))) {
     weak_globals_add_condition_.WaitHoldingLocks(self);
   }
-  return weak_globals_.Get(ref);
+  return weak_globals_.Get(ref).Ptr();
 }
 
 mirror::Object* JavaVMExt::DecodeWeakGlobalDuringShutdown(Thread* self, IndirectRef ref) {
@@ -695,7 +695,7 @@ mirror::Object* JavaVMExt::DecodeWeakGlobalDuringShutdown(Thread* self, Indirect
   if (!kUseReadBarrier) {
     DCHECK(allow_accessing_weak_globals_.LoadSequentiallyConsistent());
   }
-  return weak_globals_.SynchronizedGet(ref);
+  return weak_globals_.SynchronizedGet(ref).Ptr();
 }
 
 bool JavaVMExt::IsWeakGlobalCleared(Thread* self, IndirectRef ref) {
diff --git a/runtime/jni_env_ext-inl.h b/runtime/jni_env_ext-inl.h
index 685b056e8d..2cc7342ff8 100644
--- a/runtime/jni_env_ext-inl.h
+++ b/runtime/jni_env_ext-inl.h
@@ -19,12 +19,14 @@
 
 #include "jni_env_ext.h"
 
+#include "indirect_reference_table-inl.h"
+#include "obj_ptr-inl.h"
 #include "utils.h"
 
 namespace art {
 
 template<typename T>
-inline T JNIEnvExt::AddLocalReference(mirror::Object* obj) {
+inline T JNIEnvExt::AddLocalReference(ObjPtr<mirror::Object> obj) {
   IndirectRef ref = locals.Add(local_ref_cookie, obj);
 
   // TODO: fix this to understand PushLocalFrame, so we can turn it on.
diff --git a/runtime/jni_env_ext.h b/runtime/jni_env_ext.h
index 79dfb0d41d..121f84854b 100644
--- a/runtime/jni_env_ext.h
+++ b/runtime/jni_env_ext.h
@@ -47,8 +47,7 @@ struct JNIEnvExt : public JNIEnv {
   void PopFrame() REQUIRES_SHARED(Locks::mutator_lock_);
 
   template<typename T>
-  T AddLocalReference(mirror::Object* obj)
-      REQUIRES_SHARED(Locks::mutator_lock_);
+  T AddLocalReference(ObjPtr<mirror::Object> obj) REQUIRES_SHARED(Locks::mutator_lock_);
 
   static Offset SegmentStateOffset(size_t pointer_size);
   static Offset LocalRefCookieOffset(size_t pointer_size);
diff --git a/runtime/runtime-inl.h b/runtime/runtime-inl.h
index 2eb0bf75a1..8346550c93 100644
--- a/runtime/runtime-inl.h
+++ b/runtime/runtime-inl.h
@@ -21,11 +21,12 @@
 
 #include "art_method.h"
 #include "class_linker.h"
+#include "obj_ptr-inl.h"
 #include "read_barrier-inl.h"
 
 namespace art {
 
-inline bool Runtime::IsClearedJniWeakGlobal(mirror::Object* obj) {
+inline bool Runtime::IsClearedJniWeakGlobal(ObjPtr<mirror::Object> obj) {
   return obj == GetClearedJniWeakGlobal();
 }
 
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 84c6b6f247..5a95f78cad 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -33,6 +33,7 @@
 #include "instrumentation.h"
 #include "jobject_comparator.h"
 #include "method_reference.h"
+#include "obj_ptr.h"
 #include "object_callbacks.h"
 #include "offsets.h"
 #include "process_state.h"
@@ -292,7 +293,7 @@ class Runtime {
   }
 
   // Is the given object the special object used to mark a cleared JNI weak global?
-  bool IsClearedJniWeakGlobal(mirror::Object* obj) REQUIRES_SHARED(Locks::mutator_lock_);
+  bool IsClearedJniWeakGlobal(ObjPtr<mirror::Object> obj) REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Get the special object used to mark a cleared JNI weak global.
   mirror::Object* GetClearedJniWeakGlobal() REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/scoped_thread_state_change-inl.h b/runtime/scoped_thread_state_change-inl.h
index 1d9f13246a..ac2575715e 100644
--- a/runtime/scoped_thread_state_change-inl.h
+++ b/runtime/scoped_thread_state_change-inl.h
@@ -72,19 +72,13 @@ inline ScopedThreadStateChange::~ScopedThreadStateChange() {
 }
 
 template<typename T>
-inline T ScopedObjectAccessAlreadyRunnable::AddLocalReference(mirror::Object* obj) const {
+inline T ScopedObjectAccessAlreadyRunnable::AddLocalReference(ObjPtr<mirror::Object> obj) const {
   Locks::mutator_lock_->AssertSharedHeld(Self());
   DCHECK(IsRunnable());  // Don't work with raw objects in non-runnable states.
   DCHECK_NE(obj, Runtime::Current()->GetClearedJniWeakGlobal());
   return obj == nullptr ? nullptr : Env()->AddLocalReference<T>(obj);
 }
 
-template<typename T, typename MirrorType, bool kPoison>
-inline T ScopedObjectAccessAlreadyRunnable::AddLocalReference(
-    ObjPtr<MirrorType, kPoison> obj) const {
-  return AddLocalReference<T>(obj.Ptr());
-}
-
 template<typename T, bool kPoison>
 inline ObjPtr<T, kPoison> ScopedObjectAccessAlreadyRunnable::Decode(jobject obj) const {
   Locks::mutator_lock_->AssertSharedHeld(Self());
diff --git a/runtime/scoped_thread_state_change.h b/runtime/scoped_thread_state_change.h
index 175bec51d6..04fd9141ea 100644
--- a/runtime/scoped_thread_state_change.h
+++ b/runtime/scoped_thread_state_change.h
@@ -87,13 +87,8 @@ class ScopedObjectAccessAlreadyRunnable : public ValueObject {
    * This will be called on otherwise unreferenced objects. We cannot do GC allocations here, and
    * it's best if we don't grab a mutex.
    */
-  template<typename T, typename MirrorType, bool kPoison = kIsDebugBuild>
-  T AddLocalReference(ObjPtr<MirrorType, kPoison> obj) const
-      REQUIRES_SHARED(Locks::mutator_lock_);
-
-  // TODO: Delete
   template<typename T>
-  T AddLocalReference(mirror::Object* obj) const
+  T AddLocalReference(ObjPtr<mirror::Object> obj) const
       REQUIRES_SHARED(Locks::mutator_lock_);
 
   template<typename T, bool kPoison = kIsDebugBuild>
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 80542e8b55..3005f14503 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1854,7 +1854,7 @@ mirror::Object* Thread::DecodeJObject(jobject obj) const {
   }
   IndirectRef ref = reinterpret_cast<IndirectRef>(obj);
   IndirectRefKind kind = GetIndirectRefKind(ref);
-  mirror::Object* result;
+  ObjPtr<mirror::Object> result;
   bool expect_null = false;
   // The "kinds" below are sorted by the frequency we expect to encounter them.
   if (kind == kLocal) {
@@ -1867,7 +1867,7 @@ mirror::Object* Thread::DecodeJObject(jobject obj) const {
     if (LIKELY(HandleScopeContains(obj))) {
       // Read from handle scope.
       result = reinterpret_cast<StackReference<mirror::Object>*>(obj)->AsMirrorPtr();
-      VerifyObject(result);
+      VerifyObject(result.Ptr());
     } else {
       tlsPtr_.jni_env->vm->JniAbortF(nullptr, "use of invalid jobject %p", obj);
       expect_null = true;
@@ -1889,7 +1889,7 @@ mirror::Object* Thread::DecodeJObject(jobject obj) const {
     tlsPtr_.jni_env->vm->JniAbortF(nullptr, "use of deleted %s %p",
                                    ToStr<IndirectRefKind>(kind).c_str(), obj);
   }
-  return result;
+  return result.Ptr();
 }
 
 bool Thread::IsJWeakCleared(jweak obj) const {