Fix assert in Page::Initialize.

Fix assert in Page::Initialize.

R=mstarzinger@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=13896

[ulan, 2012-11-09 13:58:22]

Please take a look. This assertion hits in long running Google Drive extension
in 64-bit Chrome.

[ulan, 2012-11-09 14:00:20]

For the record, patch that reproduces assertion hit (not adding it to the CL):

diff --git a/src/spaces.h b/src/spaces.h
index 9121e9c..be49a8a 100644
--- a/src/spaces.h
+++ b/src/spaces.h
@@ -852,6 +852,11 @@ class CodeRange {
     return start <= address && address < start + code_range_->size();
   }
 
+  size_t current_allocation_block_size() {
+    if (allocation_list_.length() == 0) return 0;
+    return allocation_list_[current_allocation_block_index_].size;
+  }
+
   // Allocates a chunk of memory from the large-object portion of
   // the code range.  On platforms with no separate code range, should
   // not be called.
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 811973b..abbce6b 100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -2441,3 +2441,53 @@ TEST(Regression144230) {
   USE(global->SetProperty(*name, *call_function, NONE, kNonStrictMode));
   CompileRun("call();");
 }
+
+
+ static Handle<Code> CreateCode(int size) {
+  AlwaysAllocateScope always_allocate;
+#define __ assm.
+  Assembler assm(Isolate::Current(), NULL, 0);
+  for (int i = 0; i < size; i++) {
+    __ nop();
+  }
+  CodeDesc desc;
+  assm.GetCode(&desc);
+  MaybeObject* maybe_code = HEAP->CreateCode(
+      desc,
+      Code::ComputeFlags(Code::STUB),
+      Handle<Object>());
+  Object* code = NULL;
+  bool success = maybe_code->ToObject(&code);
+  CHECK(success);
+  CHECK(code->IsCode());
+  return Handle<Code>(Code::cast(code));
+}
+
+
+TEST(LargeExecutableMemoryChunk) {
+  InitializeVM();
+  v8::HandleScope scope;
+  CodeRange* code_range = ISOLATE->code_range();
+  if (!code_range->exists() ||
+      code_range->current_allocation_block_size() < 4 * Page::kPageSize) {
+    return;
+  }
+  const int kMaxCodeSize = MemoryAllocator::CodePageAreaSize() -
+                           Code::kHeaderSize;
+  Address buffer;
+  size_t requested, allocated;
+  // Allocate and release a memory chunk to make sure that code range
+  // free list is not empty. Otherwise, we will get OOM failure later on.
+  requested = kMaxCodeSize;
+  buffer = code_range->AllocateRawMemory(requested, &allocated);
+  code_range->FreeRawMemory(buffer, allocated);
+  // Leave only 3 pages in the code range.
+  requested = code_range->current_allocation_block_size() - 3 *
Page::kPageSize;
+  buffer = code_range->AllocateRawMemory(requested, &allocated);
+  {
+    v8::HandleScope inner_scope;
+    CreateCode(kMaxCodeSize);
+    CreateCode(kMaxCodeSize); // This will allocate a chunk of size > page
size.
+  }
+  code_range->FreeRawMemory(buffer, allocated);
+}

[Michael Starzinger, 2012-11-14 14:43:54]

LGTM with two comments.

https://codereview.chromium.org/11275229/diff/1/src/spaces-inl.h
File src/spaces-inl.h (right):

https://codereview.chromium.org/11275229/diff/1/src/spaces-inl.h#newcode167
src/spaces-inl.h:167: ASSERT(page->area_size() <= kPageSize);
This should actually be ASSERT(page->area_size() <= kNonCodeObjectAreaSize);

https://codereview.chromium.org/11275229/diff/1/src/spaces-inl.h#newcode169
src/spaces-inl.h:169: ASSERT(chunk->size() <= static_cast<size_t>(kPageSize) ||
I am not sure if we should even have this second assertion anymore. If nobody
beside the MemoryAllocator should rely on chunk->size() being a special size, we
should not assert it here. So I think we should completely remove the second
assert, but could be convinced otherwise.

[ulan, 2013-03-11 11:01:05]

Committed patchset #1 manually as r13896 (presubmit successful).