Revision: 20005
Author:   [email protected]
Date:     Mon Mar 17 15:43:33 2014 UTC
Log:      Fixed spec violation of storing to length of a frozen object.

BUG=chromium:350890
LOG=N
[email protected]

Review URL: https://codereview.chromium.org/196653015
http://code.google.com/p/v8/source/detail?r=20005

Added:
 /branches/bleeding_edge/test/mjsunit/regress/regress-crbug-350890.js
Modified:
 /branches/bleeding_edge/src/a64/code-stubs-a64.cc
 /branches/bleeding_edge/src/a64/stub-cache-a64.cc
 /branches/bleeding_edge/src/arm/code-stubs-arm.cc
 /branches/bleeding_edge/src/arm/stub-cache-arm.cc
 /branches/bleeding_edge/src/code-stubs.h
 /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc
 /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc
 /branches/bleeding_edge/src/ic.cc
 /branches/bleeding_edge/src/stub-cache.cc
 /branches/bleeding_edge/src/stub-cache.h
 /branches/bleeding_edge/src/x64/code-stubs-x64.cc
 /branches/bleeding_edge/src/x64/stub-cache-x64.cc

=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-crbug-350890.js Mon Mar 17 15:43:33 2014 UTC
@@ -0,0 +1,42 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+function set_length(a, l) {
+  a.length = l;
+}
+
+function test1() {
+  var l = {};
+  var a = Array(l);
+  set_length(a, 3);
+  set_length(a, 3);
+  assertEquals(3, a.length);
+}
+
+function test2() {
+  var a = [];
+  set_length(a, 10);
+  set_length(a, 10);
+  Object.freeze(a);
+  set_length(a, 3);
+  set_length(a, 3);
+  assertEquals(10, a.length);
+}
+
+function test3() {
+  var a = [2];
+  Object.defineProperty(a, "length", {value:2, writable: false});
+  %ToFastProperties(a);
+  set_length([], 10);
+  set_length([], 10);
+  set_length(a, 10);
+  set_length(a, 10);
+  assertEquals(2, a.length);
+}
+
+test1();
+test2();
+test3();
=======================================
--- /branches/bleeding_edge/src/a64/code-stubs-a64.cc Thu Mar 13 10:57:07 2014 UTC +++ /branches/bleeding_edge/src/a64/code-stubs-a64.cc Mon Mar 17 15:43:33 2014 UTC
@@ -1987,77 +1987,6 @@
   StubCompiler::TailCallBuiltin(masm,
BaseLoadStoreStubCompiler::MissBuiltin(kind()));
 }
