From 9aa9458933b38810c7c500dd75b67556f1faa2ee Mon Sep 17 00:00:00 2001 From: "kasperl@chromium.org" Date: Wed, 8 Jul 2009 11:32:03 +0000 Subject: [PATCH] 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. Review URL: http://codereview.chromium.org/149322 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2390 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ia32/stub-cache-ia32.cc | 3 +++ src/objects.cc | 13 ++++++++++--- test/cctest/test-api.cc | 22 ++++++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index ce4981d72e..2ee826e5d5 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -1149,6 +1149,9 @@ Object* LoadStubCompiler::CompileLoadGlobal(GlobalObject* object, 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); diff --git a/src/objects.cc b/src/objects.cc index 5c477a46f2..ff026b7c68 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -467,8 +467,15 @@ Object* JSObject::DeleteNormalizedProperty(String* name, DeleteMode mode) { // 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 @@ Object* JSObject::IgnoreAttributesAndSetLocalProperty( 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); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 6839f2d524..5b04b2cd8e 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -6962,6 +6962,28 @@ THREADED_TEST(ForceDeleteWithInterceptor) { } +// 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 calling_context0; v8::Persistent calling_context1; v8::Persistent calling_context2;