Out of bounds memory access in TestJSArrayForAllocationSiteInfo.

The function intended to check the map pointer of an AllocationSiteInfo object, but neglected to
subtract an offset to do so.

BUG=169928

Review URL: https://codereview.chromium.org/11931037

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13444 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc
index ffa25eb..7f641a7 100644 (file)
--- a/src/arm/macro-assembler-arm.cc
+++ b/src/arm/macro-assembler-arm.cc
@@ -3889,12 +3889,12 @@ void MacroAssembler::TestJSArrayForAllocationSiteInfo(
   ExternalReference new_space_allocation_top =
       ExternalReference::new_space_allocation_top_address(isolate());
   ldr(scratch_reg, FieldMemOperand(receiver_reg,
-                                   JSArray::kSize + AllocationSiteInfo::kSize));
+      JSArray::kSize + AllocationSiteInfo::kSize - kHeapObjectTag));
   cmp(scratch_reg, Operand(new_space_start));
   b(lt, &no_info_available);
   cmp(scratch_reg, Operand(new_space_allocation_top));
-  b(hs, &no_info_available);
-  ldr(scratch_reg, MemOperand(scratch_reg, 0));
+  b(gt, &no_info_available);
+  ldr(scratch_reg, MemOperand(scratch_reg, -AllocationSiteInfo::kSize));
   cmp(scratch_reg,
       Operand(Handle<Map>(isolate()->heap()->allocation_site_info_map())));
   b(eq, allocation_info_present);

diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc
index cebfb13..db0055a 100644 (file)
--- a/src/ia32/macro-assembler-ia32.cc
+++ b/src/ia32/macro-assembler-ia32.cc
@@ -3055,18 +3055,19 @@ void MacroAssembler::TestJSArrayForAllocationSiteInfo(
     Register scratch_reg,
     Label* allocation_info_present) {
   Label no_info_available;
+
   ExternalReference new_space_start =
       ExternalReference::new_space_start(isolate());
   ExternalReference new_space_allocation_top =
       ExternalReference::new_space_allocation_top_address(isolate());
 
   lea(scratch_reg, Operand(receiver_reg,
-                           JSArray::kSize + AllocationSiteInfo::kSize));
+      JSArray::kSize + AllocationSiteInfo::kSize - kHeapObjectTag));
   cmp(scratch_reg, Immediate(new_space_start));
   j(less, &no_info_available);
   cmp(scratch_reg, Operand::StaticVariable(new_space_allocation_top));
-  j(greater_equal, &no_info_available);
-  cmp(MemOperand(scratch_reg, 0),
+  j(greater, &no_info_available);
+  cmp(MemOperand(scratch_reg, -AllocationSiteInfo::kSize),
       Immediate(Handle<Map>(isolate()->heap()->allocation_site_info_map())));
   j(equal, allocation_info_present);
   bind(&no_info_available);

diff --git a/src/mips/macro-assembler-mips.cc b/src/mips/macro-assembler-mips.cc
index 64f3631..333d630 100644 (file)
--- a/src/mips/macro-assembler-mips.cc
+++ b/src/mips/macro-assembler-mips.cc
@@ -5455,11 +5455,11 @@ void MacroAssembler::TestJSArrayForAllocationSiteInfo(
   ExternalReference new_space_allocation_top =
       ExternalReference::new_space_allocation_top_address(isolate());
   lw(scratch_reg, FieldMemOperand(receiver_reg,
-                                  JSArray::kSize + AllocationSiteInfo::kSize));
+      JSArray::kSize + AllocationSiteInfo::kSize - kHeapObjecTag));
   Branch(&no_info_available, lt, scratch_reg, Operand(new_space_start));
-  Branch(&no_info_available, hs, scratch_reg,
+  Branch(&no_info_available, gt, scratch_reg,
       Operand(new_space_allocation_top));
-  lw(scratch_reg, MemOperand(scratch_reg));
+  lw(scratch_reg, MemOperand(scratch_reg, -AllocationSiteInfo::kSize));
   Branch(allocation_info_present, eq, scratch_reg,
       Operand(Handle<Map>(isolate()->heap()->allocation_site_info_map())));
   bind(&no_info_available);

diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc
index 52699e7..b0f3f8d 100644 (file)
--- a/src/x64/macro-assembler-x64.cc
+++ b/src/x64/macro-assembler-x64.cc
@@ -4615,13 +4615,13 @@ void MacroAssembler::TestJSArrayForAllocationSiteInfo(
       ExternalReference::new_space_allocation_top_address(isolate());
 
   lea(scratch_reg, Operand(receiver_reg,
-                           JSArray::kSize + AllocationSiteInfo::kSize));
+      JSArray::kSize + AllocationSiteInfo::kSize - kHeapObjectTag));
   movq(kScratchRegister, new_space_start);
   cmpq(scratch_reg, kScratchRegister);
   j(less, &no_info_available);
   cmpq(scratch_reg, ExternalOperand(new_space_allocation_top));
-  j(greater_equal, &no_info_available);
-  CompareRoot(MemOperand(scratch_reg, 0),
+  j(greater, &no_info_available);
+  CompareRoot(MemOperand(scratch_reg, -AllocationSiteInfo::kSize),
               Heap::kAllocationSiteInfoMapRootIndex);
   j(equal, allocation_info_present);
   bind(&no_info_available);

diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 634deef..eb4f072 100644 (file)
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -2694,3 +2694,87 @@ 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
+  HEAP->CollectGarbage(NEW_SPACE);
+
+  // 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 +
+                       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();
+}

diff --git a/test/mjsunit/allocation-site-info.js b/test/mjsunit/allocation-site-info.js
index f4263af..aa74272 100644 (file)
--- a/test/mjsunit/allocation-site-info.js
+++ b/test/mjsunit/allocation-site-info.js
@@ -75,22 +75,34 @@ function assertKind(expected, obj, name_opt) {
 }
 
 if (support_smi_only_arrays) {
-    function fastliteralcase(value) {
-        var literal = [1, 2, 3];
+    function fastliteralcase(literal, value) {
+        // var literal = [1, 2, 3];
         literal[0] = value;
         return literal;
     }
 
+    function get_standard_literal() {
+        var literal = [1, 2, 3];
+        return literal;
+    }
+
     // Case: [1,2,3] as allocation site
-    obj = fastliteralcase(1);
+    obj = fastliteralcase(get_standard_literal(), 1);
     assertKind(elements_kind.fast_smi_only, obj);
-    obj = fastliteralcase(1.5);
+    obj = fastliteralcase(get_standard_literal(), 1.5);
+    assertKind(elements_kind.fast_double, obj);
+    obj = fastliteralcase(get_standard_literal(), 2);
+    assertKind(elements_kind.fast_double, obj);
+
+    obj = fastliteralcase([5, 3, 2], 1.5);
+    assertKind(elements_kind.fast_double, obj);
+    obj = fastliteralcase([3, 6, 2], 1.5);
     assertKind(elements_kind.fast_double, obj);
-    obj = fastliteralcase(2);
+    obj = fastliteralcase([2, 6, 3], 2);
     assertKind(elements_kind.fast_double, obj);
 
     // Verify that we will not pretransition the double->fast path.
-    obj = fastliteralcase("elliot");
+    obj = fastliteralcase(get_standard_literal(), "elliot");
     assertKind(elements_kind.fast, obj);
 
     // This fails until we turn off optimistic transitions to the