Merge "ART: Remove ObjPtr kPoison template parameter"

10 files changed, 106 insertions, 101 deletions

diff --git a/runtime/gc_root-inl.h b/runtime/gc_root-inl.h
index 390ed3c664..7795c661b6 100644
--- a/runtime/gc_root-inl.h
+++ b/runtime/gc_root-inl.h
@@ -38,7 +38,7 @@ inline GcRoot<MirrorType>::GcRoot(MirrorType* ref)
     : root_(mirror::CompressedReference<mirror::Object>::FromMirrorPtr(ref)) { }
 
 template<class MirrorType>
-inline GcRoot<MirrorType>::GcRoot(ObjPtr<MirrorType, kIsDebugBuild> ref)
+inline GcRoot<MirrorType>::GcRoot(ObjPtr<MirrorType> ref)
     : GcRoot(ref.Ptr()) { }
 
 inline std::string RootInfo::ToString() const {
diff --git a/runtime/gc_root.h b/runtime/gc_root.h
index 79e80f148c..0894e9bee5 100644
--- a/runtime/gc_root.h
+++ b/runtime/gc_root.h
@@ -24,7 +24,7 @@
 namespace art {
 class ArtField;
 class ArtMethod;
-template<class MirrorType, bool kPoison> class ObjPtr;
+template<class MirrorType> class ObjPtr;
 
 namespace mirror {
 class Object;
@@ -215,7 +215,7 @@ class GcRoot {
   ALWAYS_INLINE GcRoot() {}
   explicit ALWAYS_INLINE GcRoot(MirrorType* ref)
       REQUIRES_SHARED(Locks::mutator_lock_);
-  explicit ALWAYS_INLINE GcRoot(ObjPtr<MirrorType, kIsDebugBuild> ref)
+  explicit ALWAYS_INLINE GcRoot(ObjPtr<MirrorType> ref)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
  private:
diff --git a/runtime/handle_scope-inl.h b/runtime/handle_scope-inl.h
index 077f45e8f3..492d4b4bd9 100644
--- a/runtime/handle_scope-inl.h
+++ b/runtime/handle_scope-inl.h
@@ -114,9 +114,9 @@ inline MutableHandle<T> FixedSizeHandleScope<kNumReferences>::NewHandle(T* objec
   return h;
 }
 
-template<size_t kNumReferences> template<class MirrorType, bool kPoison>
+template<size_t kNumReferences> template<class MirrorType>
 inline MutableHandle<MirrorType> FixedSizeHandleScope<kNumReferences>::NewHandle(
-    ObjPtr<MirrorType, kPoison> object) {
+    ObjPtr<MirrorType> object) {
   return NewHandle(object.Ptr());
 }
 
@@ -191,9 +191,8 @@ MutableHandle<T> VariableSizedHandleScope::NewHandle(T* object) {
   return current_scope_->NewHandle(object);
 }
 
-template<class MirrorType, bool kPoison>
-inline MutableHandle<MirrorType> VariableSizedHandleScope::NewHandle(
-    ObjPtr<MirrorType, kPoison> ptr) {
+template<class MirrorType>
+inline MutableHandle<MirrorType> VariableSizedHandleScope::NewHandle(ObjPtr<MirrorType> ptr) {
   return NewHandle(ptr.Ptr());
 }
 
diff --git a/runtime/handle_scope.h b/runtime/handle_scope.h
index f6720bd3b2..c43a482fa1 100644
--- a/runtime/handle_scope.h
+++ b/runtime/handle_scope.h
@@ -30,7 +30,7 @@
 namespace art {
 
 class HandleScope;
-template<class MirrorType, bool kPoison> class ObjPtr;
+template<class MirrorType> class ObjPtr;
 class Thread;
 class VariableSizedHandleScope;
 
@@ -224,8 +224,8 @@ class PACKED(4) FixedSizeHandleScope : public HandleScope {
   ALWAYS_INLINE HandleWrapperObjPtr<T> NewHandleWrapper(ObjPtr<T>* object)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
-  template<class MirrorType, bool kPoison>
-  ALWAYS_INLINE MutableHandle<MirrorType> NewHandle(ObjPtr<MirrorType, kPoison> object)
+  template<class MirrorType>
+  ALWAYS_INLINE MutableHandle<MirrorType> NewHandle(ObjPtr<MirrorType> object)
     REQUIRES_SHARED(Locks::mutator_lock_);
 
   ALWAYS_INLINE void SetReference(size_t i, mirror::Object* object)
@@ -286,8 +286,8 @@ class VariableSizedHandleScope : public BaseHandleScope {
   template<class T>
   MutableHandle<T> NewHandle(T* object) REQUIRES_SHARED(Locks::mutator_lock_);
 
-  template<class MirrorType, bool kPoison>
-  MutableHandle<MirrorType> NewHandle(ObjPtr<MirrorType, kPoison> ptr)
+  template<class MirrorType>
+  MutableHandle<MirrorType> NewHandle(ObjPtr<MirrorType> ptr)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Number of references contained within this handle scope.
diff --git a/runtime/mirror/object_test.cc b/runtime/mirror/object_test.cc
index e761e4db7a..d306f9c391 100644
--- a/runtime/mirror/object_test.cc
+++ b/runtime/mirror/object_test.cc
@@ -726,57 +726,60 @@ TEST_F(ObjectTest, ObjectPointer) {
   ScopedObjectAccess soa(Thread::Current());
   jobject jclass_loader = LoadDex("XandY");
   StackHandleScope<2> hs(soa.Self());
-  ObjPtr<mirror::Object, /*kPoison*/ true> null_ptr;
-  EXPECT_TRUE(null_ptr.IsNull());
-  EXPECT_TRUE(null_ptr.IsValid());
-  EXPECT_TRUE(null_ptr.Ptr() == nullptr);
-  EXPECT_TRUE(null_ptr == nullptr);
-  EXPECT_TRUE(null_ptr == null_ptr);
-  EXPECT_FALSE(null_ptr != null_ptr);
-  EXPECT_FALSE(null_ptr != nullptr);
-  null_ptr.AssertValid();
   Handle<ClassLoader> class_loader(hs.NewHandle(soa.Decode<ClassLoader>(jclass_loader)));
   Handle<mirror::Class> h_X(
       hs.NewHandle(class_linker_->FindClass(soa.Self(), "LX;", class_loader)));
-  ObjPtr<Class, /*kPoison*/ true> X(h_X.Get());
-  EXPECT_TRUE(!X.IsNull());
-  EXPECT_TRUE(X.IsValid());
-  EXPECT_TRUE(X.Ptr() != nullptr);
-  EXPECT_OBJ_PTR_EQ(h_X.Get(), X);
-  // FindClass may cause thread suspension, it should invalidate X.
-  ObjPtr<Class, /*kPoison*/ true> Y(class_linker_->FindClass(soa.Self(), "LY;", class_loader));
-  EXPECT_TRUE(!Y.IsNull());
-  EXPECT_TRUE(Y.IsValid());
-  EXPECT_TRUE(Y.Ptr() != nullptr);
-
-  // Should IsNull be safe to call on null ObjPtr? I'll allow it for now.
-  EXPECT_TRUE(!X.IsNull());
-  EXPECT_TRUE(!X.IsValid());
-  // Make X valid again by copying out of handle.
-  X.Assign(h_X.Get());
-  EXPECT_TRUE(!X.IsNull());
-  EXPECT_TRUE(X.IsValid());
-  EXPECT_OBJ_PTR_EQ(h_X.Get(), X);
-
-  // Allow thread suspension to invalidate Y.
-  soa.Self()->AllowThreadSuspension();
-  EXPECT_TRUE(!Y.IsNull());
-  EXPECT_TRUE(!Y.IsValid());
-
-  // Test unpoisoned.
-  ObjPtr<mirror::Object, /*kPoison*/ false> unpoisoned;
-  EXPECT_TRUE(unpoisoned.IsNull());
-  EXPECT_TRUE(unpoisoned.IsValid());
-  EXPECT_TRUE(unpoisoned.Ptr() == nullptr);
-  EXPECT_TRUE(unpoisoned == nullptr);
-  EXPECT_TRUE(unpoisoned == unpoisoned);
-  EXPECT_FALSE(unpoisoned != unpoisoned);
-  EXPECT_FALSE(unpoisoned != nullptr);
-
-  unpoisoned = h_X.Get();
-  EXPECT_FALSE(unpoisoned.IsNull());
-  EXPECT_TRUE(unpoisoned == h_X.Get());
-  EXPECT_OBJ_PTR_EQ(unpoisoned, h_X.Get());
+
+  if (kObjPtrPoisoning) {
+    ObjPtr<mirror::Object> null_ptr;
+    EXPECT_TRUE(null_ptr.IsNull());
+    EXPECT_TRUE(null_ptr.IsValid());
+    EXPECT_TRUE(null_ptr.Ptr() == nullptr);
+    EXPECT_TRUE(null_ptr == nullptr);
+    EXPECT_TRUE(null_ptr == null_ptr);
+    EXPECT_FALSE(null_ptr != null_ptr);
+    EXPECT_FALSE(null_ptr != nullptr);
+    null_ptr.AssertValid();
+    ObjPtr<Class> X(h_X.Get());
+    EXPECT_TRUE(!X.IsNull());
+    EXPECT_TRUE(X.IsValid());
+    EXPECT_TRUE(X.Ptr() != nullptr);
+    EXPECT_OBJ_PTR_EQ(h_X.Get(), X);
+    // FindClass may cause thread suspension, it should invalidate X.
+    ObjPtr<Class> Y(class_linker_->FindClass(soa.Self(), "LY;", class_loader));
+    EXPECT_TRUE(!Y.IsNull());
+    EXPECT_TRUE(Y.IsValid());
+    EXPECT_TRUE(Y.Ptr() != nullptr);
+
+    // Should IsNull be safe to call on null ObjPtr? I'll allow it for now.
+    EXPECT_TRUE(!X.IsNull());
+    EXPECT_TRUE(!X.IsValid());
+    // Make X valid again by copying out of handle.
+    X.Assign(h_X.Get());
+    EXPECT_TRUE(!X.IsNull());
+    EXPECT_TRUE(X.IsValid());
+    EXPECT_OBJ_PTR_EQ(h_X.Get(), X);
+
+    // Allow thread suspension to invalidate Y.
+    soa.Self()->AllowThreadSuspension();
+    EXPECT_TRUE(!Y.IsNull());
+    EXPECT_TRUE(!Y.IsValid());
+  } else {
+    // Test unpoisoned.
+    ObjPtr<mirror::Object> unpoisoned;
+    EXPECT_TRUE(unpoisoned.IsNull());
+    EXPECT_TRUE(unpoisoned.IsValid());
+    EXPECT_TRUE(unpoisoned.Ptr() == nullptr);
+    EXPECT_TRUE(unpoisoned == nullptr);
+    EXPECT_TRUE(unpoisoned == unpoisoned);
+    EXPECT_FALSE(unpoisoned != unpoisoned);
+    EXPECT_FALSE(unpoisoned != nullptr);
+
+    unpoisoned = h_X.Get();
+    EXPECT_FALSE(unpoisoned.IsNull());
+    EXPECT_TRUE(unpoisoned == h_X.Get());
+    EXPECT_OBJ_PTR_EQ(unpoisoned, h_X.Get());
+  }
 }
 
 }  // namespace mirror
diff --git a/runtime/obj_ptr-inl.h b/runtime/obj_ptr-inl.h
index d0be6dc981..f2921daeaa 100644
--- a/runtime/obj_ptr-inl.h
+++ b/runtime/obj_ptr-inl.h
@@ -22,27 +22,27 @@
 
 namespace art {
 
-template<class MirrorType, bool kPoison>
-inline bool ObjPtr<MirrorType, kPoison>::IsValid() const {
-  if (!kPoison || IsNull()) {
+template<class MirrorType>
+inline bool ObjPtr<MirrorType>::IsValid() const {
+  if (!kObjPtrPoisoning || IsNull()) {
     return true;
   }
   return GetCookie() == TrimCookie(Thread::Current()->GetPoisonObjectCookie());
 }
 
-template<class MirrorType, bool kPoison>
-inline void ObjPtr<MirrorType, kPoison>::AssertValid() const {
-  if (kPoison) {
+template<class MirrorType>
+inline void ObjPtr<MirrorType>::AssertValid() const {
+  if (kObjPtrPoisoning) {
     CHECK(IsValid()) << "Stale object pointer " << PtrUnchecked() << " , expected cookie "
         << TrimCookie(Thread::Current()->GetPoisonObjectCookie()) << " but got " << GetCookie();
   }
 }
 
-template<class MirrorType, bool kPoison>
-inline uintptr_t ObjPtr<MirrorType, kPoison>::Encode(MirrorType* ptr) {
+template<class MirrorType>
+inline uintptr_t ObjPtr<MirrorType>::Encode(MirrorType* ptr) {
   uintptr_t ref = reinterpret_cast<uintptr_t>(ptr);
   DCHECK_ALIGNED(ref, kObjectAlignment);
-  if (kPoison && ref != 0) {
+  if (kObjPtrPoisoning && ref != 0) {
     DCHECK_LE(ref, 0xFFFFFFFFU);
     ref >>= kObjectAlignmentShift;
     // Put cookie in high bits.
@@ -53,8 +53,8 @@ inline uintptr_t ObjPtr<MirrorType, kPoison>::Encode(MirrorType* ptr) {
   return ref;
 }
 
-template<class MirrorType, bool kPoison>
-inline std::ostream& operator<<(std::ostream& os, ObjPtr<MirrorType, kPoison> ptr) {
+template<class MirrorType>
+inline std::ostream& operator<<(std::ostream& os, ObjPtr<MirrorType> ptr) {
   // May be used for dumping bad pointers, do not use the checked version.
   return os << ptr.PtrUnchecked();
 }
diff --git a/runtime/obj_ptr.h b/runtime/obj_ptr.h
index 2da2ae5650..92cf4ebe23 100644
--- a/runtime/obj_ptr.h
+++ b/runtime/obj_ptr.h
@@ -26,10 +26,12 @@
 
 namespace art {
 
+constexpr bool kObjPtrPoisoning = kIsDebugBuild;
+
 // Value type representing a pointer to a mirror::Object of type MirrorType
 // Pass kPoison as a template boolean for testing in non-debug builds.
 // Since the cookie is thread based, it is not safe to share an ObjPtr between threads.
-template<class MirrorType, bool kPoison = kIsDebugBuild>
+template<class MirrorType>
 class ObjPtr {
   static constexpr size_t kCookieShift =
       sizeof(kHeapReferenceSize) * kBitsPerByte - kObjectAlignmentShift;
@@ -60,14 +62,14 @@ class ObjPtr {
 
   template <typename Type,
             typename = typename std::enable_if<std::is_base_of<MirrorType, Type>::value>::type>
-  ALWAYS_INLINE ObjPtr(const ObjPtr<Type, kPoison>& other)  // NOLINT
+  ALWAYS_INLINE ObjPtr(const ObjPtr<Type>& other)  // NOLINT
       REQUIRES_SHARED(Locks::mutator_lock_)
       : reference_(Encode(static_cast<MirrorType*>(other.Ptr()))) {
   }
 
   template <typename Type,
             typename = typename std::enable_if<std::is_base_of<MirrorType, Type>::value>::type>
-  ALWAYS_INLINE ObjPtr& operator=(const ObjPtr<Type, kPoison>& other)
+  ALWAYS_INLINE ObjPtr& operator=(const ObjPtr<Type>& other)
       REQUIRES_SHARED(Locks::mutator_lock_) {
     reference_ = Encode(static_cast<MirrorType*>(other.Ptr()));
     return *this;
@@ -130,7 +132,7 @@ class ObjPtr {
 
   // Ptr unchecked does not check that object pointer is valid. Do not use if you can avoid it.
   ALWAYS_INLINE MirrorType* PtrUnchecked() const {
-    if (kPoison) {
+    if (kObjPtrPoisoning) {
       return reinterpret_cast<MirrorType*>(
           static_cast<uintptr_t>(static_cast<uint32_t>(reference_ << kObjectAlignmentShift)));
     } else {
@@ -167,46 +169,46 @@ static_assert(std::is_trivially_copyable<ObjPtr<void>>::value,
 // Hash function for stl data structures.
 class HashObjPtr {
  public:
-  template<class MirrorType, bool kPoison>
-  size_t operator()(const ObjPtr<MirrorType, kPoison>& ptr) const NO_THREAD_SAFETY_ANALYSIS {
+  template<class MirrorType>
+  size_t operator()(const ObjPtr<MirrorType>& ptr) const NO_THREAD_SAFETY_ANALYSIS {
     return std::hash<MirrorType*>()(ptr.Ptr());
   }
 };
 
-template<class MirrorType, bool kPoison, typename PointerType>
-ALWAYS_INLINE bool operator==(const PointerType* a, const ObjPtr<MirrorType, kPoison>& b)
+template<class MirrorType, typename PointerType>
+ALWAYS_INLINE bool operator==(const PointerType* a, const ObjPtr<MirrorType>& b)
     REQUIRES_SHARED(Locks::mutator_lock_) {
   return b == a;
 }
 
-template<class MirrorType, bool kPoison>
-ALWAYS_INLINE bool operator==(std::nullptr_t, const ObjPtr<MirrorType, kPoison>& b) {
+template<class MirrorType>
+ALWAYS_INLINE bool operator==(std::nullptr_t, const ObjPtr<MirrorType>& b) {
   return b == nullptr;
 }
 
-template<typename MirrorType, bool kPoison, typename PointerType>
-ALWAYS_INLINE bool operator!=(const PointerType* a, const ObjPtr<MirrorType, kPoison>& b)
+template<typename MirrorType, typename PointerType>
+ALWAYS_INLINE bool operator!=(const PointerType* a, const ObjPtr<MirrorType>& b)
     REQUIRES_SHARED(Locks::mutator_lock_) {
   return b != a;
 }
 
-template<class MirrorType, bool kPoison>
-ALWAYS_INLINE bool operator!=(std::nullptr_t, const ObjPtr<MirrorType, kPoison>& b) {
+template<class MirrorType>
+ALWAYS_INLINE bool operator!=(std::nullptr_t, const ObjPtr<MirrorType>& b) {
   return b != nullptr;
 }
 
-template<class MirrorType, bool kPoison = kIsDebugBuild>
-static inline ObjPtr<MirrorType, kPoison> MakeObjPtr(MirrorType* ptr) {
-  return ObjPtr<MirrorType, kPoison>(ptr);
+template<class MirrorType>
+static inline ObjPtr<MirrorType> MakeObjPtr(MirrorType* ptr) {
+  return ObjPtr<MirrorType>(ptr);
 }
 
-template<class MirrorType, bool kPoison = kIsDebugBuild>
-static inline ObjPtr<MirrorType, kPoison> MakeObjPtr(ObjPtr<MirrorType, kPoison> ptr) {
-  return ObjPtr<MirrorType, kPoison>(ptr);
+template<class MirrorType>
+static inline ObjPtr<MirrorType> MakeObjPtr(ObjPtr<MirrorType> ptr) {
+  return ObjPtr<MirrorType>(ptr);
 }
 
-template<class MirrorType, bool kPoison>
-ALWAYS_INLINE std::ostream& operator<<(std::ostream& os, ObjPtr<MirrorType, kPoison> ptr);
+template<class MirrorType>
+ALWAYS_INLINE std::ostream& operator<<(std::ostream& os, ObjPtr<MirrorType> ptr);
 
 }  // namespace art
 
diff --git a/runtime/scoped_thread_state_change-inl.h b/runtime/scoped_thread_state_change-inl.h
index 000da59bd2..c817a9ee8d 100644
--- a/runtime/scoped_thread_state_change-inl.h
+++ b/runtime/scoped_thread_state_change-inl.h
@@ -79,11 +79,11 @@ inline T ScopedObjectAccessAlreadyRunnable::AddLocalReference(ObjPtr<mirror::Obj
   return obj == nullptr ? nullptr : Env()->AddLocalReference<T>(obj);
 }
 
-template<typename T, bool kPoison>
-inline ObjPtr<T, kPoison> ScopedObjectAccessAlreadyRunnable::Decode(jobject obj) const {
+template<typename T>
+inline ObjPtr<T> ScopedObjectAccessAlreadyRunnable::Decode(jobject obj) const {
   Locks::mutator_lock_->AssertSharedHeld(Self());
   DCHECK(IsRunnable());  // Don't work with raw objects in non-runnable states.
-  return ObjPtr<T, kPoison>::DownCast(Self()->DecodeJObject(obj));
+  return ObjPtr<T>::DownCast(Self()->DecodeJObject(obj));
 }
 
 inline bool ScopedObjectAccessAlreadyRunnable::IsRunnable() const {
diff --git a/runtime/scoped_thread_state_change.h b/runtime/scoped_thread_state_change.h
index 24199f76b6..a3286ac3d4 100644
--- a/runtime/scoped_thread_state_change.h
+++ b/runtime/scoped_thread_state_change.h
@@ -27,7 +27,7 @@
 namespace art {
 
 struct JNIEnvExt;
-template<class MirrorType, bool kPoison> class ObjPtr;
+template<class MirrorType> class ObjPtr;
 
 // Scoped change into and out of a particular state. Handles Runnable transitions that require
 // more complicated suspension checking. The subclasses ScopedObjectAccessUnchecked and
@@ -91,8 +91,8 @@ class ScopedObjectAccessAlreadyRunnable : public ValueObject {
   T AddLocalReference(ObjPtr<mirror::Object> obj) const
       REQUIRES_SHARED(Locks::mutator_lock_);
 
-  template<typename T, bool kPoison = kIsDebugBuild>
-  ObjPtr<T, kPoison> Decode(jobject obj) const REQUIRES_SHARED(Locks::mutator_lock_);
+  template<typename T>
+  ObjPtr<T> Decode(jobject obj) const REQUIRES_SHARED(Locks::mutator_lock_);
 
   ALWAYS_INLINE bool IsRunnable() const;
 
diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h
index 8d946262e8..482e0e39a9 100644
--- a/runtime/thread-inl.h
+++ b/runtime/thread-inl.h
@@ -29,6 +29,7 @@
 #include "base/mutex-inl.h"
 #include "gc/heap.h"
 #include "jni_env_ext.h"
+#include "obj_ptr.h"
 #include "runtime.h"
 #include "thread_pool.h"
 
@@ -355,7 +356,7 @@ inline void Thread::RevokeThreadLocalAllocationStack() {
 }
 
 inline void Thread::PoisonObjectPointersIfDebug() {
-  if (kIsDebugBuild) {
+  if (kObjPtrPoisoning) {
     Thread::Current()->PoisonObjectPointers();
   }
 }