[logging] Handlify a few Objects to prevent UAF

The GC suspect was GetAbstractPC.

Fixed: v8:9990, v8:9987, chromium:1048038
Change-Id: I86a27e2098589dbf6af0808d6770c5e69987f1f7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2050394
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66259}

src/ic/ic.cc

@@ -96,10 +96,7 @@ void IC::TraceIC(const char* type, Handle<Object> name, State old_state,
                  State new_state) {
   if (V8_LIKELY(!TracingFlags::is_ic_stats_enabled())) return;
 
-  Map map;
-  if (!receiver_map().is_null()) {
-    map = *receiver_map();
-  }
+  Handle<Map> map = receiver_map();  // Might be empty.
 
   const char* modifier = "";
   if (state() == NO_FEEDBACK) {
@@ -116,7 +113,7 @@ void IC::TraceIC(const char* type, Handle<Object> name, State old_state,
 
   if (!(TracingFlags::ic_stats.load(std::memory_order_relaxed) &
         v8::tracing::TracingCategoryObserver::ENABLED_BY_TRACING)) {
-    LOG(isolate(), ICEvent(type, keyed_prefix, map, *name,
+    LOG(isolate(), ICEvent(type, keyed_prefix, map, name,
                            TransitionMarkFromState(old_state),
                            TransitionMarkFromState(new_state), modifier,
                            slow_stub_reason_));
@@ -125,6 +122,8 @@ void IC::TraceIC(const char* type, Handle<Object> name, State old_state,
 
   JavaScriptFrameIterator it(isolate());
   JavaScriptFrame* frame = it.frame();
+
+  DisallowHeapAllocation no_gc;
   JSFunction function = frame->function();
 
   ICStats::instance()->Begin();
@@ -150,11 +149,13 @@ void IC::TraceIC(const char* type, Handle<Object> name, State old_state,
   ic_info.state += TransitionMarkFromState(new_state);
   ic_info.state += modifier;
   ic_info.state += ")";
-  ic_info.map = reinterpret_cast<void*>(map.ptr());
   if (!map.is_null()) {
-    ic_info.is_dictionary_map = map.is_dictionary_map();
-    ic_info.number_of_own_descriptors = map.NumberOfOwnDescriptors();
-    ic_info.instance_type = std::to_string(map.instance_type());
+    ic_info.map = reinterpret_cast<void*>(map->ptr());
+    ic_info.is_dictionary_map = map->is_dictionary_map();
+    ic_info.number_of_own_descriptors = map->NumberOfOwnDescriptors();
+    ic_info.instance_type = std::to_string(map->instance_type());
+  } else {
+    ic_info.map = nullptr;
   }
   // TODO(lpy) Add name as key field in ICStats.
   ICStats::instance()->End();

src/logging/log.cc

@@ -1641,9 +1641,9 @@ void Logger::TickEvent(TickSample* sample, bool overflow) {
   msg.WriteToLogFile();
 }
 
-void Logger::ICEvent(const char* type, bool keyed, Map map, Object key,
-                     char old_state, char new_state, const char* modifier,
-                     const char* slow_stub_reason) {
+void Logger::ICEvent(const char* type, bool keyed, Handle<Map> map,
+                     Handle<Object> key, char old_state, char new_state,
+                     const char* modifier, const char* slow_stub_reason) {
   if (!log_->IsEnabled() || !FLAG_trace_ic) return;
   Log::MessageBuilder msg(log_.get());
   if (keyed) msg << "Keyed";
@@ -1652,13 +1652,13 @@ void Logger::ICEvent(const char* type, bool keyed, Map map, Object key,
   Address pc = isolate_->GetAbstractPC(&line, &column);
   msg << type << kNext << reinterpret_cast<void*>(pc) << kNext << line << kNext
       << column << kNext << old_state << kNext << new_state << kNext
-      << AsHex::Address(map.ptr()) << kNext;
-  if (key.IsSmi()) {
-    msg << Smi::ToInt(key);
-  } else if (key.IsNumber()) {
-    msg << key.Number();
-  } else if (key.IsName()) {
-    msg << Name::cast(key);
+      << AsHex::Address(map.is_null() ? kNullAddress : map->ptr()) << kNext;
+  if (key->IsSmi()) {
+    msg << Smi::ToInt(*key);
+  } else if (key->IsNumber()) {
+    msg << key->Number();
+  } else if (key->IsName()) {
+    msg << Name::cast(*key);
   }
   msg << kNext << modifier << kNext;
   if (slow_stub_reason != nullptr) {
@@ -1667,11 +1667,10 @@ void Logger::ICEvent(const char* type, bool keyed, Map map, Object key,
   msg.WriteToLogFile();
 }
 
-void Logger::MapEvent(const char* type, Map from, Map to, const char* reason,
-                      HeapObject name_or_sfi) {
-  DisallowHeapAllocation no_gc;
+void Logger::MapEvent(const char* type, Handle<Map> from, Handle<Map> to,
+                      const char* reason, Handle<HeapObject> name_or_sfi) {
   if (!log_->IsEnabled() || !FLAG_trace_maps) return;
-  if (!to.is_null()) MapDetails(to);
+  if (!to.is_null()) MapDetails(*to);
   int line = -1;
   int column = -1;
   Address pc = 0;
@@ -1681,15 +1680,16 @@ void Logger::MapEvent(const char* type, Map from, Map to, const char* reason,
   }
   Log::MessageBuilder msg(log_.get());
   msg << "map" << kNext << type << kNext << timer_.Elapsed().InMicroseconds()
-      << kNext << AsHex::Address(from.ptr()) << kNext
-      << AsHex::Address(to.ptr()) << kNext << AsHex::Address(pc) << kNext
-      << line << kNext << column << kNext << reason << kNext;
+      << kNext << AsHex::Address(from.is_null() ? kNullAddress : from->ptr())
+      << kNext << AsHex::Address(to.is_null() ? kNullAddress : to->ptr())
+      << kNext << AsHex::Address(pc) << kNext << line << kNext << column
+      << kNext << reason << kNext;
 
   if (!name_or_sfi.is_null()) {
-    if (name_or_sfi.IsName()) {
-      msg << Name::cast(name_or_sfi);
-    } else if (name_or_sfi.IsSharedFunctionInfo()) {
-      SharedFunctionInfo sfi = SharedFunctionInfo::cast(name_or_sfi);
+    if (name_or_sfi->IsName()) {
+      msg << Name::cast(*name_or_sfi);
+    } else if (name_or_sfi->IsSharedFunctionInfo()) {
+      SharedFunctionInfo sfi = SharedFunctionInfo::cast(*name_or_sfi);
       msg << sfi.DebugName();
 #if V8_SFI_HAS_UNIQUE_ID
       msg << " " << sfi.unique_id();

src/logging/log.h

@@ -222,14 +222,13 @@ class Logger : public CodeEventListener {
 
   void CodeNameEvent(Address addr, int pos, const char* code_name);
 
+  void ICEvent(const char* type, bool keyed, Handle<Map> map,
+               Handle<Object> key, char old_state, char new_state,
+               const char* modifier, const char* slow_stub_reason);
 
-  void ICEvent(const char* type, bool keyed, Map map, Object key,
-               char old_state, char new_state, const char* modifier,
-               const char* slow_stub_reason);
-
-  void MapEvent(const char* type, Map from, Map to,
+  void MapEvent(const char* type, Handle<Map> from, Handle<Map> to,
                 const char* reason = nullptr,
-                HeapObject name_or_sfi = HeapObject());
+                Handle<HeapObject> name_or_sfi = Handle<HeapObject>());
   void MapCreate(Map map);
   void MapDetails(Map map);
 

src/objects/js-objects.cc

@@ -3376,7 +3376,7 @@ void JSObject::MigrateSlowToFast(Handle<JSObject> object,
     // Check that it really works.
     DCHECK(object->HasFastProperties());
     if (FLAG_trace_maps) {
-      LOG(isolate, MapEvent("SlowToFast", *old_map, *new_map, reason));
+      LOG(isolate, MapEvent("SlowToFast", old_map, new_map, reason));
     }
     return;
   }
@@ -3466,7 +3466,7 @@ void JSObject::MigrateSlowToFast(Handle<JSObject> object,
   }
 
   if (FLAG_trace_maps) {
-    LOG(isolate, MapEvent("SlowToFast", *old_map, *new_map, reason));
+    LOG(isolate, MapEvent("SlowToFast", old_map, new_map, reason));
   }
   // Transform the object.
   object->synchronized_set_map(*new_map);
@@ -5169,8 +5169,9 @@ void JSFunction::SetInitialMap(Handle<JSFunction> function, Handle<Map> map,
   function->set_prototype_or_initial_map(*map);
   map->SetConstructor(*function);
   if (FLAG_trace_maps) {
-    LOG(function->GetIsolate(), MapEvent("InitialMap", Map(), *map, "",
-                                         function->shared().DebugName()));
+    LOG(function->GetIsolate(), MapEvent("InitialMap", Handle<Map>(), map, "",
+                                         handle(function->shared().DebugName(),
+                                                function->GetIsolate())));
   }
 }
 

src/objects/map.cc

@@ -595,7 +595,7 @@ void Map::DeprecateTransitionTree(Isolate* isolate) {
   DCHECK(!constructor_or_backpointer().IsFunctionTemplateInfo());
   set_is_deprecated(true);
   if (FLAG_trace_maps) {
-    LOG(isolate, MapEvent("Deprecate", *this, Map()));
+    LOG(isolate, MapEvent("Deprecate", handle(*this, isolate), Handle<Map>()));
   }
   dependent_code().DeoptimizeDependentCodeGroup(
       isolate, DependentCode::kTransitionGroup);
@@ -1512,7 +1512,7 @@ Handle<Map> Map::Normalize(Isolate* isolate, Handle<Map> fast_map,
     }
   }
   if (FLAG_trace_maps) {
-    LOG(isolate, MapEvent("Normalize", *fast_map, *new_map, reason));
+    LOG(isolate, MapEvent("Normalize", fast_map, new_map, reason));
   }
   fast_map->NotifyLeafMapLayoutChange(isolate);
   return new_map;
@@ -1698,12 +1698,12 @@ void Map::ConnectTransition(Isolate* isolate, Handle<Map> parent,
   if (parent->is_prototype_map()) {
     DCHECK(child->is_prototype_map());
     if (FLAG_trace_maps) {
-      LOG(isolate, MapEvent("Transition", *parent, *child, "prototype", *name));
+      LOG(isolate, MapEvent("Transition", parent, child, "prototype", name));
     }
   } else {
     TransitionsAccessor(isolate, parent).Insert(name, child, flag);
     if (FLAG_trace_maps) {
-      LOG(isolate, MapEvent("Transition", *parent, *child, "", *name));
+      LOG(isolate, MapEvent("Transition", parent, child, "", name));
     }
   }
 }
@@ -1749,8 +1749,8 @@ Handle<Map> Map::CopyReplaceDescriptors(
       (map->is_prototype_map() ||
        !(flag == INSERT_TRANSITION &&
          TransitionsAccessor(isolate, map).CanHaveMoreTransitions()))) {
-    LOG(isolate, MapEvent("ReplaceDescriptors", *map, *result, reason,
-                          maybe_name.is_null() ? Name() : *name));
+    LOG(isolate, MapEvent("ReplaceDescriptors", map, result, reason,
+                          maybe_name.is_null() ? Handle<HeapObject>() : name));
   }
   return result;
 }

src/runtime/runtime-classes.cc

@@ -661,11 +661,14 @@ MaybeHandle<Object> DefineClass(
     return MaybeHandle<Object>();
   }
   if (FLAG_trace_maps) {
+    Handle<Map> empty_map;
     LOG(isolate,
-        MapEvent("InitialMap", Map(), constructor->map(),
-                 "init class constructor", constructor->shared().DebugName()));
-    LOG(isolate, MapEvent("InitialMap", Map(), prototype->map(),
-                          "init class prototype"));
+        MapEvent("InitialMap", empty_map, handle(constructor->map(), isolate),
+                 "init class constructor",
+                 handle(constructor->shared().DebugName(), isolate)));
+    LOG(isolate,
+        MapEvent("InitialMap", empty_map, handle(prototype->map(), isolate),
+                 "init class prototype"));
   }
 
   return prototype;