Reviewers: bak, Description: Fix crash that occurs when we're forced to delete a global property that used to be DontDelete and we still have an IC that reads from the cell.
Please review this at http://codereview.chromium.org/149322 SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/ia32/stub-cache-ia32.cc M src/objects.cc M test/cctest/test-api.cc Index: test/cctest/test-api.cc =================================================================== --- test/cctest/test-api.cc (revision 2386) +++ test/cctest/test-api.cc (working copy) @@ -6962,6 +6962,28 @@ } +// Make sure that forcing a delete invalidates any IC stubs, so we +// don't read the hole value. +THREADED_TEST(ForceDeleteIC) { + v8::HandleScope scope; + LocalContext context; + // Create a DontDelete variable on the global object. + CompileRun("this.__proto__ = { foo: 'horse' };" + "var foo = 'fish';" + "function f() { return foo.length; }"); + // Initialize the IC for foo in f. + CompileRun("for (var i = 0; i < 4; i++) f();"); + // Make sure the value of foo is correct before the deletion. + CHECK_EQ(4, CompileRun("f()")->Int32Value()); + // Force the deletion of foo. + CHECK(context->Global()->ForceDelete(v8_str("foo"))); + // Make sure the value for foo is read from the prototype, and that + // we don't get in trouble with reading the deleted cell value + // sentinel. + CHECK_EQ(5, CompileRun("f()")->Int32Value()); +} + + v8::Persistent<Context> calling_context0; v8::Persistent<Context> calling_context1; v8::Persistent<Context> calling_context2; Index: src/objects.cc =================================================================== --- src/objects.cc (revision 2386) +++ src/objects.cc (working copy) @@ -467,8 +467,15 @@ // If we have a global object set the cell to the hole. if (IsGlobalObject()) { PropertyDetails details = dictionary->DetailsAt(entry); - if (details.IsDontDelete() && mode != FORCE_DELETION) { - return Heap::false_value(); + if (details.IsDontDelete()) { + if (mode != FORCE_DELETION) return Heap::false_value(); + // When forced to delete global properties, we have to make a + // map change to invalidate any ICs that think they can load + // from the DontDelete cell without checking if it contains + // the hole value. + Object* new_map = map()->CopyDropDescriptors(); + if (new_map->IsFailure()) return new_map; + set_map(Map::cast(new_map)); } JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(dictionary->ValueAt(entry)); @@ -1932,7 +1939,7 @@ if (!result->IsLoaded()) { return SetLazyProperty(result, name, value, attributes); } - // Check of IsReadOnly removed from here in clone. + // Check of IsReadOnly removed from here in clone. switch (result->type()) { case NORMAL: return SetNormalizedProperty(result, value); Index: src/ia32/stub-cache-ia32.cc =================================================================== --- src/ia32/stub-cache-ia32.cc (revision 2386) +++ src/ia32/stub-cache-ia32.cc (working copy) @@ -1149,6 +1149,9 @@ if (!is_dont_delete) { __ cmp(eax, Factory::the_hole_value()); __ j(equal, &miss, not_taken); + } else if (FLAG_debug_code) { + __ cmp(eax, Factory::the_hole_value()); + __ Check(not_equal, "DontDelete cells can't contain the hole"); } __ ret(0); --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