-
-
-void StoreArrayLengthStub::Generate(MacroAssembler* masm) {
-  ASM_LOCATION("StoreArrayLengthStub::Generate");
-  // This accepts as a receiver anything JSArray::SetElementsLength accepts
- // (currently anything except for external arrays which means anything with - // elements of FixedArray type). Value must be a number, but only smis are
-  // accepted as the most common case.
-  Label miss;
-
-  Register receiver;
-  Register value;
-  if (kind() == Code::KEYED_STORE_IC) {
-    // ----------- S t a t e -------------
-    //  -- lr    : return address
-    //  -- x2    : receiver
-    //  -- x1    : key
-    //  -- x0    : value
-    // -----------------------------------
-    Register key = x1;
-    receiver = x2;
-    value = x0;
-    __ Cmp(key, Operand(masm->isolate()->factory()->length_string()));
-    __ B(ne, &miss);
-  } else {
-    ASSERT(kind() == Code::STORE_IC);
-    // ----------- S t a t e -------------
-    //  -- lr    : return address
-    //  -- x2    : key
-    //  -- x1    : receiver
-    //  -- x0    : value
-    // -----------------------------------
-    receiver = x1;
-    value = x0;
-  }
-
-  // Check that the receiver isn't a smi.
-  __ JumpIfSmi(receiver, &miss);
-
-  // Check that the object is a JS array.
-  __ CompareObjectType(receiver, x10, x11, JS_ARRAY_TYPE);
-  __ B(ne, &miss);
-
-  // Check that elements are FixedArray.
-  // We rely on StoreIC_ArrayLength below to deal with all types of
-  // fast elements (including COW).
-  __ Ldr(x10, FieldMemOperand(receiver, JSArray::kElementsOffset));
-  __ CompareObjectType(x10, x11, x12, FIXED_ARRAY_TYPE);
-  __ B(ne, &miss);
-
-  // Check that the array has fast properties, otherwise the length
-  // property might have been redefined.
-  __ Ldr(x10, FieldMemOperand(receiver, JSArray::kPropertiesOffset));
-  __ Ldr(x10, FieldMemOperand(x10, FixedArray::kMapOffset));
-  __ CompareRoot(x10, Heap::kHashTableMapRootIndex);
-  __ B(eq, &miss);
-
-  // Check that value is a smi.
-  __ JumpIfNotSmi(value, &miss);
-
-  // Prepare tail call to StoreIC_ArrayLength.
-  __ Push(receiver, value);
-
-  ExternalReference ref =
- ExternalReference(IC_Utility(IC::kStoreIC_ArrayLength), masm->isolate());
-  __ TailCallExternalReference(ref, 2, 1);
-
-  __ Bind(&miss);
-  StubCompiler::TailCallBuiltin(masm,
- BaseLoadStoreStubCompiler::MissBuiltin(kind()));
-}


 void InstanceofStub::Generate(MacroAssembler* masm) {
=======================================
--- /branches/bleeding_edge/src/a64/stub-cache-a64.cc Thu Mar 13 10:57:07 2014 UTC +++ /branches/bleeding_edge/src/a64/stub-cache-a64.cc Mon Mar 17 15:43:33 2014 UTC
@@ -1422,6 +1422,17 @@
       (number_of_handled_maps > 1) ? POLYMORPHIC : MONOMORPHIC;
   return GetICCode(kind(), type, name, state);
 }
+
+
+void StoreStubCompiler::GenerateStoreArrayLength() {
+  // Prepare tail call to StoreIC_ArrayLength.
+  __ Push(receiver(), value());
+
+  ExternalReference ref =
+      ExternalReference(IC_Utility(IC::kStoreIC_ArrayLength),
+                        masm()->isolate());
+  __ TailCallExternalReference(ref, 2, 1);
+}


 Handle<Code> KeyedStoreStubCompiler::CompileStorePolymorphic(
=======================================
--- /branches/bleeding_edge/src/arm/code-stubs-arm.cc Thu Mar 13 10:57:07 2014 UTC +++ /branches/bleeding_edge/src/arm/code-stubs-arm.cc Mon Mar 17 15:43:33 2014 UTC
@@ -2118,77 +2118,6 @@
   StubCompiler::TailCallBuiltin(
       masm, BaseLoadStoreStubCompiler::MissBuiltin(kind()));
 }
