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
-~----------~----~----~----~------~----~------~--~---

Reply via email to