[ubsan] Replace a few more Object** with alternatives

Either Address* or Handle<Object> or ObjectSlot, depending on
circumstances.

Bug: v8:3770
Change-Id: Id00dfede6eb92ec30b658c0090b5310548ba5162
Reviewed-on: https://chromium-review.googlesource.com/c/1379228
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58282}

src/deoptimizer.cc

@@ -2942,9 +2942,10 @@ Address TranslatedState::ComputeArgumentsPosition(Address input_frame_pointer,
   if (parent_frame_type ==
       StackFrame::TypeToMarker(StackFrame::ARGUMENTS_ADAPTOR)) {
     if (length)
-      *length = Smi::cast(*reinterpret_cast<Object**>(
+      *length = Smi::cast(FullObjectSlot(
                               parent_frame_pointer +
-                              ArgumentsAdaptorFrameConstants::kLengthOffset))
+                              ArgumentsAdaptorFrameConstants::kLengthOffset)
+                              .load())
                     ->value();
     arguments_frame = parent_frame_pointer;
   } else {
@@ -3002,8 +3003,8 @@ void TranslatedState::CreateArgumentsElementsTranslatedValues(
     Address argument_slot = arguments_frame +
                             CommonFrameConstants::kFixedFrameSizeAboveFp +
                             i * kPointerSize;
-    frame.Add(TranslatedValue::NewTagged(
-        this, *reinterpret_cast<Object**>(argument_slot)));
+    frame.Add(
+        TranslatedValue::NewTagged(this, FullObjectSlot(argument_slot).load()));
   }
 }
 

src/handles-inl.h

@@ -31,7 +31,7 @@ Handle<T> Handle<T>::New(T object, Isolate* isolate) {
 template <typename T>
 template <typename S>
 const Handle<T> Handle<T>::cast(Handle<S> that) {
-  T::cast(*reinterpret_cast<Object**>(that.location()));
+  T::cast(FullObjectSlot(that.location()).load());
   return Handle<T>(that.location_);
 }
 

src/heap/factory.cc

@@ -105,7 +105,8 @@ void InitializeCode(Heap* heap, Handle<Code> code, int object_size,
   }
 
   // Migrate generated code.
-  // The generated code can contain Object** values (typically from handles)
+  // The generated code can contain embedded objects (typically from handles)
+  // in a pointer-to-tagged-value format (i.e. with indirection like a handle)
   // that are dereferenced during the copy to point directly to the actual heap
   // objects. These pointers can include references to the code object itself,
   // through the self_reference parameter.

src/profiler/heap-profiler.cc

@@ -52,12 +52,10 @@ void HeapProfiler::DefineWrapperClass(
   wrapper_callbacks_[class_id] = callback;
 }
 
-
 v8::RetainedObjectInfo* HeapProfiler::ExecuteWrapperClassCallback(
-    uint16_t class_id, Object** wrapper) {
+    uint16_t class_id, Handle<Object> wrapper) {
   if (wrapper_callbacks_.size() <= class_id) return nullptr;
-  return wrapper_callbacks_[class_id](
-      class_id, Utils::ToLocal(Handle<Object>(wrapper)));
+  return wrapper_callbacks_[class_id](class_id, Utils::ToLocal(wrapper));
 }
 
 void HeapProfiler::SetGetRetainerInfosCallback(

src/profiler/heap-profiler.h

@@ -65,7 +65,7 @@ class HeapProfiler : public HeapObjectAllocationTracker {
       uint16_t class_id, v8::HeapProfiler::WrapperInfoCallback callback);
 
   v8::RetainedObjectInfo* ExecuteWrapperClassCallback(uint16_t class_id,
-                                                      Object** wrapper);
+                                                      Handle<Object> wrapper);
 
   void SetGetRetainerInfosCallback(
       v8::HeapProfiler::GetRetainerInfosCallback callback);

src/profiler/heap-snapshot-generator.cc

@@ -1834,9 +1834,7 @@ class GlobalHandlesExtractor : public PersistentHandleVisitor {
   void VisitPersistentHandle(Persistent<Value>* value,
                              uint16_t class_id) override {
     Handle<Object> object = Utils::OpenPersistent(value);
-    // TODO(3770): Get rid of Object** here.
-    explorer_->VisitSubtreeWrapper(
-        reinterpret_cast<Object**>(object.location()), class_id);
+    explorer_->VisitSubtreeWrapper(object, class_id);
   }
 
  private:
@@ -2171,7 +2169,8 @@ void NativeObjectsExplorer::SetRootNativeRootsReference() {
   }
 }
 
-void NativeObjectsExplorer::VisitSubtreeWrapper(Object** p, uint16_t class_id) {
+void NativeObjectsExplorer::VisitSubtreeWrapper(Handle<Object> p,
+                                                uint16_t class_id) {
   if (in_groups_.count(*p)) return;
   v8::RetainedObjectInfo* info =
       isolate_->heap_profiler()->ExecuteWrapperClassCallback(class_id, p);

src/profiler/heap-snapshot-generator.h

@@ -466,7 +466,7 @@ class NativeObjectsExplorer {
   void SetRootNativeRootsReference();
   void SetWrapperNativeReferences(HeapObject* wrapper,
                                       v8::RetainedObjectInfo* info);
-  void VisitSubtreeWrapper(Object** p, uint16_t class_id);
+  void VisitSubtreeWrapper(Handle<Object> p, uint16_t class_id);
 
   struct RetainedInfoHasher {
     std::size_t operator()(v8::RetainedObjectInfo* info) const {

test/cctest/heap/test-heap.cc

@@ -563,14 +563,15 @@ TEST(WeakGlobalUnmodifiedApiHandlesScavenge) {
     HandleScope scope(isolate);
 
     // Create an Api object that is unmodified.
-    auto function = FunctionTemplate::New(context->GetIsolate())
-                        ->GetFunction(context.local())
-                        .ToLocalChecked();
-    auto i = function->NewInstance(context.local()).ToLocalChecked();
+    Local<v8::Function> function = FunctionTemplate::New(context->GetIsolate())
+                                       ->GetFunction(context.local())
+                                       .ToLocalChecked();
+    Local<v8::Object> i =
+        function->NewInstance(context.local()).ToLocalChecked();
     Handle<Object> u = factory->NewNumber(1.12344);
 
     h1 = global_handles->Create(*u);
-    h2 = global_handles->Create(*(reinterpret_cast<internal::Object**>(*i)));
+    h2 = global_handles->Create(*(reinterpret_cast<internal::Address*>(*i)));
   }
 
   std::pair<Handle<Object>*, int> handle_and_id(&h2, 1234);
@@ -611,7 +612,7 @@ TEST(WeakGlobalApiHandleModifiedMapScavenge) {
         function_template->GetFunction(context.local()).ToLocalChecked();
     auto i = function->NewInstance(context.local()).ToLocalChecked();
 
-    h1 = global_handles->Create(*(reinterpret_cast<internal::Object**>(*i)));
+    h1 = global_handles->Create(*(reinterpret_cast<internal::Address*>(*i)));
   }
 
   std::pair<Handle<Object>*, int> handle_and_id(&h1, 1234);
@@ -655,7 +656,7 @@ TEST(WeakGlobalApiHandleWithElementsScavenge) {
         function_template->GetFunction(context.local()).ToLocalChecked();
     auto i = function->NewInstance(context.local()).ToLocalChecked();
 
-    h1 = global_handles->Create(*(reinterpret_cast<internal::Object**>(*i)));
+    h1 = global_handles->Create(*(reinterpret_cast<internal::Address*>(*i)));
   }
 
   std::pair<Handle<Object>*, int> handle_and_id(&h1, 1234);
@@ -1633,8 +1634,8 @@ TEST(TestInternalWeakLists) {
   // Dispose the native contexts one by one.
   for (int i = 0; i < kNumTestContexts; i++) {
     // TODO(dcarney): is there a better way to do this?
-    i::Object** unsafe = reinterpret_cast<i::Object**>(*ctx[i]);
-    *unsafe = ReadOnlyRoots(CcTest::heap()).undefined_value();
+    i::Address* unsafe = reinterpret_cast<i::Address*>(*ctx[i]);
+    *unsafe = ReadOnlyRoots(CcTest::heap()).undefined_value()->ptr();
     ctx[i].Clear();
 
     // Scavenge treats these references as strong.
@@ -5105,8 +5106,8 @@ TEST(OldSpaceAllocationCounter) {
 
 static void CheckLeak(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Isolate* isolate = CcTest::i_isolate();
-  Object* message =
-      *reinterpret_cast<Object**>(isolate->pending_message_obj_address());
+  ObjectPtr message(
+      *reinterpret_cast<Address*>(isolate->pending_message_obj_address()));
   CHECK(message->IsTheHole(isolate));
 }
 

test/cctest/test-api.h

@@ -15,7 +15,7 @@
 template <typename T>
 static void CheckReturnValue(const T& t, i::Address callback) {
   v8::ReturnValue<v8::Value> rv = t.GetReturnValue();
-  i::Object** o = *reinterpret_cast<i::Object***>(&rv);
+  i::FullObjectSlot o(*reinterpret_cast<i::Address*>(&rv));
   CHECK_EQ(CcTest::isolate(), t.GetIsolate());
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(t.GetIsolate());
   CHECK_EQ(t.GetIsolate(), rv.GetIsolate());