LGTM with a promised test. yours, anton.
On Thu, Sep 23, 2010 at 4:03 PM, <[email protected]> wrote: > Reviewers: antonm, > > Description: > Fix copy-on-write assert by setting the new array map early. > > BUG=876 > > Please review this at http://codereview.chromium.org/3466013/show > > Affected files: > M src/heap-inl.h > M src/heap.h > M src/heap.cc > M src/objects-inl.h > > > Index: src/heap-inl.h > diff --git a/src/heap-inl.h b/src/heap-inl.h > index > 8f7dd3bab8206275800e747567d538f2678a3305..b68f5c1ca43b76a2875cc5d967e6ce796859dd23 > 100644 > --- a/src/heap-inl.h > +++ b/src/heap-inl.h > @@ -59,6 +59,11 @@ Object* Heap::AllocateSymbol(Vector<const char> str, > } > > > +Object* Heap::CopyFixedArray(FixedArray* src) { > + return CopyFixedArrayWithMap(src, src->map()); > +} > + > + > Object* Heap::AllocateRaw(int size_in_bytes, > AllocationSpace space, > AllocationSpace retry_space) { > Index: src/heap.cc > diff --git a/src/heap.cc b/src/heap.cc > index > 371e278641e8a1a3a7fdd9dcf57b4be2c67a2cc1..25d199b189d5e881797fbf829495b4c5790c017d > 100644 > --- a/src/heap.cc > +++ b/src/heap.cc > @@ -3205,16 +3205,19 @@ Object* Heap::AllocateRawFixedArray(int length) { > } > > > -Object* Heap::CopyFixedArray(FixedArray* src) { > +Object* Heap::CopyFixedArrayWithMap(FixedArray* src, Map* map) { > int len = src->length(); > Object* obj = AllocateRawFixedArray(len); > if (obj->IsFailure()) return obj; > if (Heap::InNewSpace(obj)) { > HeapObject* dst = HeapObject::cast(obj); > - CopyBlock(dst->address(), src->address(), FixedArray::SizeFor(len)); > + dst->set_map(map); > + CopyBlock(dst->address() + kPointerSize, > + src->address() + kPointerSize, > + FixedArray::SizeFor(len) - kPointerSize); > return obj; > } > - HeapObject::cast(obj)->set_map(src->map()); > + HeapObject::cast(obj)->set_map(map); > FixedArray* result = FixedArray::cast(obj); > result->set_length(len); > > Index: src/heap.h > diff --git a/src/heap.h b/src/heap.h > index > cfb3b6a0300d10f9da474a869ab9e7edef82c553..8a11530d8ce086ed579a0189656f680e9b4d4071 > 100644 > --- a/src/heap.h > +++ b/src/heap.h > @@ -498,7 +498,12 @@ class Heap : public AllStatic { > > // Make a copy of src and return it. Returns > // Failure::RetryAfterGC(requested_bytes, space) if the allocation failed. > - MUST_USE_RESULT static Object* CopyFixedArray(FixedArray* src); > + MUST_USE_RESULT static inline Object* CopyFixedArray(FixedArray* src); > + > + // Make a copy of src, set the map, and return the copy. Returns > + // Failure::RetryAfterGC(requested_bytes, space) if the allocation > failed. > + MUST_USE_RESULT static Object* CopyFixedArrayWithMap(FixedArray* src, > + Map* map); > > // Allocates a fixed array initialized with the hole values. > // Returns Failure::RetryAfterGC(requested_bytes, space) if the allocation > Index: src/objects-inl.h > diff --git a/src/objects-inl.h b/src/objects-inl.h > index > b7bd4c6f1ce8c30260fce3ae160eb3b00a7e38a9..93f752f150cb2f0cfe2a3c67f5d1369408e139c2 > 100644 > --- a/src/objects-inl.h > +++ b/src/objects-inl.h > @@ -3187,9 +3187,9 @@ Object* JSObject::EnsureWritableFastElements() { > ASSERT(HasFastElements()); > FixedArray* elems = FixedArray::cast(elements()); > if (elems->map() != Heap::fixed_cow_array_map()) return elems; > - Object* writable_elems = Heap::CopyFixedArray(elems); > + Object* writable_elems = Heap::CopyFixedArrayWithMap(elems, > + > Heap::fixed_array_map()); > if (writable_elems->IsFailure()) return writable_elems; > - FixedArray::cast(writable_elems)->set_map(Heap::fixed_array_map()); > set_elements(FixedArray::cast(writable_elems)); > Counters::cow_arrays_converted.Increment(); > return writable_elems; > > > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
