[ptr-compr] Remove default implementation of ObjectVisitor::VisitCodeTarget()

which used to treat off-heap slots as on-heap ones and implement code target
visitation in derived visitor classes.

Bug: v8:8518
Change-Id: I477bf3a4a8a3de0c67bc15e2e20d8ecee6493da8
Reviewed-on: https://chromium-review.googlesource.com/c/1367745Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58121}

src/heap/concurrent-marking.cc

@@ -441,6 +441,12 @@ class ConcurrentMarkingVisitor final
       UNREACHABLE();
     }
 
+    void VisitCodeTarget(Code host, RelocInfo* rinfo) final {
+      // This should never happen, because snapshotting is performed only on
+      // JSObjects (and derived classes).
+      UNREACHABLE();
+    }
+
     void VisitCustomWeakPointers(HeapObject* host, ObjectSlot start,
                                  ObjectSlot end) override {
       DCHECK(host->IsJSWeakCell());

src/heap/heap.cc

@@ -1475,6 +1475,8 @@ class StringTableVerifier : public ObjectVisitor {
     UNREACHABLE();
   }
 
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override { UNREACHABLE(); }
+
  private:
   Isolate* isolate_;
 };
@@ -2994,6 +2996,8 @@ class SlotCollectingVisitor final : public ObjectVisitor {
     }
   }
 
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) final { UNREACHABLE(); }
+
   int number_of_slots() { return static_cast<int>(slots_.size()); }
 
   MaybeObjectSlot slot(int i) { return slots_[i]; }
@@ -3485,9 +3489,9 @@ class VerifyReadOnlyPointersVisitor : public VerifyPointersVisitor {
     VerifyPointersVisitor::VerifyPointers(host, start, end);
 
     for (MaybeObjectSlot current = start; current < end; ++current) {
-      HeapObject* object;
-      if ((*current)->GetHeapObject(&object)) {
-        CHECK(heap_->InReadOnlySpace(object));
+      HeapObject* heap_object;
+      if ((*current)->GetHeapObject(&heap_object)) {
+        CHECK(heap_->InReadOnlySpace(heap_object));
       }
     }
   }
@@ -5031,6 +5035,11 @@ class UnreachableObjectsFilter : public HeapObjectsFilter {
       MarkPointers(start, end);
     }
 
+    void VisitCodeTarget(Code host, RelocInfo* rinfo) final {
+      Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+      MarkHeapObject(target);
+    }
+
     void VisitRootPointers(Root root, const char* description,
                            FullObjectSlot start, FullObjectSlot end) override {
       MarkPointersImpl(start, end);
@@ -5056,12 +5065,17 @@ class UnreachableObjectsFilter : public HeapObjectsFilter {
         typename TSlot::TObject object = p.load();
         HeapObject* heap_object;
         if (object.GetHeapObject(&heap_object)) {
-          if (filter_->MarkAsReachable(heap_object)) {
-            marking_stack_.push_back(heap_object);
-          }
+          MarkHeapObject(heap_object);
         }
       }
     }
+
+    V8_INLINE void MarkHeapObject(HeapObject* heap_object) {
+      if (filter_->MarkAsReachable(heap_object)) {
+        marking_stack_.push_back(heap_object);
+      }
+    }
+
     UnreachableObjectsFilter* filter_;
     std::vector<HeapObject*> marking_stack_;
   };
@@ -5396,26 +5410,38 @@ void VerifyPointersVisitor::VisitRootPointers(Root root,
                                               const char* description,
                                               FullObjectSlot start,
                                               FullObjectSlot end) {
-  // TODO(ishell): visiting off-heap pointer
-  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
-  VerifyPointers(nullptr, MaybeObjectSlot(start.address()),
-                 MaybeObjectSlot(end.address()));
+  VerifyPointersImpl(start, end);
 }
 
