Out of bounds memory access in TestJSArrayForAllocationSiteInfo.

test/cctest/test-heap.cc

Index: test/cctest/test-heap.cc
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 634deefbfb2190e5d94c31d08ac9ace20eef6ef5..243a2754246c8c2b15b3c7aa119d2870b9ddbd3a 100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -2694,3 +2694,93 @@ TEST(Regress169209) {
   HEAP->CollectAllGarbage(Heap::kNoGCFlags);
   CHECK(shared1->code()->gc_metadata() == NULL);
 }
+
+
+// Helper function that simulates a fill new-space in the heap.
+static inline void AllocateAllButNBytes(v8::internal::NewSpace* space,
+                                        int extra_bytes) {
+  int space_remaining = static_cast<int>(
+      *space->allocation_limit_address() - *space->allocation_top_address());
+  CHECK(space_remaining >= extra_bytes);
+  int new_linear_size = space_remaining - extra_bytes;
+  v8::internal::MaybeObject* maybe = space->AllocateRaw(new_linear_size);
+  v8::internal::FreeListNode* node = v8::internal::FreeListNode::cast(maybe);
+  node->set_size(space->heap(), new_linear_size);
+}
+
+
+TEST(Regress169928) {
+  i::FLAG_allow_natives_syntax = true;
+  i::FLAG_crankshaft = false;
+  InitializeVM();
+  v8::HandleScope scope;
+
+  // Some flags turn Scavenge collections into Mark-sweep collections
+  // and hence are incompatible with this test case.
+  if (FLAG_gc_global || FLAG_stress_compaction) return;
+
+  // Prepare the environment
+  CompileRun("function fastliteralcase(literal, value) {"
+             "    literal[0] = value;"
+             "    return literal;"
+             "}"
+             "function get_standard_literal() {"
+             "    var literal = [1, 2, 3];"
+             "    return literal;"
+             "}"
+             "obj = fastliteralcase(get_standard_literal(), 1);"
+             "obj = fastliteralcase(get_standard_literal(), 1.5);"
+             "obj = fastliteralcase(get_standard_literal(), 2);");
+
+  // prepare the heap
+  v8::Local<v8::String> mote_code_string =
+      v8_str("fastliteralcase(mote, 2.5);");
+
+  v8::Local<v8::String> array_name = v8_str("mote");
+  v8::Context::GetCurrent()->Global()->Set(array_name, v8::Int32::New(0));
+
+  // First make sure we flip spaces
+#ifdef DEBUG
+  Address* limit_addr = HEAP->new_space()->allocation_limit_address();
+  Address limit = *limit_addr;
+#endif
+  HEAP->CollectGarbage(NEW_SPACE);
+  // Paranoid...make sure we really flipped spaces.
+  ASSERT(limit != *limit_addr);

[Michael Starzinger, 2013/01/21 09:54:26]

I think we can drop the assertion whether the limit changed after a GC.

[mvstanton, 2013/01/21 12:25:40]

Done.

+
+  // Allocate the object.
+  Handle<FixedArray> array_data = FACTORY->NewFixedArray(2, NOT_TENURED);
+  array_data->set(0, Smi::FromInt(1));
+  array_data->set(1, Smi::FromInt(2));
+
+  AllocateAllButNBytes(HEAP->new_space(),
+                       JSArray::kSize + AllocationSiteInfo::kSize +

[Michael Starzinger, 2013/01/21 09:54:26]

Wouldn't it be better to just allocate the array completely at the end of
new-space without any allocation-site info and trailing pointer? This should
still trigger the crash and on top of that make sure that the stub doesn't read
a single pointer out of bounds. Also it would make the test case much simpler.

[mvstanton, 2013/01/21 12:25:40]

Actually in this case we do need it. The crash will only happen if we have
<ARRAY> <AllocationSiteInfo> <kPointerSize filler>. There is a guard check in
place, and without the extra bytes after the array, we won't enter the failure
case.

+                       kPointerSize);
+
+  Handle<JSArray> array = FACTORY->NewJSArrayWithElements(array_data,
+                                                          FAST_SMI_ELEMENTS,
+                                                          NOT_TENURED);
+
+  CHECK_EQ(Smi::FromInt(2), array->length());
+  CHECK(array->HasFastSmiOrObjectElements());
+
+  // We need filler the size of AllocationSiteInfo object, plus an extra
+  // fill pointer value.
+  MaybeObject* maybe_object = HEAP->AllocateRaw(
+      AllocationSiteInfo::kSize + kPointerSize, NEW_SPACE, OLD_POINTER_SPACE);
+  Object* obj = NULL;
+  CHECK(maybe_object->ToObject(&obj));
+  Address addr_obj = reinterpret_cast<Address>(
+      reinterpret_cast<byte*>(obj - kHeapObjectTag));
+  HEAP->CreateFillerObjectAt(addr_obj,
+                             AllocationSiteInfo::kSize + kPointerSize);
+
+  // Give the array a name, making sure not to allocate strings.
+  v8::Handle<v8::Object> array_obj = v8::Utils::ToLocal(array);
+  v8::Context::GetCurrent()->Global()->Set(array_name, array_obj);
+
+  // This should crash with a protection violation if we are running a build
+  // with the bug.
+  AlwaysAllocateScope aa_scope;
+  v8::Script::Compile(mote_code_string)->Run();
+}