Add guard pages in front of platform allocations

src/spaces.cc

Index: src/spaces.cc
===================================================================
--- src/spaces.cc	(revision 8679)
+++ src/spaces.cc	(working copy)
@@ -402,7 +402,9 @@
                                     size_t length,
                                     Executability executable) {
 #ifdef DEBUG
-  ZapBlock(reinterpret_cast<Address>(mem), length);
+  // Do not try to zap the guard page.
+  size_t guardsize = (executable == EXECUTABLE) ? Page::kPageSize : 0;
+  ZapBlock(reinterpret_cast<Address>(mem) + guardsize, length - guardsize);
 #endif
   if (isolate_->code_range()->contains(static_cast<Address>(mem))) {
     isolate_->code_range()->FreeRawMemory(mem, length);
@@ -503,11 +505,21 @@
   if (chunk == NULL) return Page::FromAddress(NULL);
   LOG(isolate_, NewEvent("PagedChunk", chunk, chunk_size));
 
+  size_t guardsize = 0;
+  if (owner->executable() == EXECUTABLE) {
+    guardsize = Page::kPageSize;
+    OS::Guard(chunk, guardsize);
+    chunk_size -= guardsize;
+    chunk = static_cast<Address>(chunk) + guardsize;
+  }
+
   *allocated_pages = PagesInChunk(static_cast<Address>(chunk), chunk_size);
   // We may 'lose' a page due to alignment.
   ASSERT(*allocated_pages >= kPagesPerChunk - 1);

[Mads Ager (chromium), 2011/07/19 09:11:14]

Will this actually work? What if we lose a page due to alignment and lose a page
because of the guard page? We could do all the checking here in exactly the same
way that we did before. Then move the guarding to after the if below and
subtract one page from allocate_pages if EXECUTABLE? That would also remove the
"+/- guardsize" in FreeRawMemory below.

[Cris Neckar, 2011/07/19 18:35:32]

Done.

   if (*allocated_pages == 0) {
-    FreeRawMemory(chunk, chunk_size, owner->executable());
+    FreeRawMemory(static_cast<Address>(chunk) - guardsize,
+                  chunk_size + guardsize,
+                  owner->executable());
     LOG(isolate_, DeleteEvent("PagedChunk", chunk));
     return Page::FromAddress(NULL);
   }
@@ -681,7 +693,8 @@
     LOG(isolate_, DeleteEvent("PagedChunk", c.address()));
     ObjectSpace space = static_cast<ObjectSpace>(1 << c.owner_identity());
     size_t size = c.size();
-    FreeRawMemory(c.address(), size, c.executable());
+    size_t guardsize = (c.executable() == EXECUTABLE) ? Page::kPageSize : 0;
+    FreeRawMemory(c.address() - guardsize, size + guardsize, c.executable());
     PerformAllocationCallback(space, kAllocationActionFree, size);
   }
   c.init(NULL, 0, NULL);
@@ -2672,9 +2685,10 @@
                                         Executability executable) {
   size_t requested = ChunkSizeFor(size_in_bytes);
   size_t size;
+  size_t guardsize = (executable == EXECUTABLE) ? Page::kPageSize : 0;
   Isolate* isolate = Isolate::Current();
   void* mem = isolate->memory_allocator()->AllocateRawMemory(
-      requested, &size, executable);
+      requested + guardsize, &size, executable);
   if (mem == NULL) return NULL;
 
   // The start of the chunk may be overlayed with a page so we have to
@@ -2682,13 +2696,19 @@
   ASSERT((size & Page::kPageFlagMask) == 0);
 
   LOG(isolate, NewEvent("LargeObjectChunk", mem, size));
-  if (size < requested) {
+  if (size < requested + guardsize) {
     isolate->memory_allocator()->FreeRawMemory(
-        mem, size, executable);
+        static_cast<Address>(mem) - guardsize, size + guardsize, executable);

[Mads Ager (chromium), 2011/07/19 09:11:14]

This looks wrong. You should use mem and size directly as you got them from
AllocateRawMemory.

[Cris Neckar, 2011/07/19 18:35:32]

Yep I am dumb. :)

     LOG(isolate, DeleteEvent("LargeObjectChunk", mem));
     return NULL;
   }
 
+  if (guardsize != 0) {
+    OS::Guard(mem, guardsize);
+    size -= guardsize;
+    mem = static_cast<Address>(mem) + guardsize;
+  }
+
   ObjectSpace space = (executable == EXECUTABLE)
       ? kObjectSpaceCodeSpace
       : kObjectSpaceLoSpace;
@@ -2742,9 +2762,11 @@
     ObjectSpace space = kObjectSpaceLoSpace;
     if (executable == EXECUTABLE) space = kObjectSpaceCodeSpace;
     size_t size = chunk->size();
-    heap()->isolate()->memory_allocator()->FreeRawMemory(chunk->address(),
-                                                         size,
-                                                         executable);
+    size_t guardsize = (executable == EXECUTABLE) ? Page::kPageSize : 0;
+    heap()->isolate()->memory_allocator()->FreeRawMemory(
+        chunk->address() - guardsize,
+        size + guardsize,
+        executable);
     heap()->isolate()->memory_allocator()->PerformAllocationCallback(
         space, kAllocationActionFree, size);
   }
@@ -2941,10 +2963,15 @@
       objects_size_ -= object->Size();
       page_count_--;
       ObjectSpace space = kObjectSpaceLoSpace;
-      if (executable == EXECUTABLE) space = kObjectSpaceCodeSpace;
-      heap()->isolate()->memory_allocator()->FreeRawMemory(chunk_address,
-                                                           chunk_size,
-                                                           executable);
+      size_t guardsize = 0;
+      if (executable == EXECUTABLE) {
+        space = kObjectSpaceCodeSpace;
+        guardsize = Page::kPageSize;
+      }
+      heap()->isolate()->memory_allocator()->FreeRawMemory(
+          chunk_address - guardsize,
+          chunk_size + guardsize,
+          executable);
       heap()->isolate()->memory_allocator()->PerformAllocationCallback(
           space, kAllocationActionFree, size_);
       LOG(heap()->isolate(), DeleteEvent("LargeObjectChunk", chunk_address));