[factory] Flush instructions as late as possible

As my mum used to say: When it comes to flushing, later is better than early.

Tentative fix for Android invoke crashers with write protection code
enabled.

Bug: chromium:842862
Change-Id: Ib37115883a6fa615c9514aeb543dc3527335803a
Reviewed-on: https://chromium-review.googlesource.com/1059673Reviewed-by: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53202}

src/heap/factory.cc

@@ -94,7 +94,7 @@ void InitializeCode(Handle<Code> code, int object_size, const CodeDesc& desc,
   // 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.
-  code->CopyFrom(desc);
+  code->CopyFromNoFlush(desc);
 
   code->clear_padding();
 
@@ -2468,31 +2468,36 @@ MaybeHandle<Code> Factory::TryNewCode(
   Handle<DeoptimizationData> deopt_data =
       maybe_deopt_data.is_null() ? DeoptimizationData::Empty(isolate())
                                  : maybe_deopt_data.ToHandleChecked();
+  Handle<Code> code;
+  {
+    int object_size = ComputeCodeObjectSize(desc);
 
-  int object_size = ComputeCodeObjectSize(desc);
-
-  Heap* heap = isolate()->heap();
-  CodePageCollectionMemoryModificationScope code_allocation(heap);
-  HeapObject* result = heap->AllocateRawWithLigthRetry(object_size, CODE_SPACE);
+    Heap* heap = isolate()->heap();
+    CodePageCollectionMemoryModificationScope code_allocation(heap);
+    HeapObject* result =
+        heap->AllocateRawWithLigthRetry(object_size, CODE_SPACE);
 
-  // Return an empty handle if we cannot allocate the code object.
-  if (!result) return MaybeHandle<Code>();
+    // Return an empty handle if we cannot allocate the code object.
+    if (!result) return MaybeHandle<Code>();
 
-  if (movability == kImmovable) {
-    result = heap->EnsureImmovableCode(result, object_size);
-  }
+    if (movability == kImmovable) {
+      result = heap->EnsureImmovableCode(result, object_size);
+    }
 
-  // The code object has not been fully initialized yet.  We rely on the
-  // fact that no allocation will happen from this point on.
-  DisallowHeapAllocation no_gc;
+    // The code object has not been fully initialized yet.  We rely on the
+    // fact that no allocation will happen from this point on.
+    DisallowHeapAllocation no_gc;
 
-  result->set_map_after_allocation(*code_map(), SKIP_WRITE_BARRIER);
-  Handle<Code> code(Code::cast(result), isolate());
+    result->set_map_after_allocation(*code_map(), SKIP_WRITE_BARRIER);
+    code = handle(Code::cast(result), isolate());
 
-  InitializeCode(code, object_size, desc, kind, self_ref, builtin_index,
-                 source_position_table, deopt_data, reloc_info, data_container,
-                 stub_key, is_turbofanned, stack_slots, safepoint_table_offset,
-                 handler_table_offset);
+    InitializeCode(code, object_size, desc, kind, self_ref, builtin_index,
+                   source_position_table, deopt_data, reloc_info,
+                   data_container, stub_key, is_turbofanned, stack_slots,
+                   safepoint_table_offset, handler_table_offset);
+  }
+  // Flush the instruction cache after changing the permissions.
+  code->FlushICache();
 
   return code;
 }
@@ -2514,28 +2519,33 @@ Handle<Code> Factory::NewCode(
       maybe_deopt_data.is_null() ? DeoptimizationData::Empty(isolate())
                                  : maybe_deopt_data.ToHandleChecked();
 
-  int object_size = ComputeCodeObjectSize(desc);
+  Handle<Code> code;
+  {
+    int object_size = ComputeCodeObjectSize(desc);
 
-  Heap* heap = isolate()->heap();
-  CodePageCollectionMemoryModificationScope code_allocation(heap);
-  HeapObject* result =
-      heap->AllocateRawWithRetryOrFail(object_size, CODE_SPACE);
+    Heap* heap = isolate()->heap();
+    CodePageCollectionMemoryModificationScope code_allocation(heap);
+    HeapObject* result =
+        heap->AllocateRawWithRetryOrFail(object_size, CODE_SPACE);
 
-  if (movability == kImmovable) {
-    result = heap->EnsureImmovableCode(result, object_size);
-  }
+    if (movability == kImmovable) {
+      result = heap->EnsureImmovableCode(result, object_size);
+    }
 
-  // The code object has not been fully initialized yet.  We rely on the
-  // fact that no allocation will happen from this point on.
-  DisallowHeapAllocation no_gc;
+    // The code object has not been fully initialized yet.  We rely on the
+    // fact that no allocation will happen from this point on.
+    DisallowHeapAllocation no_gc;
 
-  result->set_map_after_allocation(*code_map(), SKIP_WRITE_BARRIER);
-  Handle<Code> code(Code::cast(result), isolate());
+    result->set_map_after_allocation(*code_map(), SKIP_WRITE_BARRIER);
+    code = handle(Code::cast(result), isolate());
 
-  InitializeCode(code, object_size, desc, kind, self_ref, builtin_index,
-                 source_position_table, deopt_data, reloc_info, data_container,
-                 stub_key, is_turbofanned, stack_slots, safepoint_table_offset,
-                 handler_table_offset);
+    InitializeCode(code, object_size, desc, kind, self_ref, builtin_index,
+                   source_position_table, deopt_data, reloc_info,
+                   data_container, stub_key, is_turbofanned, stack_slots,
+                   safepoint_table_offset, handler_table_offset);
+  }
+  // Flush the instruction cache after changing the permissions.
+  code->FlushICache();
 
   return code;
 }

src/objects.cc

@@ -14009,8 +14009,16 @@ void Code::Relocate(intptr_t delta) {
   Assembler::FlushICache(raw_instruction_start(), raw_instruction_size());
 }
 
+void Code::FlushICache() const {
+  Assembler::FlushICache(raw_instruction_start(), raw_instruction_size());
+}
 
 void Code::CopyFrom(const CodeDesc& desc) {
+  CopyFromNoFlush(desc);
+  FlushICache();
+}
+
+void Code::CopyFromNoFlush(const CodeDesc& desc) {
   // copy code
   CopyBytes(reinterpret_cast<byte*>(raw_instruction_start()), desc.buffer,
             static_cast<size_t>(desc.instr_size));
@@ -14060,7 +14068,6 @@ void Code::CopyFrom(const CodeDesc& desc) {
       it.rinfo()->apply(delta);
     }
   }
-  Assembler::FlushICache(raw_instruction_start(), raw_instruction_size());
 }
 
 

src/objects/code.h

@@ -306,6 +306,13 @@ class Code : public HeapObject {
   // Migrate code described by desc.
   void CopyFrom(const CodeDesc& desc);
 
+  // Migrate code from desc without flushing the instruction cache.
+  void CopyFromNoFlush(const CodeDesc& desc);
+
+  // Flushes the instruction cache for the executable instructions of this code
+  // object.
+  void FlushICache() const;
+
   // Returns the object size for a given body (used for allocation).
   static int SizeFor(int body_size) {
     DCHECK_SIZE_TAG_ALIGNED(body_size);