From d7d917e6f0848941447e9be7fa5317e6501a3c82 Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Mon, 21 Jan 2013 12:26:29 +0000 Subject: [PATCH] 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 --- src/arm/macro-assembler-arm.cc | 6 +-- src/ia32/macro-assembler-ia32.cc | 7 +-- src/mips/macro-assembler-mips.cc | 6 +-- src/x64/macro-assembler-x64.cc | 6 +-- test/cctest/test-heap.cc | 84 ++++++++++++++++++++++++++++++++++++ test/mjsunit/allocation-site-info.js | 24 ++++++++--- 6 files changed, 115 insertions(+), 18 deletions(-) diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc index ffa25eb..7f641a7 100644 --- 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(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 --- 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(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 --- 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(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 --- 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 --- 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( + *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 mote_code_string = + v8_str("fastliteralcase(mote, 2.5);"); + + v8::Local 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 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 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
( + reinterpret_cast(obj - kHeapObjectTag)); + HEAP->CreateFillerObjectAt(addr_obj, + AllocationSiteInfo::kSize + kPointerSize); + + // Give the array a name, making sure not to allocate strings. + v8::Handle 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 --- 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 -- 2.7.4