[Backport] CVE-2020-6430 2/2

Only flush feedback vector on bytecode flush

When bytecode is flushed we also want to flush the feedback vectors to
save memory. There was a bug in this code and we flushed
ClosureFeedbackCellArray too. Flushing ClosureFeedbackCellArrays causes
the closures created by this function before and after the bytecode
flush to have different feedback cells and hence different feedback
vectors. This cl fixes it so we only flush feedback vectors on a
bytecode flush.

Also this cl pretenures ClosureFeedbackCellArrays. Only FeedbackCells
and FeedbackVectors can contain ClosureFeedbackCellArrays which are
pretenured, so it is better to pretenure ClosureFeedbackCellArrays as
well.

Bug: chromium:1031479
Change-Id: Ie2757867550a3ed9814b8e2130f9b8bcf196fd32
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

7 files changed, 38 insertions, 20 deletions

diff --git a/chromium/v8/src/heap/factory.cc b/chromium/v8/src/heap/factory.cc
index 82d24c35923..17788d51281 100644
--- a/chromium/v8/src/heap/factory.cc
+++ b/chromium/v8/src/heap/factory.cc
@@ -503,7 +503,7 @@ Handle<ClosureFeedbackCellArray> Factory::NewClosureFeedbackCellArray(
   Handle<ClosureFeedbackCellArray> feedback_cell_array =
       NewFixedArrayWithMap<ClosureFeedbackCellArray>(
           RootIndex::kClosureFeedbackCellArrayMap, length,
-          AllocationType::kYoung);
+          AllocationType::kOld);
 
   return feedback_cell_array;
 }
diff --git a/chromium/v8/src/heap/mark-compact.cc b/chromium/v8/src/heap/mark-compact.cc
index d5494efb03b..bcc01b1587d 100644
--- a/chromium/v8/src/heap/mark-compact.cc
+++ b/chromium/v8/src/heap/mark-compact.cc
@@ -2196,7 +2196,11 @@ void MarkCompactCollector::ClearFlushedJsFunctions() {
   JSFunction flushed_js_function;
   while (weak_objects_.flushed_js_functions.Pop(kMainThreadTask,
                                                 &flushed_js_function)) {
-    flushed_js_function.ResetIfBytecodeFlushed();
+    auto gc_notify_updated_slot = [](HeapObject object, ObjectSlot slot,
+                                     Object target) {
+      RecordSlot(object, slot, HeapObject::cast(target));
+    };
+    flushed_js_function.ResetIfBytecodeFlushed(gc_notify_updated_slot);
   }
 }
 
diff --git a/chromium/v8/src/objects/feedback-cell-inl.h b/chromium/v8/src/objects/feedback-cell-inl.h
index 188666d4626..9a570092e8c 100644
--- a/chromium/v8/src/objects/feedback-cell-inl.h
+++ b/chromium/v8/src/objects/feedback-cell-inl.h
@@ -26,9 +26,21 @@ void FeedbackCell::clear_padding() {
          FeedbackCell::kAlignedSize - FeedbackCell::kUnalignedSize);
 }
 
-void FeedbackCell::reset() {
-  set_value(GetReadOnlyRoots().undefined_value());
+void FeedbackCell::reset_feedback_vector(
+    base::Optional<std::function<void(HeapObject object, ObjectSlot slot,
+                                      HeapObject target)>>
+        gc_notify_updated_slot) {
   set_interrupt_budget(FeedbackCell::GetInitialInterruptBudget());
+  if (value().IsUndefined() || value().IsClosureFeedbackCellArray()) return;
+
+  CHECK(value().IsFeedbackVector());
+  ClosureFeedbackCellArray closure_feedback_cell_array =
+      FeedbackVector::cast(value()).closure_feedback_cell_array();
+  set_value(closure_feedback_cell_array);
+  if (gc_notify_updated_slot) {
+    (*gc_notify_updated_slot)(*this, RawField(FeedbackCell::kValueOffset),
+                              closure_feedback_cell_array);
+  }
 }
 
 }  // namespace internal
