[cleanup] Cleanup how we check heap object tags

1) HAS_[STRONG|WEAK]_HEAP_OBJECT_TAG macros are to be used for
   checking raw representations of tagged values (Address or Tagged_t)
2) HasWeakHeapObjectTag(Object) function is for overzealous checking of
   Object tags

Bug: v8:9183
Tbr: jgruber@chromium.org
Change-Id: Iaa456dbcb21f43a8df0d9ca706c0fc3b2ede075d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1588455
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61091}

src/globals.h

@@ -910,10 +910,14 @@ constexpr int kIeeeDoubleExponentWordOffset = 0;
 #define HAS_SMI_TAG(value) \
   ((static_cast<intptr_t>(value) & ::i::kSmiTagMask) == ::i::kSmiTag)
 
-#define HAS_HEAP_OBJECT_TAG(value)                              \
+#define HAS_STRONG_HEAP_OBJECT_TAG(value)                       \
   (((static_cast<intptr_t>(value) & ::i::kHeapObjectTagMask) == \
     ::i::kHeapObjectTag))
 
+#define HAS_WEAK_HEAP_OBJECT_TAG(value)                         \
+  (((static_cast<intptr_t>(value) & ::i::kHeapObjectTagMask) == \
+    ::i::kWeakHeapObjectTag))
+
 // OBJECT_POINTER_ALIGN returns the value aligned as a HeapObject pointer
 #define OBJECT_POINTER_ALIGN(value) \
   (((value) + ::i::kObjectAlignmentMask) & ~::i::kObjectAlignmentMask)
@@ -1478,12 +1482,6 @@ enum IsolateAddressId {
       kIsolateAddressCount
 };
 
