From: ager@chromium.org Date: Thu, 20 Nov 2008 09:18:08 +0000 (+0000) Subject: When probing a dictionary backing storage in generated code, make sure X-Git-Tag: upstream/4.7.83~24994 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4bd0667324e44e6f219a52208be59b8c8759dd4f;p=platform%2Fupstream%2Fv8.git When probing a dictionary backing storage in generated code, make sure not to return functions that have not been loaded. This fixes crashes on expedia.com: http://code.google.com/p/chromium/issues/detail?id=4526 Review URL: http://codereview.chromium.org/11272 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@799 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/ic-arm.cc b/src/ic-arm.cc index 92eb5d36d..975fa0f82 100644 --- a/src/ic-arm.cc +++ b/src/ic-arm.cc @@ -125,6 +125,32 @@ static void GenerateDictionaryLoad(MacroAssembler* masm, } +// Helper function used to check that a value is either not a function +// or is loaded if it is a function. +static void GenerateCheckNonFunctionOrLoaded(MacroAssembler* masm, + Label* miss, + Register value, + Register scratch) { + Label done; + // Check if the value is a Smi. + __ tst(value, Operand(kSmiTagMask)); + __ b(eq, &done); + // Check if the value is a function. + __ ldr(scratch, FieldMemOperand(value, HeapObject::kMapOffset)); + __ ldrb(scratch, FieldMemOperand(scratch, Map::kInstanceTypeOffset)); + __ cmp(scratch, Operand(JS_FUNCTION_TYPE)); + __ b(ne, &done); + // Check if the function has been loaded. + __ ldr(scratch, + FieldMemOperand(value, JSFunction::kSharedFunctionInfoOffset)); + __ ldr(scratch, + FieldMemOperand(scratch, SharedFunctionInfo::kLazyLoadDataOffset)); + __ cmp(scratch, Operand(Factory::undefined_value())); + __ b(ne, miss); + __ bind(&done); +} + + void LoadIC::GenerateArrayLength(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- r2 : name @@ -300,6 +326,12 @@ static void GenerateNormalHelper(MacroAssembler* masm, __ cmp(r0, Operand(JS_FUNCTION_TYPE)); __ b(ne, miss); + // Check that the function has been loaded. + __ ldr(r0, FieldMemOperand(r1, JSFunction::kSharedFunctionInfoOffset)); + __ ldr(r0, FieldMemOperand(r0, SharedFunctionInfo::kLazyLoadDataOffset)); + __ cmp(r0, Operand(Factory::undefined_value())); + __ b(ne, miss); + // Patch the receiver with the global proxy if necessary. if (is_global_object) { __ ldr(r2, MemOperand(sp, argc * kPointerSize)); @@ -467,6 +499,7 @@ void LoadIC::GenerateNormal(MacroAssembler* masm) { __ bind(&probe); GenerateDictionaryLoad(masm, &miss, r1, r0); + GenerateCheckNonFunctionOrLoaded(masm, &miss, r0, r1); __ Ret(); // Global object access: Check access rights. diff --git a/src/ic-ia32.cc b/src/ic-ia32.cc index 11e55a929..7ff47b990 100644 --- a/src/ic-ia32.cc +++ b/src/ic-ia32.cc @@ -41,7 +41,7 @@ namespace v8 { namespace internal { #define __ masm-> -// Helper function used from LoadIC/CallIC GenerateNormal. +// Helper function used to load a property from a dictionary backing storage. static void GenerateDictionaryLoad(MacroAssembler* masm, Label* miss_label, Register r0, Register r1, Register r2, Register name) { @@ -121,6 +121,29 @@ static void GenerateDictionaryLoad(MacroAssembler* masm, Label* miss_label, } +// Helper function used to check that a value is either not a function +// or is loaded if it is a function. +static void GenerateCheckNonFunctionOrLoaded(MacroAssembler* masm, Label* miss, + Register value, Register scratch) { + Label done; + // Check if the value is a Smi. + __ test(value, Immediate(kSmiTagMask)); + __ j(zero, &done, not_taken); + // Check if the value is a function. + __ mov(scratch, FieldOperand(value, HeapObject::kMapOffset)); + __ movzx_b(scratch, FieldOperand(scratch, Map::kInstanceTypeOffset)); + __ cmp(scratch, JS_FUNCTION_TYPE); + __ j(not_equal, &done, taken); + // Check if the function has been loaded. + __ mov(scratch, FieldOperand(value, JSFunction::kSharedFunctionInfoOffset)); + __ mov(scratch, + FieldOperand(scratch, SharedFunctionInfo::kLazyLoadDataOffset)); + __ cmp(scratch, Factory::undefined_value()); + __ j(not_equal, miss, not_taken); + __ bind(&done); +} + + void LoadIC::GenerateArrayLength(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- ecx : name @@ -236,6 +259,7 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) { __ j(not_zero, &slow, not_taken); // Probe the dictionary leaving result in ecx. GenerateDictionaryLoad(masm, &slow, ebx, ecx, edx, eax); + GenerateCheckNonFunctionOrLoaded(masm, &slow, ecx, edx); __ mov(eax, Operand(ecx)); __ IncrementCounter(&Counters::keyed_load_generic_symbol, 1); __ ret(0); @@ -456,6 +480,12 @@ static void GenerateNormalHelper(MacroAssembler* masm, __ cmp(edx, JS_FUNCTION_TYPE); __ j(not_equal, miss, not_taken); + // Check that the function has been loaded. + __ mov(edx, FieldOperand(edi, JSFunction::kSharedFunctionInfoOffset)); + __ mov(edx, FieldOperand(edx, SharedFunctionInfo::kLazyLoadDataOffset)); + __ cmp(edx, Factory::undefined_value()); + __ j(not_equal, miss, not_taken); + // Patch the receiver with the global proxy if necessary. if (is_global_object) { __ mov(edx, Operand(esp, (argc + 1) * kPointerSize)); @@ -627,6 +657,7 @@ void LoadIC::GenerateNormal(MacroAssembler* masm) { // Search the dictionary placing the result in eax. __ bind(&probe); GenerateDictionaryLoad(masm, &miss, edx, eax, ebx, ecx); + GenerateCheckNonFunctionOrLoaded(masm, &miss, eax, edx); __ ret(0); // Global object access: Check access rights. diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 3c9129455..e1549ebcd 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -5321,3 +5321,27 @@ TEST(PreCompile) { CHECK_NE(sd->Length(), 0); CHECK_NE(sd->Data(), NULL); } + + +// This tests that we do not allow dictionary load/call inline caches +// to use functions that have not yet been compiled. The potential +// problem of loading a function that has not yet been compiled can +// arise because we share code between contexts via the compilation +// cache. +THREADED_TEST(DictionaryICLoadedFunction) { + v8::HandleScope scope; + // Test LoadIC. + for (int i = 0; i < 2; i++) { + LocalContext context; + context->Global()->Set(v8_str("tmp"), v8::True()); + context->Global()->Delete(v8_str("tmp")); + CompileRun("for (var j = 0; j < 10; j++) new RegExp('');"); + } + // Test CallIC. + for (int i = 0; i < 2; i++) { + LocalContext context; + context->Global()->Set(v8_str("tmp"), v8::True()); + context->Global()->Delete(v8_str("tmp")); + CompileRun("for (var j = 0; j < 10; j++) RegExp('')"); + } +}