diff --git a/src/accessors.cc b/src/accessors.cc index 303aa43cac..d779eb26ae 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -509,17 +509,13 @@ Object* Accessors::ObjectSetPrototype(JSObject* receiver, // SpiderMonkey behaves this way. if (!value->IsJSObject() && !value->IsNull()) return value; - bool clear_ics = false; for (Object* pt = value; pt != Heap::null_value(); pt = pt->GetPrototype()) { - JSObject *obj = JSObject::cast(pt); - if (obj == receiver) { + if (JSObject::cast(pt) == receiver) { // Cycle detected. HandleScope scope; return Top::Throw(*Factory::NewError("cyclic_proto", HandleVector(NULL, 0))); } - if (obj->HasLocalPropertyWithType(CALLBACKS)) - clear_ics = true; } // Find the first object in the chain whose prototype object is not @@ -538,10 +534,6 @@ Object* Accessors::ObjectSetPrototype(JSObject* receiver, Map::cast(new_map)->set_prototype(value); current->set_map(Map::cast(new_map)); - // Finally, if the prototype contains a setter we may have broken - // the assumptions made when creating ics so we have to clear them. - if (clear_ics) Heap::ClearStoreICs(); - // To be consistent with other Set functions, return the value. return value; } diff --git a/src/heap.cc b/src/heap.cc index b9ba68d709..e8950ee6a4 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -71,8 +71,6 @@ int Heap::old_gen_allocation_limit_ = kMinimumAllocationLimit; int Heap::old_gen_exhausted_ = false; -bool Heap::has_store_ics_ = false; - int Heap::amount_of_external_allocated_memory_ = 0; int Heap::amount_of_external_allocated_memory_at_last_global_gc_ = 0; @@ -294,14 +292,6 @@ void Heap::CollectAllGarbage() { } -void Heap::ClearStoreICs() { - if (has_store_ics_) { - Counters::clear_store_ic.Increment(); - CollectAllGarbage(); - } -} - - void Heap::CollectAllGarbageIfContextDisposed() { // If the garbage collector interface is exposed through the global // gc() function, we avoid being clever about forcing GCs when @@ -482,7 +472,6 @@ void Heap::MarkCompactPrologue(bool is_compacting) { void Heap::MarkCompactEpilogue(bool is_compacting) { Top::MarkCompactEpilogue(is_compacting); ThreadManager::MarkCompactEpilogue(is_compacting); - Heap::has_store_ics_ = false; } diff --git a/src/heap.h b/src/heap.h index a13f027ec3..6979f43f29 100644 --- a/src/heap.h +++ b/src/heap.h @@ -606,9 +606,6 @@ class Heap : public AllStatic { // Performs a full garbage collection. static void CollectAllGarbage(); - // Clears all inline caches by forcing a global garbage collection. - static void ClearStoreICs(); - // Performs a full garbage collection if a context has been disposed // since the last time the check was performed. static void CollectAllGarbageIfContextDisposed(); @@ -811,14 +808,6 @@ class Heap : public AllStatic { > old_gen_allocation_limit_; } - static bool has_store_ics() { - return has_store_ics_; - } - - static void store_ic_created() { - has_store_ics_ = true; - } - private: static int semispace_size_; static int initial_semispace_size_; @@ -884,8 +873,6 @@ class Heap : public AllStatic { // last GC. static int old_gen_exhausted_; - static bool has_store_ics_; - // Declare all the roots #define ROOT_DECLARATION(type, name) static type* name##_; ROOT_LIST(ROOT_DECLARATION) diff --git a/src/ic.cc b/src/ic.cc index 048fe3c3d6..d7bd764404 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -912,7 +912,6 @@ void StoreIC::UpdateCaches(LookupResult* lookup, // Patch the call site depending on the state of the cache. if (state == UNINITIALIZED || state == MONOMORPHIC_PROTOTYPE_FAILURE) { - Heap::store_ic_created(); set_target(Code::cast(code)); } else if (state == MONOMORPHIC) { // Only move to mega morphic if the target changes. diff --git a/src/messages.js b/src/messages.js index f8b3c512d7..cd9a1e8d61 100644 --- a/src/messages.js +++ b/src/messages.js @@ -602,7 +602,7 @@ var kAddMessageAccessorsMarker = { }; // Defines accessors for a property that is calculated the first time // the property is read and then replaces the accessor with the value. // Also, setting the property causes the accessors to be deleted. -function DefineOneShotAccessor(obj, name, fun, never_used) { +function DefineOneShotAccessor(obj, name, fun) { // Note that the accessors consistently operate on 'obj', not 'this'. // Since the object may occur in someone else's prototype chain we // can't rely on 'this' being the same as 'obj'. @@ -611,10 +611,10 @@ function DefineOneShotAccessor(obj, name, fun, never_used) { obj[name] = value; return value; }); - %DefineAccessor(ToObject(obj), ToString(name), SETTER, function (v) { + obj.__defineSetter__(name, function (v) { delete obj[name]; obj[name] = v; - }, 0, never_used); + }); } function DefineError(f) { @@ -648,7 +648,7 @@ function DefineError(f) { if (m === kAddMessageAccessorsMarker) { DefineOneShotAccessor(this, 'message', function (obj) { return FormatMessage({type: obj.type, args: obj.arguments}); - }, true); + }); } else if (!IS_UNDEFINED(m)) { this.message = ToString(m); } diff --git a/src/objects.cc b/src/objects.cc index 3d22f40089..565a50cc2b 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -2483,38 +2483,6 @@ void JSObject::LocalLookup(String* name, LookupResult* result) { } -bool JSObject::HasLocalPropertyWithType(PropertyType type) { - if (IsJSGlobalProxy()) { - Object* proto = GetPrototype(); - if (proto->IsNull()) return false; - ASSERT(proto->IsJSGlobalObject()); - return JSObject::cast(proto)->HasLocalPropertyWithType(type); - } - - if (HasFastProperties()) { - DescriptorArray* descriptors = map()->instance_descriptors(); - int length = descriptors->number_of_descriptors(); - for (int i = 0; i < length; i++) { - PropertyDetails details(descriptors->GetDetails(i)); - if (details.type() == type) - return true; - } - } else { - Handle properties = Handle(property_dictionary()); - int capacity = properties->Capacity(); - for (int i = 0; i < capacity; i++) { - if (properties->IsKey(properties->KeyAt(i))) { - PropertyDetails details = properties->DetailsAt(i); - if (details.type() == type) - return true; - } - } - } - - return false; -} - - void JSObject::Lookup(String* name, LookupResult* result) { // Ecma-262 3rd 8.6.2.4 for (Object* current = this; @@ -2642,11 +2610,8 @@ Object* JSObject::DefineGetterSetter(String* name, } -Object* JSObject::DefineAccessor(String* name, - bool is_getter, - JSFunction* fun, - PropertyAttributes attributes, - bool never_used) { +Object* JSObject::DefineAccessor(String* name, bool is_getter, JSFunction* fun, + PropertyAttributes attributes) { // Check access rights if needed. if (IsAccessCheckNeeded() && !Top::MayNamedAccess(this, name, v8::ACCESS_HAS)) { @@ -2658,22 +2623,13 @@ Object* JSObject::DefineAccessor(String* name, Object* proto = GetPrototype(); if (proto->IsNull()) return this; ASSERT(proto->IsJSGlobalObject()); - return JSObject::cast(proto)->DefineAccessor(name, - is_getter, - fun, - attributes, - never_used); + return JSObject::cast(proto)->DefineAccessor(name, is_getter, + fun, attributes); } Object* array = DefineGetterSetter(name, attributes); if (array->IsFailure() || array->IsUndefined()) return array; FixedArray::cast(array)->set(is_getter ? 0 : 1, fun); - - // Because setters are nonlocal (they're accessible through the - // prototype chain) but our inline caches are local we clear them - // when a new setter is introduced. - if (!(is_getter || never_used)) Heap::ClearStoreICs(); - return this; } diff --git a/src/objects.h b/src/objects.h index d43d616eca..165d350d3a 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1200,13 +1200,8 @@ class JSObject: public HeapObject { String* name); PropertyAttributes GetLocalPropertyAttribute(String* name); - // Defines an accessor. This may violate some of the assumptions we - // make when setting up ics so unless it is guaranteed that no ics - // exist for this property we have to clear all ics. Set the 'never_used' - // flag to true if you know there can be no ics. Object* DefineAccessor(String* name, bool is_getter, JSFunction* fun, - PropertyAttributes attributes, bool never_used); - + PropertyAttributes attributes); Object* LookupAccessor(String* name, bool is_getter); // Used from Object::GetProperty(). @@ -1278,8 +1273,6 @@ class JSObject: public HeapObject { inline bool HasNamedInterceptor(); inline bool HasIndexedInterceptor(); - bool HasLocalPropertyWithType(PropertyType type); - // Support functions for v8 api (needed for correct interceptor behavior). bool HasRealNamedProperty(String* key); bool HasRealElementProperty(uint32_t index); diff --git a/src/regexp-delay.js b/src/regexp-delay.js index 3362e88412..84918637eb 100644 --- a/src/regexp-delay.js +++ b/src/regexp-delay.js @@ -356,15 +356,12 @@ function SetupRegExp() { LAST_INPUT(lastMatchInfo) = ToString(string); }; - // All these accessors are set with the 'never_used' flag set to true. - // This is because at this point there can be no existing ics for setting - // these properties and so we don't have to bother clearing them. %DefineAccessor($RegExp, 'input', GETTER, RegExpGetInput, DONT_DELETE); - %DefineAccessor($RegExp, 'input', SETTER, RegExpSetInput, DONT_DELETE, true); + %DefineAccessor($RegExp, 'input', SETTER, RegExpSetInput, DONT_DELETE); %DefineAccessor($RegExp, '$_', GETTER, RegExpGetInput, DONT_ENUM | DONT_DELETE); - %DefineAccessor($RegExp, '$_', SETTER, RegExpSetInput, DONT_ENUM | DONT_DELETE, true); + %DefineAccessor($RegExp, '$_', SETTER, RegExpSetInput, DONT_ENUM | DONT_DELETE); %DefineAccessor($RegExp, '$input', GETTER, RegExpGetInput, DONT_ENUM | DONT_DELETE); - %DefineAccessor($RegExp, '$input', SETTER, RegExpSetInput, DONT_ENUM | DONT_DELETE, true); + %DefineAccessor($RegExp, '$input', SETTER, RegExpSetInput, DONT_ENUM | DONT_DELETE); // The properties multiline and $* are aliases for each other. When this // value is set in SpiderMonkey, the value it is set to is coerced to a @@ -379,9 +376,9 @@ function SetupRegExp() { function RegExpSetMultiline(flag) { multiline = flag ? true : false; }; %DefineAccessor($RegExp, 'multiline', GETTER, RegExpGetMultiline, DONT_DELETE); - %DefineAccessor($RegExp, 'multiline', SETTER, RegExpSetMultiline, DONT_DELETE, true); + %DefineAccessor($RegExp, 'multiline', SETTER, RegExpSetMultiline, DONT_DELETE); %DefineAccessor($RegExp, '$*', GETTER, RegExpGetMultiline, DONT_ENUM | DONT_DELETE); - %DefineAccessor($RegExp, '$*', SETTER, RegExpSetMultiline, DONT_ENUM | DONT_DELETE, true); + %DefineAccessor($RegExp, '$*', SETTER, RegExpSetMultiline, DONT_ENUM | DONT_DELETE); function NoOpSetter(ignored) {} @@ -389,25 +386,25 @@ function SetupRegExp() { // Static properties set by a successful match. %DefineAccessor($RegExp, 'lastMatch', GETTER, RegExpGetLastMatch, DONT_DELETE); - %DefineAccessor($RegExp, 'lastMatch', SETTER, NoOpSetter, DONT_DELETE, true); + %DefineAccessor($RegExp, 'lastMatch', SETTER, NoOpSetter, DONT_DELETE); %DefineAccessor($RegExp, '$&', GETTER, RegExpGetLastMatch, DONT_ENUM | DONT_DELETE); - %DefineAccessor($RegExp, '$&', SETTER, NoOpSetter, DONT_ENUM | DONT_DELETE, true); + %DefineAccessor($RegExp, '$&', SETTER, NoOpSetter, DONT_ENUM | DONT_DELETE); %DefineAccessor($RegExp, 'lastParen', GETTER, RegExpGetLastParen, DONT_DELETE); - %DefineAccessor($RegExp, 'lastParen', SETTER, NoOpSetter, DONT_DELETE, true); + %DefineAccessor($RegExp, 'lastParen', SETTER, NoOpSetter, DONT_DELETE); %DefineAccessor($RegExp, '$+', GETTER, RegExpGetLastParen, DONT_ENUM | DONT_DELETE); - %DefineAccessor($RegExp, '$+', SETTER, NoOpSetter, DONT_ENUM | DONT_DELETE, true); + %DefineAccessor($RegExp, '$+', SETTER, NoOpSetter, DONT_ENUM | DONT_DELETE); %DefineAccessor($RegExp, 'leftContext', GETTER, RegExpGetLeftContext, DONT_DELETE); - %DefineAccessor($RegExp, 'leftContext', SETTER, NoOpSetter, DONT_DELETE, true); + %DefineAccessor($RegExp, 'leftContext', SETTER, NoOpSetter, DONT_DELETE); %DefineAccessor($RegExp, '$`', GETTER, RegExpGetLeftContext, DONT_ENUM | DONT_DELETE); - %DefineAccessor($RegExp, '$`', SETTER, NoOpSetter, DONT_ENUM | DONT_DELETE, true); + %DefineAccessor($RegExp, '$`', SETTER, NoOpSetter, DONT_ENUM | DONT_DELETE); %DefineAccessor($RegExp, 'rightContext', GETTER, RegExpGetRightContext, DONT_DELETE); - %DefineAccessor($RegExp, 'rightContext', SETTER, NoOpSetter, DONT_DELETE, true); + %DefineAccessor($RegExp, 'rightContext', SETTER, NoOpSetter, DONT_DELETE); %DefineAccessor($RegExp, "$'", GETTER, RegExpGetRightContext, DONT_ENUM | DONT_DELETE); - %DefineAccessor($RegExp, "$'", SETTER, NoOpSetter, DONT_ENUM | DONT_DELETE, true); + %DefineAccessor($RegExp, "$'", SETTER, NoOpSetter, DONT_ENUM | DONT_DELETE); for (var i = 1; i < 10; ++i) { %DefineAccessor($RegExp, '$' + i, GETTER, RegExpMakeCaptureGetter(i), DONT_DELETE); - %DefineAccessor($RegExp, '$' + i, SETTER, NoOpSetter, DONT_DELETE, true); + %DefineAccessor($RegExp, '$' + i, SETTER, NoOpSetter, DONT_DELETE); } } diff --git a/src/runtime.cc b/src/runtime.cc index 06ef46acdb..3cf2439ba1 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -5074,10 +5074,10 @@ static Object* Runtime_GetArrayKeys(Arguments args) { // and setter on the first call to DefineAccessor and ignored on // subsequent calls. static Object* Runtime_DefineAccessor(Arguments args) { - RUNTIME_ASSERT(4 <= args.length() && args.length() <= 6); + RUNTIME_ASSERT(args.length() == 4 || args.length() == 5); // Compute attributes. PropertyAttributes attributes = NONE; - if (args.length() >= 5) { + if (args.length() == 5) { CONVERT_CHECKED(Smi, attrs, args[4]); int value = attrs->value(); // Only attribute bits should be set. @@ -5085,17 +5085,11 @@ static Object* Runtime_DefineAccessor(Arguments args) { attributes = static_cast(value); } - bool never_used = (args.length() == 6) && (args[5] == Heap::true_value()); - CONVERT_CHECKED(JSObject, obj, args[0]); CONVERT_CHECKED(String, name, args[1]); CONVERT_CHECKED(Smi, flag, args[2]); CONVERT_CHECKED(JSFunction, fun, args[3]); - return obj->DefineAccessor(name, - flag->value() == 0, - fun, - attributes, - never_used); + return obj->DefineAccessor(name, flag->value() == 0, fun, attributes); } diff --git a/src/v8-counters.h b/src/v8-counters.h index 20e4c163da..6596dbf470 100644 --- a/src/v8-counters.h +++ b/src/v8-counters.h @@ -122,8 +122,7 @@ namespace v8 { namespace internal { SC(enum_cache_misses, V8.EnumCacheMisses) \ SC(reloc_info_count, V8.RelocInfoCount) \ SC(reloc_info_size, V8.RelocInfoSize) \ - SC(zone_segment_bytes, V8.ZoneSegmentBytes) \ - SC(clear_store_ic, V8.ClearStoreIC) + SC(zone_segment_bytes, V8.ZoneSegmentBytes) // This file contains all the v8 counters that are in use. diff --git a/test/mjsunit/regress/regress-1344252.js b/test/mjsunit/bugs/bug-1344252.js similarity index 98% rename from test/mjsunit/regress/regress-1344252.js rename to test/mjsunit/bugs/bug-1344252.js index 1686f80ae5..1723834c09 100644 --- a/test/mjsunit/regress/regress-1344252.js +++ b/test/mjsunit/bugs/bug-1344252.js @@ -52,12 +52,14 @@ assertTrue(typeof f.x == 'undefined'); var result_y; var proto = new Object(); proto.__defineSetter__('y', function (value) { result_y = value; }); -var f = { }; +var f = new F(); +f.y = undefined; f.__proto__ = proto; F.call(f); assertEquals(87, result_y); assertTrue(typeof f.y == 'undefined'); + // Test the same issue in the runtime system. Make sure that // accessors added to the prototype chain are called instead of // following map transitions. @@ -74,3 +76,4 @@ Object.prototype.__defineSetter__('z', function(value) { result_z = value; }); o2.z = 27; assertEquals(27, result_z); assertTrue(typeof o2.z == 'undefined'); + diff --git a/test/mjsunit/regress/regress-92.js b/test/mjsunit/regress/regress-92.js deleted file mode 100644 index a774139de8..0000000000 --- a/test/mjsunit/regress/regress-92.js +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2009 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 introduceSetter(useProto, Constructor) { - // Before introducing the setter this test expects 'y' to be set - // normally. Afterwards setting 'y' will throw an exception. - var runTest = new Function("Constructor", - "var p = new Constructor(3); p.y = 4; assertEquals(p.y, 4);"); - - // Create the prototype object first because defining a setter should - // clear inline caches. - if (useProto) { - var newProto = { }; - newProto.__defineSetter__('y', function () { throw signal; }); - } - - // Ensure that monomorphic ics have been set up. - runTest(Constructor); - runTest(Constructor); - - var signal = "was called"; - if (useProto) { - // Either introduce the setter through __proto__... - Constructor.prototype.__proto__ = newProto; - } else { - // ...or introduce it directly using __defineSetter__. - Constructor.prototype.__defineSetter__('y', function () { throw signal; }); - } - - // Now setting 'y' should throw an exception. - try { - runTest(Constructor); - fail("Accessor was not called."); - } catch (e) { - assertEquals(e, signal); - } - -} - -introduceSetter(false, function FastCase(x) { this.x = x; }); -introduceSetter(true, function FastCase(x) { this.x = x; }); -introduceSetter(false, function SlowCase(x) { this.x = x; delete this.x; }); -introduceSetter(true, function SlowCase(x) { this.x = x; delete this.x; });