Handle accessors on the prototype chain in StoreICs.

Made stub compiler function signatures a bit more consistent on the way.

Review URL: https://chromiumcodereview.appspot.com/10735003

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

diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc
index 54114db..14d0bfd 100644 (file)
--- a/src/arm/stub-cache-arm.cc
+++ b/src/arm/stub-cache-arm.cc
@@ -2671,9 +2671,10 @@ Handle<Code> StoreStubCompiler::CompileStoreCallback(
 
 
 Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
+    Handle<String> name,
     Handle<JSObject> receiver,
-    Handle<JSFunction> setter,
-    Handle<String> name) {
+    Handle<JSObject> holder,
+    Handle<JSFunction> setter) {
   // ----------- S t a t e -------------
   //  -- r0    : value
   //  -- r1    : receiver
@@ -2682,9 +2683,9 @@ Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
   // -----------------------------------
   Label miss;
 
-  // Check that the map of the object hasn't changed.
-  __ CheckMap(r1, r3, Handle<Map>(receiver->map()), &miss, DO_SMI_CHECK,
-              ALLOW_ELEMENT_TRANSITION_MAPS);
+  // Check that the maps haven't changed.
+  __ JumpIfSmi(r1, &miss);
+  CheckPrototypes(receiver, r1, holder, r3, r4, r5, name, &miss);
 
   {
     FrameScope scope(masm(), StackFrame::INTERNAL);
@@ -2692,7 +2693,7 @@ Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
     // Save value register, so we can restore it later.
     __ push(r0);
 
-    // Call the JavaScript getter with the receiver and the value on the stack.
+    // Call the JavaScript setter with the receiver and the value on the stack.
     __ Push(r1, r0);
     ParameterCount actual(1);
     __ InvokeFunction(setter, actual, CALL_FUNCTION, NullCallWrapper(),

diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc
index 78a5b68..c1a5c39 100644 (file)
--- a/src/ia32/stub-cache-ia32.cc
+++ b/src/ia32/stub-cache-ia32.cc
@@ -2590,9 +2590,10 @@ Handle<Code> StoreStubCompiler::CompileStoreCallback(
 
 
 Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
+    Handle<String> name,
     Handle<JSObject> receiver,
-    Handle<JSFunction> setter,
-    Handle<String> name) {
+    Handle<JSObject> holder,
+    Handle<JSFunction> setter) {
   // ----------- S t a t e -------------
   //  -- eax    : value
   //  -- ecx    : name
@@ -2601,9 +2602,11 @@ Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
   // -----------------------------------
   Label miss;
 
-  // Check that the map of the object hasn't changed.
-  __ CheckMap(edx, Handle<Map>(receiver->map()), &miss, DO_SMI_CHECK,
-              ALLOW_ELEMENT_TRANSITION_MAPS);
+  // Check that the maps haven't changed, preserving the name register.
+  __ push(ecx);
+  __ JumpIfSmi(edx, &miss);
+  CheckPrototypes(receiver, edx, holder, ebx, ecx, edi, name, &miss);
+  __ pop(ecx);
 
   {
     FrameScope scope(masm(), StackFrame::INTERNAL);
@@ -2611,7 +2614,7 @@ Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
     // Save value register, so we can restore it later.
     __ push(eax);
 
-    // Call the JavaScript getter with the receiver and the value on the stack.
+    // Call the JavaScript setter with the receiver and the value on the stack.
     __ push(edx);
     __ push(eax);
     ParameterCount actual(1);
@@ -2627,6 +2630,7 @@ Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
   __ ret(0);
 
   __ bind(&miss);
+  __ pop(ecx);
   Handle<Code> ic = isolate()->builtins()->StoreIC_Miss();
   __ jmp(ic, RelocInfo::CODE_TARGET);
 

diff --git a/src/ic.cc b/src/ic.cc
index 1f0070b..e3663c0 100644 (file)
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1315,7 +1315,15 @@ static bool LookupForWrite(Handle<JSObject> receiver,
                            LookupResult* lookup) {
   receiver->LocalLookup(*name, lookup);
   if (!StoreICableLookup(lookup)) {
-    return false;
+    // 2nd chance: There can be accessors somewhere in the prototype chain, but
+    // for compatibility reasons we have to hide this behind a flag. Note that
+    // we explicitly exclude native accessors for now, because the stubs are not
+    // yet prepared for this scenario.
+    if (!FLAG_es5_readonly) return false;
+    receiver->Lookup(*name, lookup);
+    if (!lookup->IsCallbacks()) return false;
+    Handle<Object> callback(lookup->GetCallbackObject());
+    return callback->IsAccessorPair() && StoreICableLookup(lookup);
   }
 
   if (lookup->IsInterceptor() &&
@@ -1494,7 +1502,8 @@ void StoreIC::UpdateCaches(LookupResult* lookup,
         if (holder->IsGlobalObject()) return;
         if (!receiver->HasFastProperties()) return;
         code = isolate()->stub_cache()->ComputeStoreViaSetter(
-            name, receiver, Handle<JSFunction>::cast(setter), strict_mode);
+            name, receiver, holder, Handle<JSFunction>::cast(setter),
+            strict_mode);
       } else {
         ASSERT(callback->IsForeign());
         // No IC support for old-style native accessors.

diff --git a/src/mips/stub-cache-mips.cc b/src/mips/stub-cache-mips.cc
index d4d4de0..cf6ba81 100644 (file)
--- a/src/mips/stub-cache-mips.cc
+++ b/src/mips/stub-cache-mips.cc
@@ -2675,9 +2675,10 @@ Handle<Code> StoreStubCompiler::CompileStoreCallback(
 
 
 Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
+    Handle<String> name,
     Handle<JSObject> receiver,
-    Handle<JSFunction> setter,
-    Handle<String> name) {
+    Handle<JSObject> holder,
+    Handle<JSFunction> setter) {
   // ----------- S t a t e -------------
   //  -- a0    : value
   //  -- a1    : receiver
@@ -2686,9 +2687,9 @@ Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
   // -----------------------------------
   Label miss;
 
-  // Check that the map of the object hasn't changed.
-  __ CheckMap(a1, a3, Handle<Map>(receiver->map()), &miss, DO_SMI_CHECK,
-              ALLOW_ELEMENT_TRANSITION_MAPS);
+  // Check that the maps haven't changed.
+  __ JumpIfSmi(a1, &miss);
+  CheckPrototypes(receiver, a1, holder, a3, t0, t1, name, &miss);
 
   {
     FrameScope scope(masm(), StackFrame::INTERNAL);
@@ -2696,7 +2697,7 @@ Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
     // Save value register, so we can restore it later.
     __ push(a0);
 
-    // Call the JavaScript getter with the receiver and the value on the stack.
+    // Call the JavaScript setter with the receiver and the value on the stack.
     __ push(a1);
     __ push(a0);
     ParameterCount actual(1);

diff --git a/src/stub-cache.cc b/src/stub-cache.cc
index c5ae175..9adf3cb 100644 (file)
--- a/src/stub-cache.cc
+++ b/src/stub-cache.cc
@@ -523,6 +523,7 @@ Handle<Code> StubCache::ComputeStoreCallback(Handle<String> name,
 
 Handle<Code> StubCache::ComputeStoreViaSetter(Handle<String> name,
                                               Handle<JSObject> receiver,
+                                              Handle<JSObject> holder,
                                               Handle<JSFunction> setter,
                                               StrictModeFlag strict_mode) {
   Code::Flags flags = Code::ComputeMonomorphicFlags(
@@ -531,7 +532,8 @@ Handle<Code> StubCache::ComputeStoreViaSetter(Handle<String> name,
   if (probe->IsCode()) return Handle<Code>::cast(probe);
 
   StoreStubCompiler compiler(isolate_, strict_mode);
-  Handle<Code> code = compiler.CompileStoreViaSetter(receiver, setter, name);
+  Handle<Code> code =
+      compiler.CompileStoreViaSetter(name, receiver, holder, setter);
   PROFILE(isolate_, CodeCreateEvent(Logger::STORE_IC_TAG, *code, *name));
   GDBJIT(AddCode(GDBJITInterface::STORE_IC, *name, *code));
   JSObject::UpdateMapCodeCache(receiver, name, code);

diff --git a/src/stub-cache.h b/src/stub-cache.h
index e274c82..be13fdd 100644 (file)
--- a/src/stub-cache.h
+++ b/src/stub-cache.h
@@ -164,6 +164,7 @@ class StubCache {
 
   Handle<Code> ComputeStoreViaSetter(Handle<String> name,
                                      Handle<JSObject> receiver,
+                                     Handle<JSObject> holder,
                                      Handle<JSFunction> setter,
                                      StrictModeFlag strict_mode);
 
@@ -697,9 +698,10 @@ class StoreStubCompiler: public StubCompiler {
                                     Handle<AccessorInfo> callback,
                                     Handle<String> name);
 
-  Handle<Code> CompileStoreViaSetter(Handle<JSObject> receiver,
-                                     Handle<JSFunction> setter,
-                                     Handle<String> name);
+  Handle<Code> CompileStoreViaSetter(Handle<String> name,
+                                     Handle<JSObject> receiver,
+                                     Handle<JSObject> holder,
+                                     Handle<JSFunction> setter);
 
   Handle<Code> CompileStoreInterceptor(Handle<JSObject> object,
                                        Handle<String> name);

diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc
index a2adf06..df6b5d5 100644 (file)
--- a/src/x64/stub-cache-x64.cc
+++ b/src/x64/stub-cache-x64.cc
@@ -2427,9 +2427,10 @@ Handle<Code> StoreStubCompiler::CompileStoreCallback(
 
 
 Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
+    Handle<String> name,
     Handle<JSObject> receiver,
-    Handle<JSFunction> setter,
-    Handle<String> name) {
+    Handle<JSObject> holder,
+    Handle<JSFunction> setter) {
   // ----------- S t a t e -------------
   //  -- rax    : value
   //  -- rcx    : name
@@ -2438,9 +2439,9 @@ Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
   // -----------------------------------
   Label miss;
 
-  // Check that the map of the object hasn't changed.
-  __ CheckMap(rdx, Handle<Map>(receiver->map()), &miss, DO_SMI_CHECK,
-              ALLOW_ELEMENT_TRANSITION_MAPS);
+  // Check that the maps haven't changed.
+  __ JumpIfSmi(rdx, &miss);
+  CheckPrototypes(receiver, rdx, holder, rbx, r8, rdi, name, &miss);
 
   {
     FrameScope scope(masm(), StackFrame::INTERNAL);
@@ -2448,7 +2449,7 @@ Handle<Code> StoreStubCompiler::CompileStoreViaSetter(
     // Save value register, so we can restore it later.
     __ push(rax);
 
-    // Call the JavaScript getter with the receiver and the value on the stack.
+    // Call the JavaScript setter with the receiver and the value on the stack.
     __ push(rdx);
     __ push(rax);
     ParameterCount actual(1);

diff --git a/test/mjsunit/accessor-map-sharing.js b/test/mjsunit/accessor-map-sharing.js
index ab45afa..3afce37 100644 (file)
--- a/test/mjsunit/accessor-map-sharing.js
+++ b/test/mjsunit/accessor-map-sharing.js
@@ -35,7 +35,7 @@ function getter() { return 111; }
 function setter(x) { print(222); }
 function anotherGetter() { return 333; }
 function anotherSetter(x) { print(444); }
-var obj1, obj2;
+var obj1, obj2, obj3, obj4;
 
 // Two objects with the same getter.
 obj1 = {};
@@ -174,3 +174,19 @@ assertEquals(getter, gop(obj1, "papa").get);
 assertEquals(setter, gop(obj1, "papa").set);
 assertTrue(gop(obj1, "papa").configurable);
 assertFalse(gop(obj1, "papa").enumerable);
+
+// Two objects with the same getter on the prototype chain.
+obj1 = {};
+dp(obj1, "quebec", { get: getter });
+obj2 = Object.create(obj1);
+obj3 = Object.create(obj2);
+obj4 = Object.create(obj2);
+assertTrue(%HaveSameMap(obj3, obj4));
+
+// Two objects with the same setter on the prototype chain.
+obj1 = {};
+dp(obj1, "romeo", { set: setter });
+obj2 = Object.create(obj1);
+obj3 = Object.create(obj2);
+obj4 = Object.create(obj2);
+assertTrue(%HaveSameMap(obj3, obj4));

diff --git a/test/mjsunit/object-define-property.js b/test/mjsunit/object-define-property.js
index fdaf82d..56e67c3 100644 (file)
--- a/test/mjsunit/object-define-property.js
+++ b/test/mjsunit/object-define-property.js
@@ -27,7 +27,7 @@
 
 // Tests the object.defineProperty method - ES 15.2.3.6
 
-// Flags: --allow-natives-syntax
+// Flags: --allow-natives-syntax --es5-readonly
 
 // Check that an exception is thrown when null is passed as object.
 var exception = false;
@@ -1085,3 +1085,90 @@ assertEquals(undefined, objectWithGetter.__lookupSetter__('foo'));
 var objectWithSetter = {};
 objectWithSetter.__defineSetter__('foo', function(x) {});
 assertEquals(undefined, objectWithSetter.__lookupGetter__('foo'));
+
+// An object with a getter on the prototype chain.
+function getter() { return 111; }
+function anotherGetter() { return 222; }
+
+function testGetterOnProto(expected, o) {
+  assertEquals(expected, o.quebec);
+}
+
+obj1 = {};
+Object.defineProperty(obj1, "quebec", { get: getter, configurable: true });
+obj2 = Object.create(obj1);
+obj3 = Object.create(obj2);
+
+testGetterOnProto(111, obj3);
+testGetterOnProto(111, obj3);
+%OptimizeFunctionOnNextCall(testGetterOnProto);
+testGetterOnProto(111, obj3);
+testGetterOnProto(111, obj3);
+
+Object.defineProperty(obj1, "quebec", { get: anotherGetter });
+
+testGetterOnProto(222, obj3);
+testGetterOnProto(222, obj3);
+%OptimizeFunctionOnNextCall(testGetterOnProto);
+testGetterOnProto(222, obj3);
+testGetterOnProto(222, obj3);
+
+// An object with a setter on the prototype chain.
+var modifyMe;
+function setter(x) { modifyMe = x+1; }
+function anotherSetter(x) { modifyMe = x+2; }
+
+function testSetterOnProto(expected, o) {
+  modifyMe = 333;
+  o.romeo = 444;
+  assertEquals(expected, modifyMe);
+}
+
+obj1 = {};
+Object.defineProperty(obj1, "romeo", { set: setter, configurable: true });
+obj2 = Object.create(obj1);
+obj3 = Object.create(obj2);
+
+testSetterOnProto(445, obj3);
+testSetterOnProto(445, obj3);
+%OptimizeFunctionOnNextCall(testSetterOnProto);
+testSetterOnProto(445, obj3);
+testSetterOnProto(445, obj3);
+
+Object.defineProperty(obj1, "romeo", { set: anotherSetter });
+
+testSetterOnProto(446, obj3);
+testSetterOnProto(446, obj3);
+%OptimizeFunctionOnNextCall(testSetterOnProto);
+testSetterOnProto(446, obj3);
+testSetterOnProto(446, obj3);
+
+// Removing a setter on the prototype chain.
+function testSetterOnProtoStrict(o) {
+  "use strict";
+  o.sierra = 12345;
+}
+
+obj1 = {};
+Object.defineProperty(obj1, "sierra",
+                      { get: getter, set: setter, configurable: true });
+obj2 = Object.create(obj1);
+obj3 = Object.create(obj2);
+
+testSetterOnProtoStrict(obj3);
+testSetterOnProtoStrict(obj3);
+%OptimizeFunctionOnNextCall(testSetterOnProtoStrict);
+testSetterOnProtoStrict(obj3);
+testSetterOnProtoStrict(obj3);
+
+Object.defineProperty(obj1, "sierra",
+                      { get: getter, set: undefined, configurable: true });
+
+exception = false;
+try {
+  testSetterOnProtoStrict(obj3);
+} catch (e) {
+  exception = true;
+  assertTrue(/which has only a getter/.test(e));
+}
+assertTrue(exception);