-
-
-void StoreArrayLengthStub::Generate(MacroAssembler* masm) {
-  // This accepts as a receiver anything JSArray::SetElementsLength accepts
- // (currently anything except for external arrays which means anything with - // elements of FixedArray type). Value must be a number, but only smis are
-  // accepted as the most common case.
-  Label miss;
-
-  Register receiver;
-  Register value;
-  if (kind() == Code::KEYED_STORE_IC) {
-    // ----------- S t a t e -------------
-    //  -- lr    : return address
-    //  -- r0    : value
-    //  -- r1    : key
-    //  -- r2    : receiver
-    // -----------------------------------
-    __ cmp(r1, Operand(masm->isolate()->factory()->length_string()));
-    __ b(ne, &miss);
-    receiver = r2;
-    value = r0;
-  } else {
-    ASSERT(kind() == Code::STORE_IC);
-    // ----------- S t a t e -------------
-    //  -- lr    : return address
-    //  -- r0    : value
-    //  -- r1    : receiver
-    //  -- r2    : key
-    // -----------------------------------
-    receiver = r1;
-    value = r0;
-  }
-  Register scratch = r3;
-
-  // Check that the receiver isn't a smi.
-  __ JumpIfSmi(receiver, &miss);
-
-  // Check that the object is a JS array.
-  __ CompareObjectType(receiver, scratch, scratch, JS_ARRAY_TYPE);
-  __ b(ne, &miss);
-
-  // Check that elements are FixedArray.
-  // We rely on StoreIC_ArrayLength below to deal with all types of
-  // fast elements (including COW).
-  __ ldr(scratch, FieldMemOperand(receiver, JSArray::kElementsOffset));
-  __ CompareObjectType(scratch, scratch, scratch, FIXED_ARRAY_TYPE);
-  __ b(ne, &miss);
-
-  // Check that the array has fast properties, otherwise the length
-  // property might have been redefined.
-  __ ldr(scratch, FieldMemOperand(receiver, JSArray::kPropertiesOffset));
-  __ ldr(scratch, FieldMemOperand(scratch, FixedArray::kMapOffset));
-  __ CompareRoot(scratch, Heap::kHashTableMapRootIndex);
-  __ b(eq, &miss);
-
-  // Check that value is a smi.
-  __ JumpIfNotSmi(value, &miss);
-
-  // Prepare tail call to StoreIC_ArrayLength.
-  __ Push(receiver, value);
-
-  ExternalReference ref =
- ExternalReference(IC_Utility(IC::kStoreIC_ArrayLength), masm->isolate());
-  __ TailCallExternalReference(ref, 2, 1);
-
-  __ bind(&miss);
-
-  StubCompiler::TailCallBuiltin(
-      masm, BaseLoadStoreStubCompiler::MissBuiltin(kind()));
-}


 Register InstanceofStub::left() { return r0; }
=======================================
--- /branches/bleeding_edge/src/arm/stub-cache-arm.cc Thu Mar 13 10:57:07 2014 UTC +++ /branches/bleeding_edge/src/arm/stub-cache-arm.cc Mon Mar 17 15:43:33 2014 UTC
@@ -1434,6 +1434,17 @@
       number_of_handled_maps > 1 ? POLYMORPHIC : MONOMORPHIC;
   return GetICCode(kind(), type, name, state);
 }
+
+
+void StoreStubCompiler::GenerateStoreArrayLength() {
+  // Prepare tail call to StoreIC_ArrayLength.
+  __ Push(receiver(), value());
+
+  ExternalReference ref =
+      ExternalReference(IC_Utility(IC::kStoreIC_ArrayLength),
+                        masm()->isolate());
+  __ TailCallExternalReference(ref, 2, 1);
+}


 Handle<Code> KeyedStoreStubCompiler::CompileStorePolymorphic(
=======================================
--- /branches/bleeding_edge/src/code-stubs.h    Thu Mar 13 10:57:07 2014 UTC
+++ /branches/bleeding_edge/src/code-stubs.h    Mon Mar 17 15:43:33 2014 UTC
@@ -52,7 +52,6 @@
   V(CompareNilIC)                        \
   V(MathPow)                             \
   V(FunctionPrototype)                   \
-  V(StoreArrayLength)                    \
   V(RecordWrite)                         \
   V(StoreBufferOverflow)                 \
   V(RegExpExec)                          \
@@ -855,17 +854,6 @@
 };


-class StoreArrayLengthStub: public StoreICStub {
- public:
-  explicit StoreArrayLengthStub(Code::Kind kind, StrictMode strict_mode)
-      : StoreICStub(kind, strict_mode) { }
-  virtual void Generate(MacroAssembler* masm);
-
- private:
-  virtual CodeStub::Major MajorKey() { return StoreArrayLength; }
-};
-
-
 class HICStub: public HydrogenCodeStub {
  public:
   virtual Code::Kind GetCodeKind() const { return kind(); }
=======================================
--- /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Thu Mar 13 10:57:07 2014 UTC +++ /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Mon Mar 17 15:43:33 2014 UTC
@@ -1064,71 +1064,6 @@
   StubCompiler::TailCallBuiltin(
       masm, BaseLoadStoreStubCompiler::MissBuiltin(kind()));
 }
