From 4e405b694546fa0423aabd1f2accda3ffb33e7d7 Mon Sep 17 00:00:00 2001 From: "jkummerow@chromium.org" Date: Sun, 25 Mar 2012 15:16:06 +0000 Subject: [PATCH] Fix missing write barrier in CopyObjectToObjectElements. Passing the write barrier mode as a parameter does not make sense, as the elements kind specific copiers know best whether a write barrier is needed or not. BUG=119926 TEST=mjsunit/regress/regress-crbug-119926 R=danno@chromium.org Review URL: https://chromiumcodereview.appspot.com/9808111 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11134 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/elements.cc | 33 +++++++------------- src/elements.h | 7 ++--- src/objects.cc | 5 ++- test/mjsunit/regress/regress-crbug-119926.js | 33 ++++++++++++++++++++ 4 files changed, 49 insertions(+), 29 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-119926.js diff --git a/src/elements.cc b/src/elements.cc index ba77de77ce..1d043a153e 100644 --- a/src/elements.cc +++ b/src/elements.cc @@ -137,8 +137,7 @@ void CopyObjectToObjectElements(FixedArray* from, FixedArray* to, ElementsKind to_kind, uint32_t to_start, - int raw_copy_size, - WriteBarrierMode mode) { + int raw_copy_size) { ASSERT(to->map() != HEAP->fixed_cow_array_map()); ASSERT(from_kind == FAST_ELEMENTS || from_kind == FAST_SMI_ONLY_ELEMENTS); ASSERT(to_kind == FAST_ELEMENTS || to_kind == FAST_SMI_ONLY_ELEMENTS); @@ -166,8 +165,7 @@ void CopyObjectToObjectElements(FixedArray* from, CopyWords(reinterpret_cast(to_address) + to_start, reinterpret_cast(from_address) + from_start, copy_size); - if (from_kind == FAST_ELEMENTS && to_kind == FAST_ELEMENTS && - mode == UPDATE_WRITE_BARRIER) { + if (from_kind == FAST_ELEMENTS && to_kind == FAST_ELEMENTS) { Heap* heap = from->GetHeap(); if (!heap->InNewSpace(to)) { heap->RecordWrites(to->address(), @@ -184,8 +182,7 @@ static void CopyDictionaryToObjectElements(SeededNumberDictionary* from, FixedArray* to, ElementsKind to_kind, uint32_t to_start, - int raw_copy_size, - WriteBarrierMode mode) { + int raw_copy_size) { int copy_size = raw_copy_size; Heap* heap = from->GetHeap(); if (raw_copy_size < 0) { @@ -476,8 +473,7 @@ class ElementsAccessorBase : public ElementsAccessor { FixedArrayBase* to, ElementsKind to_kind, uint32_t to_start, - int copy_size, - WriteBarrierMode mode) { + int copy_size) { UNREACHABLE(); return NULL; } @@ -488,7 +484,6 @@ class ElementsAccessorBase : public ElementsAccessor { ElementsKind to_kind, uint32_t to_start, int copy_size, - WriteBarrierMode mode, FixedArrayBase* from) { if (from == NULL) { from = from_holder->elements(); @@ -497,7 +492,7 @@ class ElementsAccessorBase : public ElementsAccessor { return from; } return ElementsAccessorSubclass::CopyElementsImpl( - from, from_start, to, to_kind, to_start, copy_size, mode); + from, from_start, to, to_kind, to_start, copy_size); } virtual MaybeObject* AddElementsToFixedArray(Object* receiver, @@ -734,14 +729,13 @@ class FastObjectElementsAccessor FixedArrayBase* to, ElementsKind to_kind, uint32_t to_start, - int copy_size, - WriteBarrierMode mode) { + int copy_size) { switch (to_kind) { case FAST_SMI_ONLY_ELEMENTS: case FAST_ELEMENTS: { CopyObjectToObjectElements( FixedArray::cast(from), ElementsTraits::Kind, from_start, - FixedArray::cast(to), to_kind, to_start, copy_size, mode); + FixedArray::cast(to), to_kind, to_start, copy_size); return from; } case FAST_DOUBLE_ELEMENTS: @@ -809,8 +803,7 @@ class FastDoubleElementsAccessor FixedArrayBase* to, ElementsKind to_kind, uint32_t to_start, - int copy_size, - WriteBarrierMode mode) { + int copy_size) { switch (to_kind) { case FAST_SMI_ONLY_ELEMENTS: case FAST_ELEMENTS: @@ -1108,14 +1101,13 @@ class DictionaryElementsAccessor FixedArrayBase* to, ElementsKind to_kind, uint32_t to_start, - int copy_size, - WriteBarrierMode mode) { + int copy_size) { switch (to_kind) { case FAST_SMI_ONLY_ELEMENTS: case FAST_ELEMENTS: CopyDictionaryToObjectElements( SeededNumberDictionary::cast(from), from_start, - FixedArray::cast(to), to_kind, to_start, copy_size, mode); + FixedArray::cast(to), to_kind, to_start, copy_size); return from; case FAST_DOUBLE_ELEMENTS: CopyDictionaryToDoubleElements( @@ -1253,13 +1245,12 @@ class NonStrictArgumentsElementsAccessor : public ElementsAccessorBase< FixedArrayBase* to, ElementsKind to_kind, uint32_t to_start, - int copy_size, - WriteBarrierMode mode) { + int copy_size) { FixedArray* parameter_map = FixedArray::cast(from); FixedArray* arguments = FixedArray::cast(parameter_map->get(1)); ElementsAccessor* accessor = ElementsAccessor::ForArray(arguments); return accessor->CopyElements(NULL, from_start, to, to_kind, - to_start, copy_size, mode, arguments); + to_start, copy_size, arguments); } static uint32_t GetCapacityImpl(FixedArray* parameter_map) { diff --git a/src/elements.h b/src/elements.h index e853a8808e..ff97c08324 100644 --- a/src/elements.h +++ b/src/elements.h @@ -107,16 +107,14 @@ class ElementsAccessor { ElementsKind destination_kind, uint32_t destination_start, int copy_size, - WriteBarrierMode mode, FixedArrayBase* source = NULL) = 0; MaybeObject* CopyElements(JSObject* from_holder, FixedArrayBase* to, ElementsKind to_kind, - WriteBarrierMode mode, FixedArrayBase* from = NULL) { return CopyElements(from_holder, 0, to, to_kind, 0, - kCopyToEndAndInitializeToHole, mode, from); + kCopyToEndAndInitializeToHole, from); } virtual MaybeObject* AddElementsToFixedArray(Object* receiver, @@ -164,8 +162,7 @@ void CopyObjectToObjectElements(FixedArray* from_obj, FixedArray* to_obj, ElementsKind to_kind, uint32_t to_start, - int copy_size, - WriteBarrierMode mode = UPDATE_WRITE_BARRIER); + int copy_size); } } // namespace v8::internal diff --git a/src/objects.cc b/src/objects.cc index fa4f943dca..64d85a0685 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -8454,7 +8454,7 @@ MaybeObject* JSObject::SetFastElementsCapacityAndLength( ? FAST_SMI_ONLY_ELEMENTS : FAST_ELEMENTS; // int copy_size = Min(old_elements_raw->length(), new_elements->length()); - accessor->CopyElements(this, new_elements, to_kind, SKIP_WRITE_BARRIER); + accessor->CopyElements(this, new_elements, to_kind); if (elements_kind != NON_STRICT_ARGUMENTS_ELEMENTS) { set_map_and_elements(new_map, new_elements); } else { @@ -8498,8 +8498,7 @@ MaybeObject* JSObject::SetFastDoubleElementsCapacityAndLength( FixedArrayBase* old_elements = elements(); ElementsKind elements_kind = GetElementsKind(); ElementsAccessor* accessor = ElementsAccessor::ForKind(elements_kind); - accessor->CopyElements(this, elems, FAST_DOUBLE_ELEMENTS, - SKIP_WRITE_BARRIER); + accessor->CopyElements(this, elems, FAST_DOUBLE_ELEMENTS); if (elements_kind != NON_STRICT_ARGUMENTS_ELEMENTS) { set_map_and_elements(new_map, elems); } else { diff --git a/test/mjsunit/regress/regress-crbug-119926.js b/test/mjsunit/regress/regress-crbug-119926.js new file mode 100644 index 0000000000..26b84fad7f --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-119926.js @@ -0,0 +1,33 @@ +// Copyright 2012 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Test that array elements don't break upon garbage collection. + +var a = new Array(500); +for (var i = 0; i < 500000; i++) { + a[i] = new Object(); +}