[in-place weak refs] Fix weak_objects_in_code handling.

If it points to a new space object which doesn't get scavenged, we need to drop
the reference.

BUG=v8:7308, v8:7768

Change-Id: I4485a7abcac3a26781811cc9bf134fd80e5f35b5
Reviewed-on: https://chromium-review.googlesource.com/1069127
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53287}

src/heap/incremental-marking.cc

@@ -653,9 +653,10 @@ void IncrementalMarking::UpdateMarkingWorklistAfterScavenge() {
 }
 
 void IncrementalMarking::UpdateWeakReferencesAfterScavenge() {
+  Heap* heap = heap_;
   weak_objects_->weak_references.Update(
-      [](std::pair<HeapObject*, HeapObjectReference**> slot_in,
-         std::pair<HeapObject*, HeapObjectReference**>* slot_out) -> bool {
+      [heap](std::pair<HeapObject*, HeapObjectReference**> slot_in,
+             std::pair<HeapObject*, HeapObjectReference**>* slot_out) -> bool {
         HeapObject* heap_obj = slot_in.first;
         MapWord map_word = heap_obj->map_word();
         if (map_word.IsForwardingAddress()) {
@@ -669,7 +670,7 @@ void IncrementalMarking::UpdateWeakReferencesAfterScavenge() {
           slot_out->second = reinterpret_cast<HeapObjectReference**>(new_slot);
           return true;
         }
-        if (heap_obj->GetHeap()->InNewSpace(heap_obj)) {
+        if (heap->InNewSpace(heap_obj)) {
           // The new space object containing the weak reference died.
           return false;
         }
@@ -677,16 +678,21 @@ void IncrementalMarking::UpdateWeakReferencesAfterScavenge() {
         return true;
       });
   weak_objects_->weak_objects_in_code.Update(
-      [](std::pair<HeapObject*, Code*> slot_in,
-         std::pair<HeapObject*, Code*>* slot_out) -> bool {
+      [heap](std::pair<HeapObject*, Code*> slot_in,
+             std::pair<HeapObject*, Code*>* slot_out) -> bool {
         HeapObject* heap_obj = slot_in.first;
         MapWord map_word = heap_obj->map_word();
         if (map_word.IsForwardingAddress()) {
           slot_out->first = map_word.ToForwardingAddress();
           slot_out->second = slot_in.second;
-        } else {
-          *slot_out = slot_in;
+          return true;
         }
+        if (heap->InNewSpace(heap_obj)) {
+          // The new space object which is referred weakly is dead (i.e., didn't
+          // get scavenged). Drop references to it.
+          return false;
+        }
+        *slot_out = slot_in;
         return true;
       });
 }

test/cctest/heap/heap-tester.h

@@ -96,6 +96,8 @@ class HeapTester {
   static AllocationResult AllocateMapForTest(v8::internal::Isolate* isolate);
   static AllocationResult AllocateFixedArrayForTest(Heap* heap, int length,
                                                     PretenureFlag pretenure);
+
+  static void UncommitFromSpace(Heap* heap);
 };
 
 }  // namespace heap

test/cctest/heap/test-heap.cc

@@ -6137,6 +6137,11 @@ UNINITIALIZED_TEST(OutOfMemoryLargeObjects) {
   reinterpret_cast<v8::Isolate*>(isolate)->Dispose();
 }
 
+void HeapTester::UncommitFromSpace(Heap* heap) {
+  heap->UncommitFromSpace();
+  heap->memory_allocator()->unmapper()->EnsureUnmappingCompleted();
+}
+
 }  // namespace heap
 }  // namespace internal
 }  // namespace v8

test/cctest/heap/test-weak-references.cc

@@ -7,6 +7,7 @@
 #include "src/heap/factory.h"
 #include "src/isolate.h"
 #include "test/cctest/cctest.h"
+#include "test/cctest/heap/heap-tester.h"
 #include "test/cctest/heap/heap-utils.h"
 
 namespace v8 {
@@ -521,6 +522,49 @@ TEST(WeakArrayListBasic) {
   CHECK_EQ(Smi::ToInt(array->Get(7)->ToSmi()), 7);
 }
 
+TEST(Regress7768) {
+  i::FLAG_allow_natives_syntax = true;
+  i::FLAG_turbo_inlining = false;
+  if (!FLAG_incremental_marking) {
+    return;
+  }
+  ManualGCScope manual_gc_scope;
+  CcTest::InitializeVM();
+  LocalContext context;
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+  HandleScope outer_scope(isolate);
+  // Create an optimized code which will contain a weak reference to another
+  // function ("f"). The weak reference is the only reference to the function.
+  CompileRun(
+      "function myfunc(f) { f(); } "
+      "(function wrapper() { "
+      "   function f() {}; myfunc(f); myfunc(f); "
+      "   %OptimizeFunctionOnNextCall(myfunc); myfunc(f); "
+      "   %ClearFunctionFeedback(wrapper);"
+      "})(); "
+      "%ClearFunctionFeedback(myfunc);");
+
+  // Do marking steps; this will store the objects pointed by myfunc for later
+  // processing.
+  SimulateIncrementalMarking(heap, true);
+
+  // Deoptimize the code; now the pointers inside it will be replaced with
+  // undefined, and the weak_objects_in_code is the only place pointing to the
+  // function f.
+  CompileRun("%DeoptimizeFunction(myfunc);");
+
+  // The object pointed to by the weak reference won't be scavenged.
+  CcTest::CollectGarbage(NEW_SPACE);
+
+  // Make sure the memory where it's stored is invalidated, so that we'll crash
+  // if we try to access it.
+  HeapTester::UncommitFromSpace(heap);
+
+  // This used to crash when processing the dead weak reference.
+  CcTest::CollectAllGarbage();
+}
+
 }  // namespace heap
 }  // namespace internal
 }  // namespace v8