When probing a dictionary backing storage in generated code, make sure
authorager@chromium.org <ager@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Thu, 20 Nov 2008 09:18:08 +0000 (09:18 +0000)
committerager@chromium.org <ager@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Thu, 20 Nov 2008 09:18:08 +0000 (09:18 +0000)
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

src/ic-arm.cc
src/ic-ia32.cc
test/cctest/test-api.cc

index 92eb5d36dd08c6f4c6ada0ab7eee8e00fb4a2300..975fa0f821c4dd2bb6308d75f840aa98858298e2 100644 (file)
@@ -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.
index 11e55a9291fb2b9f5eb2a7705393623e3fe7a676..7ff47b9902be24a3552e4b7ec7f411205a72fd47 100644 (file)
@@ -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.
index 3c912945583f6232132054092517f3738e337039..e1549ebcdf71078b6893987531c98723bac3f194 100644 (file)
@@ -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('')");
+  }
+}