Revision: 14061
Author: verwa...@chromium.org
Date: Mon Mar 25 05:55:27 2013
Log: Change LookupForWrite to always do a full lookup and check the
result.
If we find a property in the prototype-chain that we can overwrite, and
we have a transition, keep the holder in the lookup-result as the actual
holder. We will need it for the consistency-check in GenerateStoreField.
By directly checking the entire chain we avoid having to lazily bail out
to a copy of the miss stub while generating the Field Store IC.
Currently this CL disallows a normal non-receiver holder, given that
that would require a positive lookup + details verification to ensure
the property did not become read-only. This fixes the regressions in the
attached tests.
Review URL: https://chromiumcodereview.appspot.com/12810006
http://code.google.com/p/v8/source/detail?r=14061
Added:
/branches/bleeding_edge/test/mjsunit/regress/negative_lookup.js
/branches/bleeding_edge/test/mjsunit/regress/readonly1.js
/branches/bleeding_edge/test/mjsunit/regress/readonly2.js
/branches/bleeding_edge/test/mjsunit/regress/readonly3.js
/branches/bleeding_edge/test/mjsunit/regress/readonly4.js
/branches/bleeding_edge/test/mjsunit/regress/setter.js
Modified:
/branches/bleeding_edge/src/arm/stub-cache-arm.cc
/branches/bleeding_edge/src/ia32/stub-cache-ia32.cc
/branches/bleeding_edge/src/ic.cc
/branches/bleeding_edge/src/property.h
/branches/bleeding_edge/src/stub-cache.cc
/branches/bleeding_edge/src/stub-cache.h
/branches/bleeding_edge/src/x64/stub-cache-x64.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/negative_lookup.js Mon Mar
25 05:55:27 2013
@@ -0,0 +1,65 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+function s(v) {
+ v.x = 1;
+}
+
+function c(p) {
+ return {__proto__: p};
+}
+
+var p = {};
+
+// Make p the last prototype in the chain.
+p.__proto__ = null;
+
+var o1 = c(p);
+var o2 = c(p);
+var o3 = c(p);
+var o4 = c(p);
+
+// Make y go to slow mode.
+// Do this after using p as prototype, since using an object as prototype
kicks
+// it back into fast mode.
+p.y = 1;
+delete p.y;
+
+// Initialize the store IC.
+s(o1);
+s(o2);
+
+// Do something with x in slow-mode p.
+Object.defineProperty(p, "x", { writable: false, value: 5 });
+
+// Verify that directly setting x fails.
+o3.x = 10;
+assertEquals(5, o3.x);
+
+// Verify that setting x through the IC fails.
+s(o4);
+assertEquals(5, o4.x);
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/readonly1.js Mon Mar 25
05:55:27 2013
@@ -0,0 +1,71 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+function s(v) {
+ v.x = 1;
+}
+
+function s_strict(v) {
+ "use strict";
+ v.x = 1;
+}
+
+function c(p) {
+ return {__proto__: p};
+}
+
+var p = {};
+
+var o1 = c(p);
+var o2 = c(p);
+var o3 = c(p);
+var o4 = c(p);
+
+// Make p go slow.
+// Do this after using p as prototype, since using an object as prototype
kicks
+// it back into fast mode.
+p.y = 1;
+delete p.y;
+p.x = 5;
+
+// Initialize the store IC.
+s(o1);
+s(o2);
+s_strict(o1);
+s_strict(o2);
+
+// Make x non-writable.
+Object.defineProperty(p, "x", { writable: false });
+
+// Verify that direct setting fails.
+o3.x = 20;
+assertEquals(5, o3.x);
+
+// Verify that setting through the IC fails.
+s(o4);
+assertEquals(5, o4.x);
+assertThrows("s_strict(o4);", TypeError);
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/readonly2.js Mon Mar 25
05:55:27 2013
@@ -0,0 +1,62 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+Object.defineProperty(this, "x", { writable:true });
+
+function s(v) {
+ v.x = 1;
+}
+
+function s_strict(v) {
+ "use strict";
+ v.x = 1;
+}
+
+function c(p) {
+ return {__proto__: p};
+}
+
+var o1 = c(this);
+var o2 = c(this);
+
+// Initialize the store IC.
+s(c(this));
+s(c(this));
+s_strict(c(this));
+s_strict(c(this));
+
+// Make x non-writable.
+Object.defineProperty(this, "x", { writable:false, value:5 });
+
+// Verify that direct setting fails.
+o1.x = 20;
+assertEquals(5, o1.x);
+
+// Verify that setting through the IC fails.
+s(o2);
+assertEquals(5, o2.x);
+assertThrows("s_strict(o2);", TypeError);
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/readonly3.js Mon Mar 25
05:55:27 2013
@@ -0,0 +1,65 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+this.x = 0;
+
+var p = {};
+Object.defineProperty(p, "x", {writable:false, value:5});
+this.__proto__ = p;
+
+function s(v) {
+ v.x = 1;
+}
+
+function s_strict(v) {
+ "use strict";
+ v.x = 1;
+}
+
+function c(p) {
+ return {__proto__: p};
+}
+
+var o1 = c(this);
+var o2 = c(this);
+
+// Initialize the store IC.
+s(c(this));
+s(c(this));
+s_strict(c(this));
+s_strict(c(this));
+
+delete this.x;
+
+// Verify that direct setting fails.
+o1.x = 20;
+assertEquals(5, o1.x);
+
+// Verify that setting through the IC fails.
+s(o2);
+assertEquals(5, o2.x);
+assertThrows("s_strict(o2);", TypeError);
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/readonly4.js Mon Mar 25
05:55:27 2013
@@ -0,0 +1,74 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+var slow = {};
+var p = {};
+
+slow.__proto__ = p;
+slow.x = 10;
+slow.y = 10;
+Object.defineProperty(p, "x", {writable:false, value:5});
+
+function c(p) {
+ return {__proto__: p};
+}
+
+function s(v) {
+ return v.x = 1;
+}
+
+function s_strict(v) {
+ "use strict";
+ v.x = 1;
+}
+
+var o1 = c(slow);
+var o2 = c(slow);
+var o1_strict = c(slow);
+var o2_strict = c(slow);
+var o3 = c(slow);
+var o4 = c(slow);
+
+// Make s slow.
+// Do this after using slow as prototype, since using an object as
prototype
+// kicks it back into fast mode.
+delete slow.y;
+
+s(o1);
+s(o2);
+s_strict(o1_strict);
+s_strict(o2_strict);
+
+delete slow.x;
+// Directly setting x should fail.
+o3.x = 20
+assertEquals(5, o3.x);
+
+// Setting x through IC should fail.
+s(o4);
+assertEquals(5, o4.x);
+assertThrows("s_strict(o4);", TypeError);
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/setter.js Mon Mar 25
05:55:27 2013
@@ -0,0 +1,66 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+function s(v) {
+ v.x = 1;
+}
+
+function c(p) {
+ return {__proto__: p};
+}
+
+var p = {};
+
+var o1 = c(p);
+var o2 = c(p);
+var o3 = c(p);
+var o4 = c(p);
+var o5 = c(p);
+
+// Initialize the store IC.
+s(o1);
+s(o2);
+
+// Install a setter on p.x
+var count = 0;
+Object.defineProperty(p, "x", {
+ set: function(x) {
+ count += 1;
+ }
+});
+
+// Verify that the setter was called directly.
+o3.x = 20;
+assertEquals(1, count);
+
+// Verify that monomorphic prototype failure is triggered in the IC.
+s(o4);
+assertEquals(2, count);
+
+// Verify that the newly installed IC is correct.
+s(o5);
+assertEquals(3, count);
=======================================
--- /branches/bleeding_edge/src/arm/stub-cache-arm.cc Fri Mar 22 09:33:50
2013
+++ /branches/bleeding_edge/src/arm/stub-cache-arm.cc Mon Mar 25 05:55:27
2013
@@ -423,7 +423,7 @@
// registers have their original values.
void StubCompiler::GenerateStoreField(MacroAssembler* masm,
Handle<JSObject> object,
- int index,
+ LookupResult* lookup,
Handle<Map> transition,
Handle<Name> name,
Register receiver_reg,
@@ -435,16 +435,6 @@
Label* miss_restore_name) {
// r0 : value
Label exit;
-
- LookupResult lookup(masm->isolate());
- object->Lookup(*name, &lookup);
- if (lookup.IsFound() && (lookup.IsReadOnly() || !lookup.IsCacheable())) {
- // In sloppy mode, we could just return the value and be done.
However, we
- // might be in strict mode, where we have to throw. Since we cannot
tell,
- // go into slow case unconditionally.
- __ jmp(miss_label);
- return;
- }
// Check that the map of the object hasn't changed.
CompareMapMode mode = transition.is_null() ?
ALLOW_ELEMENT_TRANSITION_MAPS
@@ -460,8 +450,9 @@
// Check that we are allowed to write this.
if (!transition.is_null() && object->GetPrototype()->IsJSObject()) {
JSObject* holder;
- if (lookup.IsFound()) {
- holder = lookup.holder();
+ // holder == object indicates that no property was found.
+ if (lookup->holder() != *object) {
+ holder = lookup->holder();
} else {
// Find the top object.
holder = *object;
@@ -469,8 +460,19 @@
holder = JSObject::cast(holder->GetPrototype());
} while (holder->GetPrototype()->IsJSObject());
}
- CheckPrototypes(object, receiver_reg, Handle<JSObject>(holder),
name_reg,
- scratch1, scratch2, name, miss_restore_name);
+ Register holder_reg = CheckPrototypes(
+ object, receiver_reg, Handle<JSObject>(holder), name_reg,
+ scratch1, scratch2, name, miss_restore_name);
+ // If no property was found, and the holder (the last object in the
+ // prototype chain) is in slow mode, we need to do a negative lookup
on the
+ // holder.
+ if (lookup->holder() == *object &&
+ !holder->HasFastProperties() &&
+ !holder->IsJSGlobalProxy() &&
+ !holder->IsJSGlobalObject()) {
+ GenerateDictionaryNegativeLookup(
+ masm, miss_restore_name, holder_reg, name, scratch1, scratch2);
+ }
}
// Stub never generated for non-global objects that require access
@@ -492,6 +494,7 @@
return;
}
+ int index;
if (!transition.is_null()) {
// Update the map of the object.
__ mov(scratch1, Operand(transition));
@@ -507,6 +510,10 @@
kDontSaveFPRegs,
OMIT_REMEMBERED_SET,
OMIT_SMI_CHECK);
+ index = transition->instance_descriptors()->GetFieldIndex(
+ transition->LastAdded());
+ } else {
+ index = lookup->GetFieldIndex().field_index();
}
// Adjust for the number of properties stored in the object. Even in the
=======================================
--- /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Fri Mar 22 09:33:50
2013
+++ /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Mon Mar 25 05:55:27
2013
@@ -730,7 +730,7 @@
// but may be destroyed if store is successful.
void StubCompiler::GenerateStoreField(MacroAssembler* masm,
Handle<JSObject> object,
- int index,
+ LookupResult* lookup,
Handle<Map> transition,
Handle<Name> name,
Register receiver_reg,
@@ -740,16 +740,6 @@
Register scratch2,
Label* miss_label,
Label* miss_restore_name) {
- LookupResult lookup(masm->isolate());
- object->Lookup(*name, &lookup);
- if (lookup.IsFound() && (lookup.IsReadOnly() || !lookup.IsCacheable())) {
- // In sloppy mode, we could just return the value and be done.
However, we
- // might be in strict mode, where we have to throw. Since we cannot
tell,
- // go into slow case unconditionally.
- __ jmp(miss_label);
- return;
- }
-
// Check that the map of the object hasn't changed.
CompareMapMode mode = transition.is_null() ?
ALLOW_ELEMENT_TRANSITION_MAPS
: REQUIRE_EXACT_MAP;
@@ -764,8 +754,9 @@
// Check that we are allowed to write this.
if (!transition.is_null() && object->GetPrototype()->IsJSObject()) {
JSObject* holder;
- if (lookup.IsFound()) {
- holder = lookup.holder();
+ // holder == object indicates that no property was found.
+ if (lookup->holder() != *object) {
+ holder = lookup->holder();
} else {
// Find the top object.
holder = *object;
@@ -774,8 +765,19 @@
} while (holder->GetPrototype()->IsJSObject());
}
// We need an extra register, push
- CheckPrototypes(object, receiver_reg, Handle<JSObject>(holder),
name_reg,
- scratch1, scratch2, name, miss_restore_name);
+ Register holder_reg = CheckPrototypes(
+ object, receiver_reg, Handle<JSObject>(holder), name_reg,
+ scratch1, scratch2, name, miss_restore_name);
+ // If no property was found, and the holder (the last object in the
+ // prototype chain) is in slow mode, we need to do a negative lookup
on the
+ // holder.
+ if (lookup->holder() == *object &&
+ !holder->HasFastProperties() &&
+ !holder->IsJSGlobalProxy() &&
+ !holder->IsJSGlobalObject()) {
+ GenerateDictionaryNegativeLookup(
+ masm, miss_restore_name, holder_reg, name, scratch1, scratch2);
+ }
}
// Stub never generated for non-global objects that require access
@@ -799,6 +801,7 @@
return;
}
+ int index;
if (!transition.is_null()) {
// Update the map of the object.
__ mov(scratch1, Immediate(transition));
@@ -813,7 +816,12 @@
kDontSaveFPRegs,
OMIT_REMEMBERED_SET,
OMIT_SMI_CHECK);
+ index = transition->instance_descriptors()->GetFieldIndex(
+ transition->LastAdded());
+ } else {
+ index = lookup->GetFieldIndex().field_index();
}
+
// Adjust for the number of properties stored in the object. Even in the
// face of a transition we can use the old map here because the size of
the
=======================================
--- /branches/bleeding_edge/src/ic.cc Fri Mar 22 09:33:50 2013
+++ /branches/bleeding_edge/src/ic.cc Mon Mar 25 05:55:27 2013
@@ -1416,43 +1416,44 @@
}
return Handle<Code>::null();
}
-
-
-static bool StoreICableLookup(LookupResult* lookup) {
- // Bail out if we didn't find a result.
- if (!lookup->IsFound()) return false;
-
- // Bail out if inline caching is not allowed.
- if (!lookup->IsCacheable()) return false;
-
- // If the property is read-only, we leave the IC in its current state.
- if (lookup->IsTransition()) {
- return !lookup->GetTransitionDetails().IsReadOnly();
- }
- return !lookup->IsReadOnly();
-}
static bool LookupForWrite(Handle<JSObject> receiver,
Handle<String> name,
LookupResult* lookup) {
- receiver->LocalLookup(*name, lookup);
- if (!lookup->IsFound()) {
- receiver->map()->LookupTransition(*receiver, *name, lookup);
- }
- if (!StoreICableLookup(lookup)) {
- // 2nd chance: There can be accessors somewhere in the prototype chain.
- receiver->Lookup(*name, lookup);
- return lookup->IsPropertyCallbacks() && StoreICableLookup(lookup);
- }
+ Handle<JSObject> holder = receiver;
+ receiver->Lookup(*name, lookup);
+ if (lookup->IsFound()) {
+ if (lookup->IsReadOnly() || !lookup->IsCacheable()) return false;
- if (lookup->IsInterceptor() &&
- receiver->GetNamedInterceptor()->setter()->IsUndefined()) {
- receiver->LocalLookupRealNamedProperty(*name, lookup);
- return StoreICableLookup(lookup);
+ if (lookup->holder() == *receiver) {
+ if (lookup->IsInterceptor() &&
+ receiver->GetNamedInterceptor()->setter()->IsUndefined()) {
+ receiver->LocalLookupRealNamedProperty(*name, lookup);
+ return lookup->IsFound() &&
+ !lookup->IsReadOnly() &&
+ lookup->IsCacheable();
+ }
+ return true;
+ }
+
+ if (lookup->IsPropertyCallbacks()) return true;
+
+ // Currently normal holders in the prototype chain are not supported.
They
+ // would require a runtime positive lookup and verification that the
details
+ // have not changed.
+ if (lookup->IsInterceptor() || lookup->IsNormal()) return false;
+ holder = Handle<JSObject>(lookup->holder(), lookup->isolate());
}
- return true;
+ // While normally LookupTransition gets passed the receiver, in this
case we
+ // pass the holder of the property that we overwrite. This keeps the
holder in
+ // the LookupResult intact so we can later use it to generate a prototype
+ // chain check. This avoids a double lookup, but requires us to pass in
the
+ // receiver when trying to fetch extra information from the transition.
+ receiver->map()->LookupTransition(*holder, *name, lookup);
+ return lookup->IsTransition() &&
+ !lookup->GetTransitionDetails(receiver->map()).IsReadOnly();
}
@@ -1552,7 +1553,6 @@
Handle<String> name,
Handle<Object> value) {
ASSERT(!receiver->IsJSGlobalProxy());
- ASSERT(StoreICableLookup(lookup));
ASSERT(lookup->IsFound());
// These are not cacheable, so we never see such LookupResults here.
@@ -1575,8 +1575,7 @@
switch (lookup->type()) {
case FIELD:
return isolate()->stub_cache()->ComputeStoreField(
- name, receiver, lookup->GetFieldIndex().field_index(),
- Handle<Map>::null(), strict_mode);
+ name, receiver, lookup, Handle<Map>::null(), strict_mode);
case NORMAL:
if (receiver->IsGlobalObject()) {
// The stub generated for the global object picks the value
directly
@@ -1588,7 +1587,7 @@
return isolate()->stub_cache()->ComputeStoreGlobal(
name, global, cell, strict_mode);
}
- if (!holder.is_identical_to(receiver)) break;
+ ASSERT(holder.is_identical_to(receiver));
return isolate()->stub_cache()->ComputeStoreNormal(strict_mode);
case CALLBACKS: {
Handle<Object> callback(lookup->GetCallbackObject(), isolate());
@@ -1623,7 +1622,10 @@
case CONSTANT_FUNCTION:
break;
case TRANSITION: {
- Handle<Map> transition(lookup->GetTransitionTarget(), isolate());
+ // Explicitly pass in the receiver map since LookupForWrite may have
+ // stored something else than the receiver in the holder.
+ Handle<Map> transition(
+ lookup->GetTransitionTarget(receiver->map()), isolate());
int descriptor = transition->LastAdded();
DescriptorArray* target_descriptors =
transition->instance_descriptors();
@@ -1631,9 +1633,8 @@
if (details.type() != FIELD || details.attributes() != NONE) break;
- int field_index = target_descriptors->GetFieldIndex(descriptor);
return isolate()->stub_cache()->ComputeStoreField(
- name, receiver, field_index, transition, strict_mode);
+ name, receiver, lookup, transition, strict_mode);
}
case NONEXISTENT:
case HANDLER:
@@ -1964,19 +1965,20 @@
switch (lookup->type()) {
case FIELD:
return isolate()->stub_cache()->ComputeKeyedStoreField(
- name, receiver, lookup->GetFieldIndex().field_index(),
- Handle<Map>::null(), strict_mode);
+ name, receiver, lookup, Handle<Map>::null(), strict_mode);
case TRANSITION: {
- Handle<Map> transition(lookup->GetTransitionTarget(), isolate());
+ // Explicitly pass in the receiver map since LookupForWrite may have
+ // stored something else than the receiver in the holder.
+ Handle<Map> transition(
+ lookup->GetTransitionTarget(receiver->map()), isolate());
int descriptor = transition->LastAdded();
DescriptorArray* target_descriptors =
transition->instance_descriptors();
PropertyDetails details = target_descriptors->GetDetails(descriptor);
if (details.type() == FIELD && details.attributes() == NONE) {
- int field_index = target_descriptors->GetFieldIndex(descriptor);
return isolate()->stub_cache()->ComputeKeyedStoreField(
- name, receiver, field_index, transition, strict_mode);
+ name, receiver, lookup, transition, strict_mode);
}
// fall through.
}
=======================================
--- /branches/bleeding_edge/src/property.h Mon Mar 4 07:00:57 2013
+++ /branches/bleeding_edge/src/property.h Mon Mar 25 05:55:27 2013
@@ -362,11 +362,15 @@
return NULL;
}
- Map* GetTransitionTarget() {
+ Map* GetTransitionTarget(Map* map) {
ASSERT(IsTransition());
- TransitionArray* transitions = holder()->map()->transitions();
+ TransitionArray* transitions = map->transitions();
return transitions->GetTarget(number_);
}
+
+ Map* GetTransitionTarget() {
+ return GetTransitionTarget(holder()->map());
+ }
PropertyDetails GetTransitionDetails(Map* map) {
ASSERT(IsTransition());
=======================================
--- /branches/bleeding_edge/src/stub-cache.cc Wed Mar 20 03:37:13 2013
+++ /branches/bleeding_edge/src/stub-cache.cc Mon Mar 25 05:55:27 2013
@@ -397,7 +397,7 @@
Handle<Code> StubCache::ComputeStoreField(Handle<Name> name,
Handle<JSObject> receiver,
- int field_index,
+ LookupResult* lookup,
Handle<Map> transition,
StrictModeFlag strict_mode) {
Code::StubType type =
@@ -409,7 +409,7 @@
StoreStubCompiler compiler(isolate_, strict_mode);
Handle<Code> code =
- compiler.CompileStoreField(receiver, field_index, transition, name);
+ compiler.CompileStoreField(receiver, lookup, transition, name);
JSObject::UpdateMapCodeCache(receiver, name, code);
return code;
}
@@ -532,7 +532,7 @@
Handle<Code> StubCache::ComputeKeyedStoreField(Handle<Name> name,
Handle<JSObject> receiver,
- int field_index,
+ LookupResult* lookup,
Handle<Map> transition,
StrictModeFlag strict_mode)
{
Code::StubType type =
@@ -543,7 +543,7 @@
KeyedStoreStubCompiler compiler(isolate(), strict_mode, STANDARD_STORE);
Handle<Code> code =
- compiler.CompileStoreField(receiver, field_index, transition, name);
+ compiler.CompileStoreField(receiver, lookup, transition, name);
JSObject::UpdateMapCodeCache(receiver, name, code);
return code;
}
@@ -1584,7 +1584,7 @@
Handle<Code> BaseStoreStubCompiler::CompileStoreField(Handle<JSObject>
object,
- int index,
+ LookupResult* lookup,
Handle<Map>
transition,
Handle<Name> name) {
Label miss, miss_restore_name;
@@ -1594,7 +1594,7 @@
// Generate store field code.
GenerateStoreField(masm(),
object,
- index,
+ lookup,
transition,
name,
receiver(), this->name(), value(), scratch1(),
scratch2(),
=======================================
--- /branches/bleeding_edge/src/stub-cache.h Fri Mar 15 05:07:35 2013
+++ /branches/bleeding_edge/src/stub-cache.h Mon Mar 25 05:55:27 2013
@@ -161,7 +161,7 @@
Handle<Code> ComputeStoreField(Handle<Name> name,
Handle<JSObject> object,
- int field_index,
+ LookupResult* lookup,
Handle<Map> transition,
StrictModeFlag strict_mode);
@@ -192,7 +192,7 @@
Handle<Code> ComputeKeyedStoreField(Handle<Name> name,
Handle<JSObject> object,
- int field_index,
+ LookupResult* lookup,
Handle<Map> transition,
StrictModeFlag strict_mode);
@@ -511,7 +511,7 @@
void GenerateStoreField(MacroAssembler* masm,
Handle<JSObject> object,
- int index,
+ LookupResult* lookup,
Handle<Map> transition,
Handle<Name> name,
Register receiver_reg,
@@ -782,7 +782,7 @@
virtual ~BaseStoreStubCompiler() { }
Handle<Code> CompileStoreField(Handle<JSObject> object,
- int index,
+ LookupResult* lookup,
Handle<Map> transition,
Handle<Name> name);
=======================================
--- /branches/bleeding_edge/src/x64/stub-cache-x64.cc Fri Mar 22 09:33:50
2013
+++ /branches/bleeding_edge/src/x64/stub-cache-x64.cc Mon Mar 25 05:55:27
2013
@@ -716,7 +716,7 @@
// but may be destroyed if store is successful.
void StubCompiler::GenerateStoreField(MacroAssembler* masm,
Handle<JSObject> object,
- int index,
+ LookupResult* lookup,
Handle<Map> transition,
Handle<Name> name,
Register receiver_reg,
@@ -726,16 +726,6 @@
Register scratch2,
Label* miss_label,
Label* miss_restore_name) {
- LookupResult lookup(masm->isolate());
- object->Lookup(*name, &lookup);
- if (lookup.IsFound() && (lookup.IsReadOnly() || !lookup.IsCacheable())) {
- // In sloppy mode, we could just return the value and be done.
However, we
- // might be in strict mode, where we have to throw. Since we cannot
tell,
- // go into slow case unconditionally.
- __ jmp(miss_label);
- return;
- }
-
// Check that the map of the object hasn't changed.
CompareMapMode mode = transition.is_null() ?
ALLOW_ELEMENT_TRANSITION_MAPS
: REQUIRE_EXACT_MAP;
@@ -750,8 +740,9 @@
// Check that we are allowed to write this.
if (!transition.is_null() && object->GetPrototype()->IsJSObject()) {
JSObject* holder;
- if (lookup.IsFound()) {
- holder = lookup.holder();
+ // holder == object indicates that no property was found.
+ if (lookup->holder() != *object) {
+ holder = lookup->holder();
} else {
// Find the top object.
holder = *object;
@@ -759,8 +750,19 @@
holder = JSObject::cast(holder->GetPrototype());
} while (holder->GetPrototype()->IsJSObject());
}
- CheckPrototypes(object, receiver_reg, Handle<JSObject>(holder),
name_reg,
- scratch1, scratch2, name, miss_restore_name);
+ Register holder_reg = CheckPrototypes(
+ object, receiver_reg, Handle<JSObject>(holder), name_reg,
+ scratch1, scratch2, name, miss_restore_name);
+ // If no property was found, and the holder (the last object in the
+ // prototype chain) is in slow mode, we need to do a negative lookup
on the
+ // holder.
+ if (lookup->holder() == *object &&
+ !holder->HasFastProperties() &&
+ !holder->IsJSGlobalProxy() &&
+ !holder->IsJSGlobalObject()) {
+ GenerateDictionaryNegativeLookup(
+ masm, miss_restore_name, holder_reg, name, scratch1, scratch2);
+ }
}
// Stub never generated for non-global objects that require access
@@ -784,6 +786,7 @@
return;
}
+ int index;
if (!transition.is_null()) {
// Update the map of the object.
__ Move(scratch1, transition);
@@ -798,6 +801,10 @@
kDontSaveFPRegs,
OMIT_REMEMBERED_SET,
OMIT_SMI_CHECK);
+ index = transition->instance_descriptors()->GetFieldIndex(
+ transition->LastAdded());
+ } else {
+ index = lookup->GetFieldIndex().field_index();
}
// Adjust for the number of properties stored in the object. Even in the
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
