Fixed spec violation of storing to length of a frozen object.
authorishell@chromium.org <ishell@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 17 Mar 2014 15:43:33 +0000 (15:43 +0000)
committerishell@chromium.org <ishell@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 17 Mar 2014 15:43:33 +0000 (15:43 +0000)
BUG=chromium:350890
LOG=N
R=verwaest@chromium.org

Review URL: https://codereview.chromium.org/196653015

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20005 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

13 files changed:
src/a64/code-stubs-a64.cc
src/a64/stub-cache-a64.cc
src/arm/code-stubs-arm.cc
src/arm/stub-cache-arm.cc
src/code-stubs.h
src/ia32/code-stubs-ia32.cc
src/ia32/stub-cache-ia32.cc
src/ic.cc
src/stub-cache.cc
src/stub-cache.h
src/x64/code-stubs-x64.cc
src/x64/stub-cache-x64.cc
test/mjsunit/regress/regress-crbug-350890.js [new file with mode: 0644]

index 6a74e7a..2996df9 100644 (file)
@@ -1989,77 +1989,6 @@ void FunctionPrototypeStub::Generate(MacroAssembler* masm) {
 }
 
 
-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) {
   // Stack on entry:
   // jssp[0]: function.
index bbcb64b..2f3ffff 100644 (file)
@@ -1424,6 +1424,17 @@ Handle<Code> BaseLoadStoreStubCompiler::CompilePolymorphicIC(
 }
 
 
+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(
     MapHandleList* receiver_maps,
     CodeHandleList* handler_stubs,
index 81da465..ca8e001 100644 (file)
@@ -2120,77 +2120,6 @@ void FunctionPrototypeStub::Generate(MacroAssembler* masm) {
 }
 
 
-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; }
 
 
index 35aab5f..c595e42 100644 (file)
@@ -1436,6 +1436,17 @@ Handle<Code> BaseLoadStoreStubCompiler::CompilePolymorphicIC(
 }
 
 
+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(
     MapHandleList* receiver_maps,
     CodeHandleList* handler_stubs,
index b65f085..d4240c8 100644 (file)
@@ -52,7 +52,6 @@ namespace internal {
   V(CompareNilIC)                        \
   V(MathPow)                             \
   V(FunctionPrototype)                   \
-  V(StoreArrayLength)                    \
   V(RecordWrite)                         \
   V(StoreBufferOverflow)                 \
   V(RegExpExec)                          \
@@ -855,17 +854,6 @@ class StoreICStub: public ICStub {
 };
 
 
-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(); }
index 4ff8a68..929b9db 100644 (file)
@@ -1066,71 +1066,6 @@ void FunctionPrototypeStub::Generate(MacroAssembler* masm) {
 }
 
 
-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) {
   // The key is in edx and the parameter count is in eax.
 
index 33cbf2b..28f4028 100644 (file)
@@ -1274,6 +1274,20 @@ Handle<Code> StoreStubCompiler::CompileStoreInterceptor(
 }
 
 
+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(
     MapHandleList* receiver_maps,
     CodeHandleList* handler_stubs,
index 335e687..a3f99e4 100644 (file)
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1234,26 +1234,6 @@ MaybeObject* StoreIC::Store(Handle<Object> object,
     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);
   if (!can_store &&
@@ -1404,6 +1384,17 @@ Handle<Code> StoreIC::CompileHandler(LookupResult* lookup,
       // 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;
     }
index 52e7f4c..8368e9b 100644 (file)
@@ -1118,6 +1118,30 @@ Handle<Code> StoreStubCompiler::CompileStoreField(Handle<JSObject> object,
 }
 
 
+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(
     Handle<JSObject> object,
     Handle<JSObject> holder,
index f6528c8..7a304fe 100644 (file)
@@ -679,6 +679,12 @@ class StoreStubCompiler: public BaseLoadStoreStubCompiler {
                                  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,
index d1ea51b..779e77e 100644 (file)
@@ -933,70 +933,6 @@ void FunctionPrototypeStub::Generate(MacroAssembler* masm) {
 }
 
 
-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) {
   // The key is in rdx and the parameter count is in rax.
 
index dcadce5..6463a37 100644 (file)
@@ -1169,6 +1169,20 @@ Handle<Code> StoreStubCompiler::CompileStoreInterceptor(
 }
 
 
+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(
     MapHandleList* receiver_maps,
     CodeHandleList* handler_stubs,
diff --git a/test/mjsunit/regress/regress-crbug-350890.js b/test/mjsunit/regress/regress-crbug-350890.js
new file mode 100644 (file)
index 0000000..b60a2aa
--- /dev/null
@@ -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();