Use CopyElements for SetFastDoubleElementsCapacityAndLength

src/builtins.cc

Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index ca202f2dc405e5d7331a5484f130a2edb3c28ae3..3fc47a78af4844c1a65050343efa96a0b05ba214 100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -508,8 +508,7 @@ BUILTIN(ArrayPush) {
     }
     FixedArray* new_elms = FixedArray::cast(obj);
 
-    AssertNoAllocation no_gc;

[Jakob Kummerow, 2012/03/12 15:33:28]

Did you delete this line on purpose? AFAICS we don't allocate anything during
the call, so we might want to keep the assertion. 

[danno, 2012/03/12 20:35:03]

The AssertNoAllocation doesn't help here, and actually it's misleading (the code
here is wrong as it was). There can't be an allocation between the time that
new_elms is allocated and the copy happens. But the AssertNoAllocation object
doesn't guarantee that, it only guarantees that there is no allocation since the
no allocation object is created. I talked about this with Starzinger, and we
agreed that it's misleading to suggest that there is some sort of protection
through stack-allocated protection objects when there isn't. In other new code,
the WriteBarrierMode is explicit. Until there is a replacement for
AssertNoAllocation that actually is bound tighter to the allocation that it's
trying to protect (that's a bigger chunk of work), the explicit mode at least
forces the caller to think about what is safe and what is not. 

On 2012/03/12 15:33:28, Jakob wrote:

-    CopyObjectToObjectElements(&no_gc, elms, FAST_ELEMENTS, 0,
+    CopyObjectToObjectElements(elms, FAST_ELEMENTS, 0,
                                new_elms, FAST_ELEMENTS, 0, len);
     FillWithHoles(heap, new_elms, new_length, capacity);
 
@@ -646,7 +645,7 @@ BUILTIN(ArrayUnshift) {
     }
     FixedArray* new_elms = FixedArray::cast(obj);
     AssertNoAllocation no_gc;
-    CopyObjectToObjectElements(&no_gc, elms, FAST_ELEMENTS, 0,
+    CopyObjectToObjectElements(elms, FAST_ELEMENTS, 0,
                                new_elms, FAST_ELEMENTS, to_add, len);
     FillWithHoles(heap, new_elms, new_length, capacity);
     elms = new_elms;
@@ -758,7 +757,7 @@ BUILTIN(ArraySlice) {
   if (!maybe_array->To(&result_array)) return maybe_array;
 
   AssertNoAllocation no_gc;
-  CopyObjectToObjectElements(&no_gc, elms, FAST_ELEMENTS, k,
+  CopyObjectToObjectElements(elms, FAST_ELEMENTS, k,
                              FixedArray::cast(result_array->elements()),
                              FAST_ELEMENTS, 0, result_len);
 
@@ -833,7 +832,7 @@ BUILTIN(ArraySplice) {
   {
     AssertNoAllocation no_gc;
     // Fill newly created array.
-    CopyObjectToObjectElements(&no_gc, elms, FAST_ELEMENTS, actual_start,
+    CopyObjectToObjectElements(elms, FAST_ELEMENTS, actual_start,
                                FixedArray::cast(result_array->elements()),
                                FAST_ELEMENTS, 0, actual_delete_count);
   }
@@ -885,10 +884,10 @@ BUILTIN(ArraySplice) {
       {
         AssertNoAllocation no_gc;
         // Copy the part before actual_start as is.
-        CopyObjectToObjectElements(&no_gc, elms, FAST_ELEMENTS, 0,
+        CopyObjectToObjectElements(elms, FAST_ELEMENTS, 0,
                                    new_elms, FAST_ELEMENTS, 0, actual_start);
         const int to_copy = len - actual_delete_count - actual_start;
-        CopyObjectToObjectElements(&no_gc, elms, FAST_ELEMENTS,
+        CopyObjectToObjectElements(elms, FAST_ELEMENTS,
                                    actual_start + actual_delete_count,
                                    new_elms, FAST_ELEMENTS,
                                    actual_start + item_count, to_copy);
@@ -973,14 +972,13 @@ BUILTIN(ArrayConcat) {
   if (result_len == 0) return result_array;
 
   // Copy data.
-  AssertNoAllocation no_gc;

[Jakob Kummerow, 2012/03/12 15:33:28]

Same question here.

[danno, 2012/03/12 20:35:03]

See above.
On 2012/03/12 15:33:28, Jakob wrote:

   int start_pos = 0;
   FixedArray* result_elms(FixedArray::cast(result_array->elements()));
   for (int i = 0; i < n_arguments; i++) {
     JSArray* array = JSArray::cast(args[i]);
     int len = Smi::cast(array->length())->value();
     FixedArray* elms = FixedArray::cast(array->elements());
-    CopyObjectToObjectElements(&no_gc, elms, FAST_ELEMENTS, 0,
+    CopyObjectToObjectElements(elms, FAST_ELEMENTS, 0,
                                result_elms, FAST_ELEMENTS,
                                start_pos, len);
     start_pos += len;