-
-
-void StoreArrayLengthStub::Generate(MacroAssembler* masm) {
-  // ----------- S t a t e -------------
-  //  -- eax    : value
-  //  -- ecx    : name
-  //  -- edx    : receiver
-  //  -- esp[0] : return address
-  // -----------------------------------
-  //
-  // This accepts as a receiver anything JSArray::SetElementsLength accepts
- // (currently anything except for external arrays which means anything with - // elements of FixedArray type). Value must be a number, but only smis are
-  // accepted as the most common case.
-
-  Label miss;
-
-  Register receiver = edx;
-  Register value = eax;
-  Register scratch = ebx;
-
-  if (kind() == Code::KEYED_STORE_IC) {
-    __ cmp(ecx, Immediate(masm->isolate()->factory()->length_string()));
-    __ j(not_equal, &miss);
-  }
-
-  // Check that the receiver isn't a smi.
-  __ JumpIfSmi(receiver, &miss);
-
-  // Check that the object is a JS array.
-  __ CmpObjectType(receiver, JS_ARRAY_TYPE, scratch);
-  __ j(not_equal, &miss);
-
-  // Check that elements are FixedArray.
-  // We rely on StoreIC_ArrayLength below to deal with all types of
-  // fast elements (including COW).
-  __ mov(scratch, FieldOperand(receiver, JSArray::kElementsOffset));
-  __ CmpObjectType(scratch, FIXED_ARRAY_TYPE, scratch);
-  __ j(not_equal, &miss);
-
-  // Check that the array has fast properties, otherwise the length
-  // property might have been redefined.
-  __ mov(scratch, FieldOperand(receiver, JSArray::kPropertiesOffset));
-  __ CompareRoot(FieldOperand(scratch, FixedArray::kMapOffset),
-                 Heap::kHashTableMapRootIndex);
-  __ j(equal, &miss);
-
-  // Check that value is a smi.
-  __ JumpIfNotSmi(value, &miss);
-
-  // Prepare tail call to StoreIC_ArrayLength.
-  __ pop(scratch);
-  __ push(receiver);
-  __ push(value);
-  __ push(scratch);  // return address
-
-  ExternalReference ref =
- ExternalReference(IC_Utility(IC::kStoreIC_ArrayLength), masm->isolate());
-  __ TailCallExternalReference(ref, 2, 1);
-
-  __ bind(&miss);
-
-  StubCompiler::TailCallBuiltin(
-      masm, BaseLoadStoreStubCompiler::MissBuiltin(kind()));
-}


 void ArgumentsAccessStub::GenerateReadElement(MacroAssembler* masm) {
=======================================
--- /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Thu Mar 13 10:57:07 2014 UTC +++ /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Mon Mar 17 15:43:33 2014 UTC
@@ -1272,6 +1272,20 @@
   // Return the generated code.
   return GetCode(kind(), Code::FAST, name);
 }
+
+
+void StoreStubCompiler::GenerateStoreArrayLength() {
+  // Prepare tail call to StoreIC_ArrayLength.
+  __ pop(scratch1());  // remove the return address
+  __ push(receiver());
+  __ push(value());
+  __ push(scratch1());  // restore return address
+
+  ExternalReference ref =
+      ExternalReference(IC_Utility(IC::kStoreIC_ArrayLength),
+                        masm()->isolate());
+  __ TailCallExternalReference(ref, 2, 1);
+}


 Handle<Code> KeyedStoreStubCompiler::CompileStorePolymorphic(
=======================================
--- /branches/bleeding_edge/src/ic.cc   Mon Mar 17 14:51:31 2014 UTC
+++ /branches/bleeding_edge/src/ic.cc   Mon Mar 17 15:43:33 2014 UTC
@@ -1233,26 +1233,6 @@
     RETURN_IF_EMPTY_HANDLE(isolate(), result);
     return *result;
   }
