Author: [EMAIL PROTECTED]
Date: Thu Nov 20 01:18:08 2008
New Revision: 799
Modified:
branches/bleeding_edge/src/ic-arm.cc
branches/bleeding_edge/src/ic-ia32.cc
branches/bleeding_edge/test/cctest/test-api.cc
Log:
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
Modified: branches/bleeding_edge/src/ic-arm.cc
==============================================================================
--- branches/bleeding_edge/src/ic-arm.cc (original)
+++ branches/bleeding_edge/src/ic-arm.cc Thu Nov 20 01:18:08 2008
@@ -125,6 +125,32 @@
}
+// 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 @@
__ 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 @@
__ bind(&probe);
GenerateDictionaryLoad(masm, &miss, r1, r0);
+ GenerateCheckNonFunctionOrLoaded(masm, &miss, r0, r1);
__ Ret();
// Global object access: Check access rights.
Modified: branches/bleeding_edge/src/ic-ia32.cc
==============================================================================
--- branches/bleeding_edge/src/ic-ia32.cc (original)
+++ branches/bleeding_edge/src/ic-ia32.cc Thu Nov 20 01:18:08 2008
@@ -41,7 +41,7 @@
#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 @@
}
+// 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 @@
__ 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 @@
__ 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 @@
// 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.
Modified: branches/bleeding_edge/test/cctest/test-api.cc
==============================================================================
--- branches/bleeding_edge/test/cctest/test-api.cc (original)
+++ branches/bleeding_edge/test/cctest/test-api.cc Thu Nov 20 01:18:08 2008
@@ -5321,3 +5321,27 @@
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('')");
+ }
+}
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---