From 2ebfd6e90e7f45171e90afd54c38b103ad21c77e Mon Sep 17 00:00:00 2001 From: "jkummerow@chromium.org" Date: Mon, 4 Nov 2013 14:14:09 +0000 Subject: [PATCH] Add missing negative dictionary lookup to NonexistentHandlerFrontend BUG=v8:2980 R=verwaest@chromium.org Review URL: https://codereview.chromium.org/57433003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@17459 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/stub-cache-arm.cc | 62 ++++++++------------------- src/ia32/stub-cache-ia32.cc | 72 ++++++++++---------------------- src/mips/stub-cache-mips.cc | 65 ++++++++--------------------- src/stub-cache.cc | 34 +++++++++++++++ src/stub-cache.h | 30 +++++++++++++ src/x64/stub-cache-x64.cc | 81 ++++++++++++------------------------ test/mjsunit/regress/regress-2980.js | 64 ++++++++++++++++++++++++++++ 7 files changed, 213 insertions(+), 195 deletions(-) create mode 100644 test/mjsunit/regress/regress-2980.js diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index 004e067..afb65a0 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -121,18 +121,14 @@ static void ProbeTable(Isolate* isolate, } -// Helper function used to check that the dictionary doesn't contain -// the property. This function may return false negatives, so miss_label -// must always call a backup property check that is complete. -// This function is safe to call if the receiver has fast properties. -// Name must be unique and receiver must be a heap object. -static void GenerateDictionaryNegativeLookup(MacroAssembler* masm, - Label* miss_label, - Register receiver, - Handle name, - Register scratch0, - Register scratch1) { +void StubCompiler::GenerateDictionaryNegativeLookup(MacroAssembler* masm, + Label* miss_label, + Register receiver, + Handle name, + Register scratch0, + Register scratch1) { ASSERT(name->IsUniqueName()); + ASSERT(!receiver.is(scratch0)); Counters* counters = masm->isolate()->counters(); __ IncrementCounter(counters->negative_lookups(), 1, scratch0, scratch1); __ IncrementCounter(counters->negative_lookups_miss(), 1, scratch0, scratch1); @@ -418,11 +414,11 @@ void StubCompiler::GenerateLoadFunctionPrototype(MacroAssembler* masm, // Generate code to check that a global property cell is empty. Create // the property cell at compilation time if no cell exists for the // property. -static void GenerateCheckPropertyCell(MacroAssembler* masm, - Handle global, - Handle name, - Register scratch, - Label* miss) { +void StubCompiler::GenerateCheckPropertyCell(MacroAssembler* masm, + Handle global, + Handle name, + Register scratch, + Label* miss) { Handle cell = GlobalObject::EnsurePropertyCell(global, name); ASSERT(cell->value()->IsTheHole()); __ mov(scratch, Operand(cell)); @@ -1156,14 +1152,12 @@ class CallInterceptorCompiler BASE_EMBEDDED { }; -// Calls GenerateCheckPropertyCell for each global object in the prototype chain -// from object to (but not including) holder. -static void GenerateCheckPropertyCells(MacroAssembler* masm, - Handle object, - Handle holder, - Handle name, - Register scratch, - Label* miss) { +void StubCompiler::GenerateCheckPropertyCells(MacroAssembler* masm, + Handle object, + Handle holder, + Handle name, + Register scratch, + Label* miss) { Handle current = object; while (!current.is_identical_to(holder)) { if (current->IsGlobalObject()) { @@ -1373,26 +1367,6 @@ Register LoadStubCompiler::CallbackHandlerFrontend( } -void LoadStubCompiler::NonexistentHandlerFrontend( - Handle object, - Handle last, - Handle name, - Label* success, - Handle global) { - Label miss; - - HandlerFrontendHeader(object, receiver(), last, name, &miss); - - // If the last object in the prototype chain is a global object, - // check that the global property cell is empty. - if (!global.is_null()) { - GenerateCheckPropertyCell(masm(), global, name, scratch2(), &miss); - } - - HandlerFrontendFooter(name, success, &miss); -} - - void LoadStubCompiler::GenerateLoadField(Register reg, Handle holder, PropertyIndex field, diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index 9786cff..fda9c20 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -137,38 +137,34 @@ static void ProbeTable(Isolate* isolate, } -// Helper function used to check that the dictionary doesn't contain -// the property. This function may return false negatives, so miss_label -// must always call a backup property check that is complete. -// This function is safe to call if the receiver has fast properties. -// Name must be unique and receiver must be a heap object. -static void GenerateDictionaryNegativeLookup(MacroAssembler* masm, - Label* miss_label, - Register receiver, - Handle name, - Register r0, - Register r1) { +void StubCompiler::GenerateDictionaryNegativeLookup(MacroAssembler* masm, + Label* miss_label, + Register receiver, + Handle name, + Register scratch0, + Register scratch1) { ASSERT(name->IsUniqueName()); + ASSERT(!receiver.is(scratch0)); Counters* counters = masm->isolate()->counters(); __ IncrementCounter(counters->negative_lookups(), 1); __ IncrementCounter(counters->negative_lookups_miss(), 1); - __ mov(r0, FieldOperand(receiver, HeapObject::kMapOffset)); + __ mov(scratch0, FieldOperand(receiver, HeapObject::kMapOffset)); const int kInterceptorOrAccessCheckNeededMask = (1 << Map::kHasNamedInterceptor) | (1 << Map::kIsAccessCheckNeeded); // Bail out if the receiver has a named interceptor or requires access checks. - __ test_b(FieldOperand(r0, Map::kBitFieldOffset), + __ test_b(FieldOperand(scratch0, Map::kBitFieldOffset), kInterceptorOrAccessCheckNeededMask); __ j(not_zero, miss_label); // Check that receiver is a JSObject. - __ CmpInstanceType(r0, FIRST_SPEC_OBJECT_TYPE); + __ CmpInstanceType(scratch0, FIRST_SPEC_OBJECT_TYPE); __ j(below, miss_label); // Load properties array. - Register properties = r0; + Register properties = scratch0; __ mov(properties, FieldOperand(receiver, JSObject::kPropertiesOffset)); // Check that the properties array is a dictionary. @@ -182,7 +178,7 @@ static void GenerateDictionaryNegativeLookup(MacroAssembler* masm, &done, properties, name, - r1); + scratch1); __ bind(&done); __ DecrementCounter(counters->negative_lookups_miss(), 1); } @@ -792,11 +788,11 @@ void StoreStubCompiler::GenerateRestoreName(MacroAssembler* masm, // Generate code to check that a global property cell is empty. Create // the property cell at compilation time if no cell exists for the // property. -static void GenerateCheckPropertyCell(MacroAssembler* masm, - Handle global, - Handle name, - Register scratch, - Label* miss) { +void StubCompiler::GenerateCheckPropertyCell(MacroAssembler* masm, + Handle global, + Handle name, + Register scratch, + Label* miss) { Handle cell = GlobalObject::EnsurePropertyCell(global, name); ASSERT(cell->value()->IsTheHole()); @@ -1122,14 +1118,12 @@ void StoreStubCompiler::GenerateStoreField(MacroAssembler* masm, } -// Calls GenerateCheckPropertyCell for each global object in the prototype chain -// from object to (but not including) holder. -static void GenerateCheckPropertyCells(MacroAssembler* masm, - Handle object, - Handle holder, - Handle name, - Register scratch, - Label* miss) { +void StubCompiler::GenerateCheckPropertyCells(MacroAssembler* masm, + Handle object, + Handle holder, + Handle name, + Register scratch, + Label* miss) { Handle current = object; while (!current.is_identical_to(holder)) { if (current->IsGlobalObject()) { @@ -1355,26 +1349,6 @@ Register LoadStubCompiler::CallbackHandlerFrontend( } -void LoadStubCompiler::NonexistentHandlerFrontend( - Handle object, - Handle last, - Handle name, - Label* success, - Handle global) { - Label miss; - - HandlerFrontendHeader(object, receiver(), last, name, &miss); - - // If the last object in the prototype chain is a global object, - // check that the global property cell is empty. - if (!global.is_null()) { - GenerateCheckPropertyCell(masm(), global, name, scratch2(), &miss); - } - - HandlerFrontendFooter(name, success, &miss); -} - - void LoadStubCompiler::GenerateLoadField(Register reg, Handle holder, PropertyIndex field, diff --git a/src/mips/stub-cache-mips.cc b/src/mips/stub-cache-mips.cc index 471c25e..d81bd75 100644 --- a/src/mips/stub-cache-mips.cc +++ b/src/mips/stub-cache-mips.cc @@ -117,18 +117,14 @@ static void ProbeTable(Isolate* isolate, } -// Helper function used to check that the dictionary doesn't contain -// the property. This function may return false negatives, so miss_label -// must always call a backup property check that is complete. -// This function is safe to call if the receiver has fast properties. -// Name must be unique and receiver must be a heap object. -static void GenerateDictionaryNegativeLookup(MacroAssembler* masm, - Label* miss_label, - Register receiver, - Handle name, - Register scratch0, - Register scratch1) { +void StubCompiler::GenerateDictionaryNegativeLookup(MacroAssembler* masm, + Label* miss_label, + Register receiver, + Handle name, + Register scratch0, + Register scratch1) { ASSERT(name->IsUniqueName()); + ASSERT(!receiver.is(scratch0)); Counters* counters = masm->isolate()->counters(); __ IncrementCounter(counters->negative_lookups(), 1, scratch0, scratch1); __ IncrementCounter(counters->negative_lookups_miss(), 1, scratch0, scratch1); @@ -408,14 +404,11 @@ void StubCompiler::GenerateLoadFunctionPrototype(MacroAssembler* masm, } -// Generate code to check that a global property cell is empty. Create -// the property cell at compilation time if no cell exists for the -// property. -static void GenerateCheckPropertyCell(MacroAssembler* masm, - Handle global, - Handle name, - Register scratch, - Label* miss) { +void StubCompiler::GenerateCheckPropertyCell(MacroAssembler* masm, + Handle global, + Handle name, + Register scratch, + Label* miss) { Handle cell = GlobalObject::EnsurePropertyCell(global, name); ASSERT(cell->value()->IsTheHole()); __ li(scratch, Operand(cell)); @@ -1149,14 +1142,12 @@ class CallInterceptorCompiler BASE_EMBEDDED { }; -// Calls GenerateCheckPropertyCell for each global object in the prototype chain -// from object to (but not including) holder. -static void GenerateCheckPropertyCells(MacroAssembler* masm, - Handle object, - Handle holder, - Handle name, - Register scratch, - Label* miss) { +void StubCompiler::GenerateCheckPropertyCells(MacroAssembler* masm, + Handle object, + Handle holder, + Handle name, + Register scratch, + Label* miss) { Handle current = object; while (!current.is_identical_to(holder)) { if (current->IsGlobalObject()) { @@ -1364,26 +1355,6 @@ Register LoadStubCompiler::CallbackHandlerFrontend( } -void LoadStubCompiler::NonexistentHandlerFrontend( - Handle object, - Handle last, - Handle name, - Label* success, - Handle global) { - Label miss; - - HandlerFrontendHeader(object, receiver(), last, name, &miss); - - // If the last object in the prototype chain is a global object, - // check that the global property cell is empty. - if (!global.is_null()) { - GenerateCheckPropertyCell(masm(), global, name, scratch2(), &miss); - } - - HandlerFrontendFooter(name, success, &miss); -} - - void LoadStubCompiler::GenerateLoadField(Register reg, Handle holder, PropertyIndex field, diff --git a/src/stub-cache.cc b/src/stub-cache.cc index 67002a3..751798d 100644 --- a/src/stub-cache.cc +++ b/src/stub-cache.cc @@ -1208,6 +1208,40 @@ Register BaseLoadStoreStubCompiler::HandlerFrontend(Handle object, } +void LoadStubCompiler::NonexistentHandlerFrontend( + Handle object, + Handle last, + Handle name, + Label* success, + Handle global) { + Label miss; + + Register holder = + HandlerFrontendHeader(object, receiver(), last, name, &miss); + + if (!last->HasFastProperties() && + !last->IsJSGlobalObject() && + !last->IsJSGlobalProxy()) { + if (!name->IsUniqueName()) { + ASSERT(name->IsString()); + name = factory()->InternalizeString(Handle::cast(name)); + } + ASSERT(last->property_dictionary()->FindEntry(*name) == + NameDictionary::kNotFound); + GenerateDictionaryNegativeLookup(masm(), &miss, holder, name, + scratch2(), scratch3()); + } + + // If the last object in the prototype chain is a global object, + // check that the global property cell is empty. + if (!global.is_null()) { + GenerateCheckPropertyCell(masm(), global, name, scratch2(), &miss); + } + + HandlerFrontendFooter(name, success, &miss); +} + + Handle LoadStubCompiler::CompileLoadField( Handle object, Handle holder, diff --git a/src/stub-cache.h b/src/stub-cache.h index 38bc7a3..c481cc6 100644 --- a/src/stub-cache.h +++ b/src/stub-cache.h @@ -434,6 +434,18 @@ class StubCompiler BASE_EMBEDDED { int index, Register prototype); + // Helper function used to check that the dictionary doesn't contain + // the property. This function may return false negatives, so miss_label + // must always call a backup property check that is complete. + // This function is safe to call if the receiver has fast properties. + // Name must be unique and receiver must be a heap object. + static void GenerateDictionaryNegativeLookup(MacroAssembler* masm, + Label* miss_label, + Register receiver, + Handle name, + Register r0, + Register r1); + // Generates prototype loading code that uses the objects from the // context we were in when this function was called. If the context // has changed, a jump to miss is performed. This ties the generated @@ -469,6 +481,24 @@ class StubCompiler BASE_EMBEDDED { Register scratch2, Label* miss_label); + // Generate code to check that a global property cell is empty. Create + // the property cell at compilation time if no cell exists for the + // property. + static void GenerateCheckPropertyCell(MacroAssembler* masm, + Handle global, + Handle name, + Register scratch, + Label* miss); + + // Calls GenerateCheckPropertyCell for each global object in the prototype + // chain from object to (but not including) holder. + static void GenerateCheckPropertyCells(MacroAssembler* masm, + Handle object, + Handle holder, + Handle name, + Register scratch, + Label* miss); + static void TailCallBuiltin(MacroAssembler* masm, Builtins::Name name); // Generates code that verifies that the property holder has not changed diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index 2a0c367..579ede5 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -107,38 +107,34 @@ static void ProbeTable(Isolate* isolate, } -// Helper function used to check that the dictionary doesn't contain -// the property. This function may return false negatives, so miss_label -// must always call a backup property check that is complete. -// This function is safe to call if the receiver has fast properties. -// Name must be unique and receiver must be a heap object. -static void GenerateDictionaryNegativeLookup(MacroAssembler* masm, - Label* miss_label, - Register receiver, - Handle name, - Register r0, - Register r1) { +void StubCompiler::GenerateDictionaryNegativeLookup(MacroAssembler* masm, + Label* miss_label, + Register receiver, + Handle name, + Register scratch0, + Register scratch1) { ASSERT(name->IsUniqueName()); + ASSERT(!receiver.is(scratch0)); Counters* counters = masm->isolate()->counters(); __ IncrementCounter(counters->negative_lookups(), 1); __ IncrementCounter(counters->negative_lookups_miss(), 1); - __ movq(r0, FieldOperand(receiver, HeapObject::kMapOffset)); + __ movq(scratch0, FieldOperand(receiver, HeapObject::kMapOffset)); const int kInterceptorOrAccessCheckNeededMask = (1 << Map::kHasNamedInterceptor) | (1 << Map::kIsAccessCheckNeeded); // Bail out if the receiver has a named interceptor or requires access checks. - __ testb(FieldOperand(r0, Map::kBitFieldOffset), + __ testb(FieldOperand(scratch0, Map::kBitFieldOffset), Immediate(kInterceptorOrAccessCheckNeededMask)); __ j(not_zero, miss_label); // Check that receiver is a JSObject. - __ CmpInstanceType(r0, FIRST_SPEC_OBJECT_TYPE); + __ CmpInstanceType(scratch0, FIRST_SPEC_OBJECT_TYPE); __ j(below, miss_label); // Load properties array. - Register properties = r0; + Register properties = scratch0; __ movq(properties, FieldOperand(receiver, JSObject::kPropertiesOffset)); // Check that the properties array is a dictionary. @@ -152,7 +148,7 @@ static void GenerateDictionaryNegativeLookup(MacroAssembler* masm, &done, properties, name, - r1); + scratch1); __ bind(&done); __ DecrementCounter(counters->negative_lookups_miss(), 1); } @@ -777,14 +773,11 @@ void StoreStubCompiler::GenerateRestoreName(MacroAssembler* masm, } -// Generate code to check that a global property cell is empty. Create -// the property cell at compilation time if no cell exists for the -// property. -static void GenerateCheckPropertyCell(MacroAssembler* masm, - Handle global, - Handle name, - Register scratch, - Label* miss) { +void StubCompiler::GenerateCheckPropertyCell(MacroAssembler* masm, + Handle global, + Handle name, + Register scratch, + Label* miss) { Handle cell = GlobalObject::EnsurePropertyCell(global, name); ASSERT(cell->value()->IsTheHole()); @@ -1054,14 +1047,12 @@ void StoreStubCompiler::GenerateStoreField(MacroAssembler* masm, } -// Calls GenerateCheckPropertyCell for each global object in the prototype chain -// from object to (but not including) holder. -static void GenerateCheckPropertyCells(MacroAssembler* masm, - Handle object, - Handle holder, - Handle name, - Register scratch, - Label* miss) { +void StubCompiler::GenerateCheckPropertyCells(MacroAssembler* masm, + Handle object, + Handle holder, + Handle name, + Register scratch, + Label* miss) { Handle current = object; while (!current.is_identical_to(holder)) { if (current->IsGlobalObject()) { @@ -1282,30 +1273,10 @@ Register LoadStubCompiler::CallbackHandlerFrontend( } -void LoadStubCompiler::NonexistentHandlerFrontend( - Handle object, - Handle last, - Handle name, - Label* success, - Handle global) { - Label miss; - - HandlerFrontendHeader(object, receiver(), last, name, &miss); - - // If the last object in the prototype chain is a global object, - // check that the global property cell is empty. - if (!global.is_null()) { - GenerateCheckPropertyCell(masm(), global, name, scratch2(), &miss); - } - - HandlerFrontendFooter(name, success, &miss); -} - - void LoadStubCompiler::GenerateLoadField(Register reg, - Handle holder, - PropertyIndex field, - Representation representation) { + Handle holder, + PropertyIndex field, + Representation representation) { if (!reg.is(receiver())) __ movq(receiver(), reg); if (kind() == Code::LOAD_IC) { LoadFieldStub stub(field.is_inobject(holder), diff --git a/test/mjsunit/regress/regress-2980.js b/test/mjsunit/regress/regress-2980.js new file mode 100644 index 0000000..071a733 --- /dev/null +++ b/test/mjsunit/regress/regress-2980.js @@ -0,0 +1,64 @@ +// 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 test(expected, holder) { + assertEquals(expected, holder.property); +} + +var holder = {} +holder.__proto__ = null; +holder.property = "foo"; +delete holder.property; +test(undefined, holder); +test(undefined, holder); +test(undefined, holder); +holder.property = "bar"; +test("bar", holder); +test("bar", holder); + +// Now the same thing with a nontrivial prototype chain. + +function test2(expected, holder) { + assertEquals(expected, holder.prop2); +} + +var holder2 = {} +holder2.prop2 = "foo"; +holder2.__proto__ = null; +function Receiver() {} +Receiver.prototype = holder2; + +var rec2 = new Receiver(); +delete holder2.prop2; + +test2(undefined, rec2); +test2(undefined, rec2); +test2(undefined, rec2); +holder2.prop2 = "bar"; +test2("bar", rec2); +test2("bar", rec2); -- 2.7.4