-void VerifyPointersVisitor::VerifyPointers(HeapObject* host,
-                                           MaybeObjectSlot start,
-                                           MaybeObjectSlot end) {
-  for (MaybeObjectSlot current = start; current < end; ++current) {
-    HeapObject* object;
-    if ((*current)->GetHeapObject(&object)) {
-      CHECK(heap_->Contains(object));
-      CHECK(object->map()->IsMap());
+void VerifyPointersVisitor::VerifyHeapObjectImpl(HeapObject* heap_object) {
+  CHECK(heap_->Contains(heap_object));
+  CHECK(heap_object->map()->IsMap());
+}
+
+template <typename TSlot>
+void VerifyPointersVisitor::VerifyPointersImpl(TSlot start, TSlot end) {
+  for (TSlot slot = start; slot < end; ++slot) {
+    typename TSlot::TObject object = slot.load();
+    HeapObject* heap_object;
+    if (object.GetHeapObject(&heap_object)) {
+      VerifyHeapObjectImpl(heap_object);
     } else {
-      CHECK((*current)->IsSmi() || (*current)->IsCleared());
+      CHECK(object->IsSmi() || object->IsCleared());
     }
   }
 }
 
+void VerifyPointersVisitor::VerifyPointers(HeapObject* host,
+                                           MaybeObjectSlot start,
+                                           MaybeObjectSlot end) {
+  VerifyPointersImpl(start, end);
+}
+
+void VerifyPointersVisitor::VisitCodeTarget(Code host, RelocInfo* rinfo) {
+  Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+  VerifyHeapObjectImpl(target);
+}
+
 void VerifySmisVisitor::VisitRootPointers(Root root, const char* description,
                                           FullObjectSlot start,
                                           FullObjectSlot end) {

src/heap/heap.h

@@ -2161,10 +2161,17 @@ class VerifyPointersVisitor : public ObjectVisitor, public RootVisitor {
                      ObjectSlot end) override;
   void VisitPointers(HeapObject* host, MaybeObjectSlot start,
                      MaybeObjectSlot end) override;
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override;
+
   void VisitRootPointers(Root root, const char* description,
                          FullObjectSlot start, FullObjectSlot end) override;
 
  protected:
+  V8_INLINE void VerifyHeapObjectImpl(HeapObject* heap_object);
+
+  template <typename TSlot>
+  V8_INLINE void VerifyPointersImpl(TSlot start, TSlot end);
+
   virtual void VerifyPointers(HeapObject* host, MaybeObjectSlot start,
                               MaybeObjectSlot end);
 

src/heap/mark-compact.cc

@@ -202,24 +202,31 @@ class FullMarkingVerifier : public MarkingVerifier {
     VerifyPointersImpl(start, end);
   }
 
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override {
+    Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+    VerifyHeapObjectImpl(target);
+  }
+
   void VisitEmbeddedPointer(Code host, RelocInfo* rinfo) override {
     DCHECK(rinfo->rmode() == RelocInfo::EMBEDDED_OBJECT);
     if (!host->IsWeakObject(rinfo->target_object())) {
-      Object* p = rinfo->target_object();
-      // TODO(ishell): visiting off-heap pointer
-      STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
-      VisitPointer(host, ObjectSlot(&p));
+      HeapObject* object = rinfo->target_object();
+      VerifyHeapObjectImpl(object);
     }
   }
 
  private:
+  V8_INLINE void VerifyHeapObjectImpl(HeapObject* heap_object) {
+    CHECK(marking_state_->IsBlackOrGrey(heap_object));
+  }
+
   template <typename TSlot>
   V8_INLINE void VerifyPointersImpl(TSlot start, TSlot end) {
     for (TSlot slot = start; slot < end; ++slot) {
       typename TSlot::TObject object = slot.load();
       HeapObject* heap_object;
       if (object.GetHeapObjectIfStrong(&heap_object)) {
-        CHECK(marking_state_->IsBlackOrGrey(heap_object));
+        VerifyHeapObjectImpl(heap_object);
       }
     }
   }
@@ -313,16 +320,18 @@ class FullEvacuationVerifier : public EvacuationVerifier {
   }
 
  protected:
+  V8_INLINE void VerifyHeapObjectImpl(HeapObject* heap_object) {
+    CHECK_IMPLIES(Heap::InNewSpace(heap_object), Heap::InToSpace(heap_object));
+    CHECK(!MarkCompactCollector::IsOnEvacuationCandidate(heap_object));
+  }
+
   template <typename TSlot>
   void VerifyPointersImpl(TSlot start, TSlot end) {
     for (TSlot current = start; current < end; ++current) {
       typename TSlot::TObject object = current.load();
       HeapObject* heap_object;
       if (object.GetHeapObjectIfStrong(&heap_object)) {
-        if (Heap::InNewSpace(heap_object)) {
-          CHECK(Heap::InToSpace(heap_object));
-        }
-        CHECK(!MarkCompactCollector::IsOnEvacuationCandidate(heap_object));
+        VerifyHeapObjectImpl(heap_object);
       }
     }
   }
@@ -333,6 +342,10 @@ class FullEvacuationVerifier : public EvacuationVerifier {
   void VerifyPointers(MaybeObjectSlot start, MaybeObjectSlot end) override {
     VerifyPointersImpl(start, end);
   }
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override {
+    Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+    VerifyHeapObjectImpl(target);
+  }
   void VerifyRootPointers(FullObjectSlot start, FullObjectSlot end) override {
     VerifyPointersImpl(start, end);
   }
@@ -925,9 +938,13 @@ class MarkCompactCollector::CustomRootBodyMarkingVisitor final
   }
 
   // VisitEmbedderPointer is defined by ObjectVisitor to call VisitPointers.
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override {
+    Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+    MarkObject(host, target);
+  }
 
  private:
-  void MarkObject(HeapObject* host, Object* object) {
+  V8_INLINE void MarkObject(HeapObject* host, Object* object) {
     if (!object->IsHeapObject()) return;
     collector_->MarkObject(host, HeapObject::cast(object));
   }
@@ -968,6 +985,8 @@ class InternalizedStringTableCleaner : public ObjectVisitor {
     UNREACHABLE();
   }
 
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) final { UNREACHABLE(); }
+
   int PointersRemoved() {
     return pointers_removed_;
   }
@@ -3636,6 +3655,7 @@ class YoungGenerationMarkingVerifier : public MarkingVerifier {
     VerifyMarking(heap_->new_space());
   }
 
+ protected:
   void VerifyPointers(ObjectSlot start, ObjectSlot end) override {
     VerifyPointersImpl(start, end);
   }
@@ -3644,11 +3664,20 @@ class YoungGenerationMarkingVerifier : public MarkingVerifier {
     VerifyPointersImpl(start, end);
   }
 
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override {
+    Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+    VerifyHeapObjectImpl(target);
+  }
+
   void VerifyRootPointers(FullObjectSlot start, FullObjectSlot end) override {
     VerifyPointersImpl(start, end);
   }
 
  private:
+  V8_INLINE void VerifyHeapObjectImpl(HeapObject* heap_object) {
+    CHECK_IMPLIES(Heap::InNewSpace(heap_object), IsMarked(heap_object));
+  }
+
   template <typename TSlot>
   V8_INLINE void VerifyPointersImpl(TSlot start, TSlot end) {
     for (TSlot slot = start; slot < end; ++slot) {
@@ -3656,7 +3685,7 @@ class YoungGenerationMarkingVerifier : public MarkingVerifier {
       HeapObject* heap_object;
       // Minor MC treats weak references as strong.
       if (object.GetHeapObject(&heap_object)) {
-        CHECK_IMPLIES(Heap::InNewSpace(heap_object), IsMarked(heap_object));
+        VerifyHeapObjectImpl(heap_object);
       }
     }
   }
@@ -3678,14 +3707,17 @@ class YoungGenerationEvacuationVerifier : public EvacuationVerifier {
   }
 
  protected:
+  V8_INLINE void VerifyHeapObjectImpl(HeapObject* heap_object) {
+    CHECK_IMPLIES(Heap::InNewSpace(heap_object), Heap::InToSpace(heap_object));
+  }
+
   template <typename TSlot>
   void VerifyPointersImpl(TSlot start, TSlot end) {
     for (TSlot current = start; current < end; ++current) {
       typename TSlot::TObject object = current.load();
       HeapObject* heap_object;
       if (object.GetHeapObject(&heap_object)) {
-        CHECK_IMPLIES(Heap::InNewSpace(heap_object),
-                      Heap::InToSpace(heap_object));
+        VerifyHeapObjectImpl(heap_object);
       }
     }
   }
@@ -3696,6 +3728,10 @@ class YoungGenerationEvacuationVerifier : public EvacuationVerifier {
   void VerifyPointers(MaybeObjectSlot start, MaybeObjectSlot end) override {
     VerifyPointersImpl(start, end);
   }
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override {
+    Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+    VerifyHeapObjectImpl(target);
+  }
   void VerifyRootPointers(FullObjectSlot start, FullObjectSlot end) override {
     VerifyPointersImpl(start, end);
   }
@@ -3752,6 +3788,11 @@ class YoungGenerationMarkingVisitor final
     VisitPointerImpl(host, slot);
   }
 
+  V8_INLINE void VisitCodeTarget(Code host, RelocInfo* rinfo) final {
+    // Code objects are not expected in new space.
+    UNREACHABLE();
+  }
+
  private:
   template <typename TSlot>
   V8_INLINE void VisitPointersImpl(HeapObject* host, TSlot start, TSlot end) {

src/heap/object-stats.cc

@@ -77,6 +77,11 @@ class FieldStatsCollector : public ObjectVisitor {
     *tagged_fields_count_ += (end - start);
   }
 
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override {
+    // Code target is most likely encoded as a relative 32-bit offset and not
+    // as a full tagged value, so there's nothing to count.
+  }
+
  private:
   struct JSObjectFieldStats {
     JSObjectFieldStats()

src/heap/scavenger-inl.h

@@ -413,18 +413,22 @@ SlotCallbackResult Scavenger::CheckAndScavengeObject(Heap* heap,
   return REMOVE_SLOT;
 }
 
-V8_INLINE void ScavengeVisitor::VisitPointers(HeapObject* host,
-                                              ObjectSlot start,
-                                              ObjectSlot end) {
+void ScavengeVisitor::VisitPointers(HeapObject* host, ObjectSlot start,
+                                    ObjectSlot end) {
   return VisitPointersImpl(host, start, end);
 }
 
-V8_INLINE void ScavengeVisitor::VisitPointers(HeapObject* host,
-                                              MaybeObjectSlot start,
-                                              MaybeObjectSlot end) {
+void ScavengeVisitor::VisitPointers(HeapObject* host, MaybeObjectSlot start,
+                                    MaybeObjectSlot end) {
   return VisitPointersImpl(host, start, end);
 }
 
+void ScavengeVisitor::VisitCodeTarget(Code host, RelocInfo* rinfo) {
+  Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+  FullObjectSlot slot(&target);
+  VisitPointersImpl(host, slot, slot + 1);
+}
+
 template <typename TSlot>
 void ScavengeVisitor::VisitPointersImpl(HeapObject* host, TSlot start,
                                         TSlot end) {

src/heap/scavenger.cc

@@ -86,6 +86,11 @@ class IterateAndScavengePromotedObjectsVisitor final : public ObjectVisitor {
     VisitPointersImpl(host, start, end);
   }
 
+  V8_INLINE void VisitCodeTarget(Code host, RelocInfo* rinfo) final {
+    Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+    HandleSlot(host, FullHeapObjectSlot(&target), target);
+  }
+
  private:
   template <typename TSlot>
   V8_INLINE void VisitPointersImpl(HeapObject* host, TSlot start, TSlot end) {

src/heap/scavenger.h

@@ -235,6 +235,8 @@ class ScavengeVisitor final : public NewSpaceVisitor<ScavengeVisitor> {
   V8_INLINE void VisitPointers(HeapObject* host, MaybeObjectSlot start,
                                MaybeObjectSlot end) final;
 
+  V8_INLINE void VisitCodeTarget(Code host, RelocInfo* rinfo) final;
+
  private:
   template <typename TSlot>
   V8_INLINE void VisitPointersImpl(HeapObject* host, TSlot start, TSlot end);

src/objects.cc

@@ -14488,16 +14488,6 @@ void Map::StartInobjectSlackTracking() {
   set_construction_counter(Map::kSlackTrackingCounterStart);
 }
 
-void ObjectVisitor::VisitCodeTarget(Code host, RelocInfo* rinfo) {
-  DCHECK(RelocInfo::IsCodeTargetMode(rinfo->rmode()));
-  Object* old_pointer = Code::GetCodeFromTargetAddress(rinfo->target_address());
-  Object* new_pointer = old_pointer;
-  // TODO(ishell): we are actually visiting off-heap slot here.
-  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
-  VisitPointer(host, FullObjectSlot(&new_pointer));
-  DCHECK_EQ(old_pointer, new_pointer);
-}
-
 void ObjectVisitor::VisitEmbeddedPointer(Code host, RelocInfo* rinfo) {
   DCHECK(rinfo->rmode() == RelocInfo::EMBEDDED_OBJECT);
   Object* old_pointer = rinfo->target_object();

src/objects/heap-object.h

@@ -115,6 +115,9 @@ class ObjectPtr {
     DCHECK(!HasWeakHeapObjectTag(ptr()));
     return false;
   }
+  // Always returns false because Object is not expected to be a weak pointer
+  // to a HeapObject.
+  inline bool IsCleared() const { return false; }
 
 #ifdef VERIFY_HEAP
   void ObjectVerify(Isolate* isolate) {

src/objects/slots.h

@@ -179,6 +179,8 @@ class FullHeapObjectSlot
  public:
   FullHeapObjectSlot() : SlotBase(kNullAddress) {}
   explicit FullHeapObjectSlot(Address ptr) : SlotBase(ptr) {}
+  explicit FullHeapObjectSlot(ObjectPtr* ptr)
+      : SlotBase(reinterpret_cast<Address>(ptr)) {}
   template <typename T>
   explicit FullHeapObjectSlot(SlotBase<T, TData, kSlotDataSize> slot)
       : SlotBase(slot.address()) {}

src/profiler/heap-snapshot-generator.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "src/api-inl.h"
+#include "src/assembler-inl.h"
 #include "src/conversions.h"
 #include "src/debug/debug.h"
 #include "src/global-handles.h"
@@ -669,42 +670,53 @@ class IndexedReferencesExtractor : public ObjectVisitor {
         parent_start_(HeapObject::RawMaybeWeakField(parent_obj_, 0)),
         parent_end_(
             HeapObject::RawMaybeWeakField(parent_obj_, parent_obj_->Size())),
-        parent_(parent) {}
+        parent_(parent),
+        next_index_(0) {}
   void VisitPointers(HeapObject* host, ObjectSlot start,
                      ObjectSlot end) override {
     VisitPointers(host, MaybeObjectSlot(start), MaybeObjectSlot(end));
   }
   void VisitPointers(HeapObject* host, MaybeObjectSlot start,
                      MaybeObjectSlot end) override {
-    int next_index = 0;
     for (MaybeObjectSlot p = start; p < end; ++p) {
-      int index = -1;
+      int field_index = -1;
       // |p| could be outside of the object, e.g., while visiting RelocInfo of
       // code objects.
       if (parent_start_ <= p && p < parent_end_) {
-        index = static_cast<int>(p - parent_start_);
-        if (generator_->visited_fields_[index]) {
-          generator_->visited_fields_[index] = false;
+        field_index = static_cast<int>(p - parent_start_);
+        if (generator_->visited_fields_[field_index]) {
+          generator_->visited_fields_[field_index] = false;
           continue;
         }
       }
       HeapObject* heap_object;
       if ((*p)->GetHeapObject(&heap_object)) {
-        // The last parameter {field_offset} is only used to check some
-        // well-known skipped references, so passing -1 * kPointerSize
-        // for out-of-object slots is fine.
-        generator_->SetHiddenReference(parent_obj_, parent_, next_index++,
-                                       heap_object, index * kPointerSize);
+        VisitHeapObjectImpl(heap_object, field_index);
       }
     }
   }
 
+  void VisitCodeTarget(Code host, RelocInfo* rinfo) override {
+    Code target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+    VisitHeapObjectImpl(target, -1);
+  }
+
  private:
+  V8_INLINE void VisitHeapObjectImpl(HeapObject* heap_object, int field_index) {
+    DCHECK_LE(-1, field_index);
+    // The last parameter {field_offset} is only used to check some well-known
+    // skipped references, so passing -1 * kPointerSize for objects embedded
+    // into code is fine.
+    generator_->SetHiddenReference(parent_obj_, parent_, next_index_++,
+                                   heap_object, field_index * kPointerSize);
+  }
+
   V8HeapExplorer* generator_;
   HeapObject* parent_obj_;
   MaybeObjectSlot parent_start_;
   MaybeObjectSlot parent_end_;
   HeapEntry* parent_;
+  int next_index_;
 };
 
 void V8HeapExplorer::ExtractReferences(HeapEntry* entry, HeapObject* obj) {

src/snapshot/deserializer.cc

@@ -116,7 +116,10 @@ void Deserializer::VisitRootPointers(Root root, const char* description,
                                      FullObjectSlot start, FullObjectSlot end) {
   // The space must be new space.  Any other space would cause ReadChunk to try
   // to update the remembered using nullptr as the address.
-  ReadData(UnalignedSlot(start), UnalignedSlot(end), NEW_SPACE, kNullAddress);
+  // TODO(ishell): this will not work once we actually compress pointers.
+  STATIC_ASSERT(kTaggedSize == kSystemPointerSize);
+  ReadData(UnalignedSlot(start.address()), UnalignedSlot(end.address()),
+           NEW_SPACE, kNullAddress);
 }
 
 void Deserializer::Synchronize(VisitorSynchronization::SyncTag tag) {

src/visitors.h

@@ -121,7 +121,7 @@ class ObjectVisitor {
   // a rich interface for iterating over Code objects ...
 
   // Visits a code target in the instruction stream.
-  virtual void VisitCodeTarget(Code host, RelocInfo* rinfo);
+  virtual void VisitCodeTarget(Code host, RelocInfo* rinfo) = 0;
 
   // Visits a runtime entry in the instruction stream.
   virtual void VisitRuntimeEntry(Code host, RelocInfo* rinfo) {}