Properly report OOM when deoptimizer allocation fails

src/deoptimizer.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #include "src/accessors.h"
 #include "src/codegen.h"
 #include "src/cpu-profiler.h"
 #include "src/deoptimizer.h"
@@ skipping 2763 matching lines @@
 }
 
 
 void Deoptimizer::AddDoubleValue(intptr_t slot_address, double value) {
   HeapNumberMaterializationDescriptor<Address> value_desc(
       reinterpret_cast<Address>(slot_address), value);
   deferred_heap_numbers_.Add(value_desc);
 }
 
 
 void Deoptimizer::EnsureCodeForDeoptimizationEntry(Isolate* isolate,

[Michael Starzinger, 2015/04/16 15:32:23]

As mentioned offline: I am not entirely sure that it is safe to trigger a GC
within this method. It seems the call-site in
Deoptimizer::PatchCodeForDeoptimization is un-handlified and has some raw
pointers into the heap (i.e. deopt_data pointing to a DeoptimizationInputData)
that would become stale. Can we guarantee that this call-site will never trigger
an expansion of the deopt entry table?

[Erik Corry Chromium.org, 2015/04/16 16:39:10]

Nice catch.  Removed the GC, now just report the OOM.

                                                    BailoutType type,
                                                    int max_entry_id) {
   // We cannot run this if the serializer is enabled because this will
   // cause us to emit relocation information for the external
   // references. This is fine because the deoptimizer's code section
   // isn't meant to be serialized at all.
   CHECK(type == EAGER || type == SOFT || type == LAZY);
   DeoptimizerData* data = isolate->deoptimizer_data();
   int entry_count = data->deopt_entry_code_entries_[type];
   if (max_entry_id < entry_count) return;
   entry_count = Max(entry_count, Deoptimizer::kMinNumberOfEntries);
   while (max_entry_id >= entry_count) entry_count *= 2;
   CHECK(entry_count <= Deoptimizer::kMaxNumberOfEntries);
 
   MacroAssembler masm(isolate, NULL, 16 * KB);
   masm.set_emit_debug_code(false);
   GenerateDeoptimizationEntries(&masm, entry_count, type);
   CodeDesc desc;
   masm.GetCode(&desc);
   DCHECK(!RelocInfo::RequiresRelocation(desc));
 
   MemoryChunk* chunk = data->deopt_entry_code_[type];
   CHECK(static_cast<int>(Deoptimizer::GetMaxDeoptTableSize()) >=
         desc.instr_size);
-  chunk->CommitArea(desc.instr_size);
+  if (!chunk->CommitArea(desc.instr_size)) {
+    isolate->heap()->CollectAllAvailableGarbage(
+        "Allocation for deoptimizer failed");
+    if (!chunk->CommitArea(desc.instr_size)) {
+      V8::FatalProcessOutOfMemory(

[Michael Starzinger, 2015/04/16 15:32:23]

I like that we are properly reporting the allocation failure via
V8::FatalProcessOutOfMemory now, we should definitely land that part.

+          "Deoptimizer::EnsureCodeForDeoptimizationEntry");
+    }
+  }
   CopyBytes(chunk->area_start(), desc.buffer,
       static_cast<size_t>(desc.instr_size));
   CpuFeatures::FlushICache(chunk->area_start(), desc.instr_size);
 
   data->deopt_entry_code_entries_[type] = entry_count;
 }
 
 
 FrameDescription::FrameDescription(uint32_t frame_size,
                                    JSFunction* function)
@@ skipping 840 matching lines @@
       int raw_position = static_cast<int>(info->data());
       last_position = raw_position ? SourcePosition::FromRaw(raw_position)
                                    : SourcePosition::Unknown();
     } else if (info->rmode() == RelocInfo::DEOPT_REASON) {
       last_reason = static_cast<Deoptimizer::DeoptReason>(info->data());
     }
   }
   return DeoptInfo(SourcePosition::Unknown(), NULL, Deoptimizer::kNoReason);
 }
 } }  // namespace v8::internal