-
-  // Use specialized code for setting the length of arrays with fast
-  // properties. Slow properties might indicate redefinition of the length
-  // property. Note that when redefined using Object.freeze, it's possible
-  // to have fast properties but a read-only length.
-  if (FLAG_use_ic &&
-      receiver->IsJSArray() &&
-      name->Equals(isolate()->heap()->length_string()) &&
-      Handle<JSArray>::cast(receiver)->AllowsSetElementsLength() &&
-      receiver->HasFastProperties() &&
-      !receiver->map()->is_frozen()) {
-    Handle<Code> stub =
-        StoreArrayLengthStub(kind(), strict_mode()).GetCode(isolate());
-    set_target(*stub);
-    TRACE_IC("StoreIC", name);
-    Handle<Object> result = JSReceiver::SetProperty(
-        receiver, name, value, NONE, strict_mode(), store_mode);
-    RETURN_IF_EMPTY_HANDLE(isolate(), result);
-    return *result;
-  }

   LookupResult lookup(isolate());
   bool can_store = LookupForWrite(receiver, name, value, &lookup, this);
@@ -1404,6 +1384,17 @@
       // TODO(dcarney): Handle correctly.
       if (callback->IsDeclaredAccessorInfo()) break;
       ASSERT(callback->IsForeign());
+
+      // Use specialized code for setting the length of arrays with fast
+ // properties. Slow properties might indicate redefinition of the length
+      // property.
+      if (receiver->IsJSArray() &&
+          name->Equals(isolate()->heap()->length_string()) &&
+          Handle<JSArray>::cast(receiver)->AllowsSetElementsLength() &&
+          receiver->HasFastProperties()) {
+        return compiler.CompileStoreArrayLength(receiver, lookup, name);
+      }
+
       // No IC support for old-style native accessors.
       break;
     }
=======================================
--- /branches/bleeding_edge/src/stub-cache.cc   Tue Mar 11 14:41:22 2014 UTC
+++ /branches/bleeding_edge/src/stub-cache.cc   Mon Mar 17 15:43:33 2014 UTC
@@ -1116,6 +1116,30 @@
   // Return the generated code.
   return GetCode(kind(), Code::FAST, name);
 }
+
+
+Handle<Code> StoreStubCompiler::CompileStoreArrayLength(Handle<JSObject> object, + LookupResult* lookup, + Handle<Name> name) {
+  // This accepts as a receiver anything JSArray::SetElementsLength accepts
+ // (currently anything except for external arrays which means anything with + // elements of FixedArray type). Value must be a number, but only smis are
+  // accepted as the most common case.
+  Label miss;
+
+  // Check that value is a smi.
+  __ JumpIfNotSmi(value(), &miss);
+
+  // Generate tail call to StoreIC_ArrayLength.
+  GenerateStoreArrayLength();
+
+  // Handle miss case.
+  __ bind(&miss);
+  TailCallBuiltin(masm(), MissBuiltin(kind()));
+
+  // Return the generated code.
+  return GetCode(kind(), Code::FAST, name);
+}


 Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
=======================================
--- /branches/bleeding_edge/src/stub-cache.h    Thu Mar 13 10:57:07 2014 UTC
+++ /branches/bleeding_edge/src/stub-cache.h    Mon Mar 17 15:43:33 2014 UTC
@@ -679,6 +679,12 @@
                                  LookupResult* lookup,
                                  Handle<Name> name);

+  Handle<Code> CompileStoreArrayLength(Handle<JSObject> object,
+                                       LookupResult* lookup,
+                                       Handle<Name> name);
+
+  void GenerateStoreArrayLength();
+
   void GenerateNegativeHolderLookup(MacroAssembler* masm,
                                     Handle<JSObject> holder,
                                     Register holder_reg,
=======================================
--- /branches/bleeding_edge/src/x64/code-stubs-x64.cc Thu Mar 13 10:57:07 2014 UTC +++ /branches/bleeding_edge/src/x64/code-stubs-x64.cc Mon Mar 17 15:43:33 2014 UTC
@@ -931,70 +931,6 @@
   StubCompiler::TailCallBuiltin(
       masm, BaseLoadStoreStubCompiler::MissBuiltin(kind()));
 }