-V8_INLINE static bool HasWeakHeapObjectTag(Address value) {
-  // TODO(jkummerow): Consolidate integer types here.
-  return ((static_cast<intptr_t>(value) & kHeapObjectTagMask) ==
-          kWeakHeapObjectTag);
-}
-
 enum class HeapObjectReferenceType {
   WEAK,
   STRONG,

src/heap/heap-write-barrier-inl.h

@@ -226,7 +226,7 @@ inline void MarkingBarrierForElements(Heap* heap, HeapObject object) {
 
 inline void MarkingBarrierForCode(Code host, RelocInfo* rinfo,
                                   HeapObject object) {
-  DCHECK(!HasWeakHeapObjectTag(object.ptr()));
+  DCHECK(!HasWeakHeapObjectTag(object));
   heap_internals::MemoryChunk* object_chunk =
       heap_internals::MemoryChunk::FromHeapObject(object);
   if (!object_chunk->IsMarking()) return;

src/heap/mark-compact.cc

@@ -2525,8 +2525,8 @@ static inline SlotCallbackResult UpdateSlot(TSlot slot) {
 
 template <AccessMode access_mode, typename TSlot>
 static inline SlotCallbackResult UpdateStrongSlot(TSlot slot) {
-  DCHECK(!HasWeakHeapObjectTag((*slot).ptr()));
   typename TSlot::TObject obj = slot.Relaxed_Load();
+  DCHECK(!HAS_WEAK_HEAP_OBJECT_TAG(obj.ptr()));
   HeapObject heap_obj;
   if (obj.GetHeapObject(&heap_obj)) {
     return UpdateSlot<access_mode, HeapObjectReferenceType::STRONG>(slot, obj,

src/heap/remembered-set.h

@@ -303,7 +303,7 @@ class UpdateTypedSlotHelper {
     Code code = Code::GetObjectFromEntryAddress(entry_address);
     Code old_code = code;
     SlotCallbackResult result = callback(FullMaybeObjectSlot(&code));
-    DCHECK(!HasWeakHeapObjectTag(code.ptr()));
+    DCHECK(!HasWeakHeapObjectTag(code));
     if (code != old_code) {
       Memory<Address>(entry_address) = code->entry();
     }
@@ -319,7 +319,7 @@ class UpdateTypedSlotHelper {
     Code old_target = Code::GetCodeFromTargetAddress(rinfo->target_address());
     Code new_target = old_target;
     SlotCallbackResult result = callback(FullMaybeObjectSlot(&new_target));
-    DCHECK(!HasWeakHeapObjectTag(new_target.ptr()));
+    DCHECK(!HasWeakHeapObjectTag(new_target));
     if (new_target != old_target) {
       rinfo->set_target_address(
           Code::cast(new_target)->raw_instruction_start());
@@ -336,7 +336,7 @@ class UpdateTypedSlotHelper {
     HeapObject old_target = rinfo->target_object();
     HeapObject new_target = old_target;
     SlotCallbackResult result = callback(FullMaybeObjectSlot(&new_target));
-    DCHECK(!HasWeakHeapObjectTag(new_target->ptr()));
+    DCHECK(!HasWeakHeapObjectTag(new_target));
     if (new_target != old_target) {
       rinfo->set_target_object(heap, HeapObject::cast(new_target));
     }

src/objects-debug.cc

@@ -570,7 +570,7 @@ void VerifyJSObjectElements(Isolate* isolate, JSObject object) {
   } else if (object->HasObjectElements()) {
     for (int i = 0; i < elements->length(); i++) {
       Object element = elements->get(i);
-      CHECK_IMPLIES(!element->IsSmi(), !HasWeakHeapObjectTag(element));
+      CHECK(!HasWeakHeapObjectTag(element));
     }
   }
 }

src/objects.h

@@ -909,7 +909,7 @@ class Object {
   // Always returns false because Object is not expected to be a weak pointer
   // to a HeapObject.
   inline bool GetHeapObjectIfWeak(HeapObject* result) const {
-    DCHECK(!HasWeakHeapObjectTag(ptr()));
+    DCHECK(!HAS_WEAK_HEAP_OBJECT_TAG(ptr_));
     return false;
   }
   // Always returns false because Object is not expected to be a weak pointer
@@ -1033,7 +1033,7 @@ V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os, const Brief& v);
 // Objects should never have the weak tag; this variant is for overzealous
 // checking.
 V8_INLINE static bool HasWeakHeapObjectTag(const Object value) {
-  return ((value->ptr() & kHeapObjectTagMask) == kWeakHeapObjectTag);
+  return HAS_WEAK_HEAP_OBJECT_TAG(value.ptr());
 }
 
 // Heap objects typically have a map pointer in their first word.  However,

src/objects/maybe-object-inl.h

@@ -50,19 +50,17 @@ bool MaybeObject::GetHeapObject(HeapObject* result,
   if (IsSmi() || IsCleared()) {
     return false;
   }
-  *reference_type = HasWeakHeapObjectTag(ptr_)
+  *reference_type = HAS_WEAK_HEAP_OBJECT_TAG(ptr_)
                         ? HeapObjectReferenceType::WEAK
                         : HeapObjectReferenceType::STRONG;
   *result = GetHeapObject();
   return true;
 }
 
-bool MaybeObject::IsStrong() const {
-  return !HasWeakHeapObjectTag(ptr_) && !IsSmi();
-}
+bool MaybeObject::IsStrong() const { return HAS_STRONG_HEAP_OBJECT_TAG(ptr_); }
 
 bool MaybeObject::GetHeapObjectIfStrong(HeapObject* result) const {
-  if (!HasWeakHeapObjectTag(ptr_) && !IsSmi()) {
+  if (!HAS_WEAK_HEAP_OBJECT_TAG(ptr_) && !IsSmi()) {
     *result = HeapObject::cast(Object(ptr_));
     return true;
   }
@@ -75,10 +73,12 @@ HeapObject MaybeObject::GetHeapObjectAssumeStrong() const {
 }
 
 bool MaybeObject::IsWeak() const {
-  return HasWeakHeapObjectTag(ptr_) && !IsCleared();
+  return HAS_WEAK_HEAP_OBJECT_TAG(ptr_) && !IsCleared();
 }
 
-bool MaybeObject::IsWeakOrCleared() const { return HasWeakHeapObjectTag(ptr_); }
+bool MaybeObject::IsWeakOrCleared() const {
+  return HAS_WEAK_HEAP_OBJECT_TAG(ptr_);
+}
 
 bool MaybeObject::GetHeapObjectIfWeak(HeapObject* result) const {
   if (IsWeak()) {
@@ -137,18 +137,18 @@ void HeapObjectReference::Update(THeapObjectSlot slot, HeapObject value) {
                 "Only FullHeapObjectSlot and HeapObjectSlot are expected here");
   Address old_value = (*slot).ptr();
   DCHECK(!HAS_SMI_TAG(old_value));
-  Address new_value = value->ptr();
+  Address new_value = value.ptr();
   DCHECK(Internals::HasHeapObjectTag(new_value));
 
 #ifdef DEBUG
-  bool weak_before = HasWeakHeapObjectTag(old_value);
+  bool weak_before = HAS_WEAK_HEAP_OBJECT_TAG(old_value);
 #endif
 
   slot.store(
       HeapObjectReference(new_value | (old_value & kWeakHeapObjectMask)));
 
 #ifdef DEBUG
-  bool weak_after = HasWeakHeapObjectTag((*slot).ptr());
+  bool weak_after = HAS_WEAK_HEAP_OBJECT_TAG((*slot).ptr());
   DCHECK_EQ(weak_before, weak_after);
 #endif
 }

src/objects/maybe-object.h

@@ -82,7 +82,7 @@ class MaybeObject {
   inline bool IsObject() const;
   template <typename T>
   T cast() const {
-    DCHECK(!HasWeakHeapObjectTag(ptr_));
+    DCHECK(!HAS_WEAK_HEAP_OBJECT_TAG(ptr_));
     return T::cast(Object(ptr_));
   }
 
@@ -92,7 +92,7 @@ class MaybeObject {
   }
 
   static MaybeObject FromObject(Object object) {
-    DCHECK(!HasWeakHeapObjectTag(object.ptr()));
+    DCHECK(!HasWeakHeapObjectTag(object));
     return MaybeObject(object.ptr());
   }
 

src/profiler/tick-sample.cc

@@ -273,7 +273,8 @@ bool TickSample::GetStackSample(Isolate* v8_isolate, RegisterState* regs,
 
       // If the bytecode array is a heap object and the bytecode offset is a
       // Smi, use those, otherwise fall back to using the frame's pc.
-      if (HAS_HEAP_OBJECT_TAG(bytecode_array) && HAS_SMI_TAG(bytecode_offset)) {
+      if (HAS_STRONG_HEAP_OBJECT_TAG(bytecode_array) &&
+          HAS_SMI_TAG(bytecode_offset)) {
         frames[i++] = reinterpret_cast<void*>(
             bytecode_array + i::Internals::SmiValue(bytecode_offset));
         continue;

src/snapshot/deserializer.cc

@@ -361,7 +361,7 @@ HeapObject Deserializer::GetBackReferencedObject(int space) {
   }
 
   hot_objects_.Add(obj);
-  DCHECK(!HasWeakHeapObjectTag(obj->ptr()));
+  DCHECK(!HasWeakHeapObjectTag(obj));
   return obj;
 }