From 74ddab9d94ba7d630c0e9d4dcb67902262537a0c Mon Sep 17 00:00:00 2001 From: "whesse@chromium.org" Date: Mon, 22 Jun 2009 07:41:15 +0000 Subject: [PATCH] Fix issue 386, a bug in JSObject::ReplaceSlowProperty with constant transitions. Review URL: http://codereview.chromium.org/141031 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2228 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/objects.cc | 17 ++++++++------ src/objects.h | 1 + test/mjsunit/regress/regress-386.js | 47 +++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 test/mjsunit/regress/regress-386.js diff --git a/src/objects.cc b/src/objects.cc index c69dc37..0457269 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1302,16 +1302,19 @@ Object* JSObject::ReplaceSlowProperty(String* name, Object* value, PropertyAttributes attributes) { Dictionary* dictionary = property_dictionary(); - PropertyDetails old_details = - dictionary->DetailsAt(dictionary->FindStringEntry(name)); - int new_index = old_details.index(); - if (old_details.IsTransition()) new_index = 0; + int old_index = dictionary->FindStringEntry(name); + int new_enumeration_index = 0; // 0 means "Use the next available index." + if (old_index != -1) { + // All calls to ReplaceSlowProperty have had all transitions removed. + ASSERT(!dictionary->DetailsAt(old_index).IsTransition()); + new_enumeration_index = dictionary->DetailsAt(old_index).index(); + } - PropertyDetails new_details(attributes, NORMAL, old_details.index()); + PropertyDetails new_details(attributes, NORMAL, new_enumeration_index); Object* result = - property_dictionary()->SetOrAddStringEntry(name, value, new_details); + dictionary->SetOrAddStringEntry(name, value, new_details); if (result->IsFailure()) return result; - if (property_dictionary() != result) { + if (dictionary != result) { set_properties(Dictionary::cast(result)); } return value; diff --git a/src/objects.h b/src/objects.h index 8a0099f..0aaf51c 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2033,6 +2033,7 @@ class Dictionary: public DictionaryBase { // Returns the property details for the property at entry. PropertyDetails DetailsAt(int entry) { + ASSERT(entry >= 0); // Not found is -1, which is not caught by get(). return PropertyDetails(Smi::cast(get(EntryToIndex(entry) + 2))); } diff --git a/test/mjsunit/regress/regress-386.js b/test/mjsunit/regress/regress-386.js new file mode 100644 index 0000000..06e4b8e --- /dev/null +++ b/test/mjsunit/regress/regress-386.js @@ -0,0 +1,47 @@ +// 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. + + +// Test for http://code.google.com/p/v8/issues/detail?id=386 +// This test creates enough properties in A so that adding i as +// a constant function, in the first call to the constructor, leaves +// the object's map in the fast case and adds a constant function map +// transition. +// Adding i in the second call to the constructor creates a real property, +// and simultaneously converts the object from fast case to slow case +// and changes i from a map transition to a real property. There was +// a flaw in the code that handled this combination of events. + +function A() { + for (var i = 0; i < 13; i++) { + this['a' + i] = i; + } + this.i = function(){}; +}; + +new A(); +new A(); -- 2.7.4