-
-
-void StoreArrayLengthStub::Generate(MacroAssembler* masm) {
-  // ----------- S t a t e -------------
-  //  -- rax    : value
-  //  -- rcx    : key
-  //  -- rdx    : receiver
-  //  -- rsp[0] : return address
-  // -----------------------------------
-  //
-  // This accepts as a receiver anything JSArray::SetElementsLength accepts
- // (currently anything except for external arrays which means anything with - // elements of FixedArray type). Value must be a number, but only smis are
-  // accepted as the most common case.
-
-  Label miss;
-
-  Register receiver = rdx;
-  Register value = rax;
-  Register scratch = rbx;
-  if (kind() == Code::KEYED_STORE_IC) {
-    __ Cmp(rcx, masm->isolate()->factory()->length_string());
-    __ j(not_equal, &miss);
-  }
-
-  // Check that the receiver isn't a smi.
-  __ JumpIfSmi(receiver, &miss);
-
-  // Check that the object is a JS array.
-  __ CmpObjectType(receiver, JS_ARRAY_TYPE, scratch);
-  __ j(not_equal, &miss);
-
-  // Check that elements are FixedArray.
-  // We rely on StoreIC_ArrayLength below to deal with all types of
-  // fast elements (including COW).
-  __ movp(scratch, FieldOperand(receiver, JSArray::kElementsOffset));
-  __ CmpObjectType(scratch, FIXED_ARRAY_TYPE, scratch);
-  __ j(not_equal, &miss);
-
-  // Check that the array has fast properties, otherwise the length
-  // property might have been redefined.
-  __ movp(scratch, FieldOperand(receiver, JSArray::kPropertiesOffset));
-  __ CompareRoot(FieldOperand(scratch, FixedArray::kMapOffset),
-                 Heap::kHashTableMapRootIndex);
-  __ j(equal, &miss);
-
-  // Check that value is a smi.
-  __ JumpIfNotSmi(value, &miss);
-
-  // Prepare tail call to StoreIC_ArrayLength.
-  __ PopReturnAddressTo(scratch);
-  __ push(receiver);
-  __ push(value);
-  __ PushReturnAddressFrom(scratch);
-
-  ExternalReference ref =
- ExternalReference(IC_Utility(IC::kStoreIC_ArrayLength), masm->isolate());
-  __ TailCallExternalReference(ref, 2, 1);
-
-  __ bind(&miss);
-
-  StubCompiler::TailCallBuiltin(
-      masm, BaseLoadStoreStubCompiler::MissBuiltin(kind()));
-}


 void ArgumentsAccessStub::GenerateReadElement(MacroAssembler* masm) {
=======================================
--- /branches/bleeding_edge/src/x64/stub-cache-x64.cc Thu Mar 13 10:57:07 2014 UTC +++ /branches/bleeding_edge/src/x64/stub-cache-x64.cc Mon Mar 17 15:43:33 2014 UTC
@@ -1167,6 +1167,20 @@
   // Return the generated code.
   return GetCode(kind(), Code::FAST, name);
 }
+
+
+void StoreStubCompiler::GenerateStoreArrayLength() {
+  // Prepare tail call to StoreIC_ArrayLength.
+  __ PopReturnAddressTo(scratch1());
+  __ push(receiver());
+  __ push(value());
+  __ PushReturnAddressFrom(scratch1());
+
+  ExternalReference ref =
+      ExternalReference(IC_Utility(IC::kStoreIC_ArrayLength),
+                        masm()->isolate());
+  __ TailCallExternalReference(ref, 2, 1);
+}


 Handle<Code> KeyedStoreStubCompiler::CompileStorePolymorphic(

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to