diff --git a/chromium/v8/src/objects/feedback-cell.h b/chromium/v8/src/objects/feedback-cell.h
index 669efaeaeca..29304f709dd 100644
--- a/chromium/v8/src/objects/feedback-cell.h
+++ b/chromium/v8/src/objects/feedback-cell.h
@@ -34,7 +34,10 @@ class FeedbackCell : public TorqueGeneratedFeedbackCell<FeedbackCell, Struct> {
   static const int kAlignedSize = RoundUp<kObjectAlignment>(int{kSize});
 
   inline void clear_padding();
-  inline void reset();
+  inline void reset_feedback_vector(
+      base::Optional<std::function<void(HeapObject object, ObjectSlot slot,
+                                        HeapObject target)>>
+          gc_notify_updated_slot = base::nullopt);
 
   using BodyDescriptor =
       FixedBodyDescriptor<kValueOffset, kInterruptBudgetOffset, kAlignedSize>;
diff --git a/chromium/v8/src/objects/js-objects-inl.h b/chromium/v8/src/objects/js-objects-inl.h
index f2d4245d376..99e48808a98 100644
--- a/chromium/v8/src/objects/js-objects-inl.h
+++ b/chromium/v8/src/objects/js-objects-inl.h
@@ -701,12 +701,15 @@ bool JSFunction::NeedsResetDueToFlushedBytecode() {
          code.builtin_index() != Builtins::kCompileLazy;
 }
 
-void JSFunction::ResetIfBytecodeFlushed() {
+void JSFunction::ResetIfBytecodeFlushed(
+    base::Optional<std::function<void(HeapObject object, ObjectSlot slot,
+                                      HeapObject target)>>
+        gc_notify_updated_slot) {
   if (FLAG_flush_bytecode && NeedsResetDueToFlushedBytecode()) {
     // Bytecode was flushed and function is now uncompiled, reset JSFunction
     // by setting code to CompileLazy and clearing the feedback vector.
     set_code(GetIsolate()->builtins()->builtin(i::Builtins::kCompileLazy));
-    raw_feedback_cell().reset();
+    raw_feedback_cell().reset_feedback_vector(gc_notify_updated_slot);
   }
 }
 
diff --git a/chromium/v8/src/objects/js-objects.h b/chromium/v8/src/objects/js-objects.h
index 1dad4cf0dd0..99037c220ab 100644
--- a/chromium/v8/src/objects/js-objects.h
+++ b/chromium/v8/src/objects/js-objects.h
@@ -1079,7 +1079,10 @@ class JSFunction : public JSFunctionOrBoundFunction {
 
   // Resets function to clear compiled data after bytecode has been flushed.
   inline bool NeedsResetDueToFlushedBytecode();
-  inline void ResetIfBytecodeFlushed();
+  inline void ResetIfBytecodeFlushed(
+      base::Optional<std::function<void(HeapObject object, ObjectSlot slot,
+                                        HeapObject target)>>
+          gc_notify_updated_slot = base::nullopt);
 
   DECL_GETTER(has_prototype_slot, bool)
 
diff --git a/chromium/v8/src/runtime/runtime-compiler.cc b/chromium/v8/src/runtime/runtime-compiler.cc
index 41dd8638d88..29ad7755b9f 100644
--- a/chromium/v8/src/runtime/runtime-compiler.cc
+++ b/chromium/v8/src/runtime/runtime-compiler.cc
@@ -188,18 +188,11 @@ static bool IsSuitableForOnStackReplacement(Isolate* isolate,
                                             Handle<JSFunction> function) {
   // Keep track of whether we've succeeded in optimizing.
   if (function->shared().optimization_disabled()) return false;
-  // TODO(chromium:1031479): When we flush the bytecode we also reset the
-  // feedback_cell that holds feedback vector / closure feedback cell array.
-  // Closure feedback cell array holds the feedback cells that should be used
-  // when creating closures from the given function. If we happen to execute the
-  // function again, we re-compile and re-intialize this array. So, the closures
-  // created by this function after the flush would have different feedback
-  // cells when compared to the corresponding closures created before the flush.
-  // Currently, OSR triggering mechanism is tied to the bytecode array. So, it
-  // is possible to mark one closure for OSR but optimize a different one.
-  // These two factors could cause us to OSR functions that don't have feedback
-  // vector. This is a temporary fix and we should fix one (or both) of the two
-  // causes.
+  // TODO(chromium:1031479): Currently, OSR triggering mechanism is tied to the
+  // bytecode array. So, it might be possible to mark closure in one native
+  // context and optimize a closure from a different native context. So check if
+  // there is a feedback vector before OSRing. We don't expect this to happen
+  // often.
   if (!function->has_feedback_vector()) return false;
   // If we are trying to do OSR when there are already optimized
   // activations of the function, it means (a) the function is directly or