CallIC customization stubs must accept that a vector slot is cleared.

The CallIC Array custom IC stub read from the type vector, expecting
to get an AllocationSite. But there are paths in the system where a
type vector can be re-created with default values, even though we
currently grant an exception to clearing of vector slots with
AllocationSites in them at gc time.

BUG=392114
LOG=N
R=verwaest@chromium.org

Review URL: https://codereview.chromium.org/418023002

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22668 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/code-stubs-arm.cc

@@ -2958,9 +2958,14 @@ void CallIC_ArrayStub::Generate(MacroAssembler* masm) {
 
   __ mov(r0, Operand(arg_count()));
   __ add(r4, r2, Operand::PointerOffsetFromSmiKey(r3));
-  __ ldr(r2, FieldMemOperand(r4, FixedArray::kHeaderSize));
-  // Verify that r2 contains an AllocationSite
-  __ AssertUndefinedOrAllocationSite(r2, r4);
+  __ ldr(r4, FieldMemOperand(r4, FixedArray::kHeaderSize));
+
+  // Verify that r4 contains an AllocationSite
+  __ ldr(r5, FieldMemOperand(r4, HeapObject::kMapOffset));
+  __ CompareRoot(r5, Heap::kAllocationSiteMapRootIndex);
+  __ b(ne, &miss);
+
+  __ mov(r2, r4);
   ArrayConstructorStub stub(masm->isolate(), arg_count());
   __ TailCallStub(&stub);
 
@@ -3027,7 +3032,11 @@ void CallICStub::Generate(MacroAssembler* masm) {
   __ b(eq, &miss);
 
   if (!FLAG_trace_ic) {
-    // We are going megamorphic, and we don't want to visit the runtime.
+    // We are going megamorphic. If the feedback is a JSFunction, it is fine
+    // to handle it here. More complex cases are dealt with in the runtime.
+    __ AssertNotSmi(r4);
+    __ CompareObjectType(r4, r5, r5, JS_FUNCTION_TYPE);
+    __ b(ne, &miss);
     __ add(r4, r2, Operand::PointerOffsetFromSmiKey(r3));
     __ LoadRoot(ip, Heap::kMegamorphicSymbolRootIndex);
     __ str(ip, FieldMemOperand(r4, FixedArray::kHeaderSize));

src/arm64/code-stubs-arm64.cc

@@ -3222,15 +3222,19 @@ void CallIC_ArrayStub::Generate(MacroAssembler* masm) {
   __ Cmp(function, scratch);
   __ B(ne, &miss);
 
-  Register allocation_site = feedback_vector;
   __ Mov(x0, Operand(arg_count()));
 
   __ Add(scratch, feedback_vector,
          Operand::UntagSmiAndScale(index, kPointerSizeLog2));
-  __ Ldr(allocation_site, FieldMemOperand(scratch, FixedArray::kHeaderSize));
+  __ Ldr(scratch, FieldMemOperand(scratch, FixedArray::kHeaderSize));
+
+  // Verify that scratch contains an AllocationSite
+  Register map = x5;
+  __ Ldr(map, FieldMemOperand(scratch, HeapObject::kMapOffset));
+  __ JumpIfNotRoot(map, Heap::kAllocationSiteMapRootIndex, &miss);
 
-  // Verify that x2 contains an AllocationSite
-  __ AssertUndefinedOrAllocationSite(allocation_site, scratch);
+  Register allocation_site = feedback_vector;
+  __ Mov(allocation_site, scratch);
   ArrayConstructorStub stub(masm->isolate(), arg_count());
   __ TailCallStub(&stub);
 
@@ -3306,7 +3310,10 @@ void CallICStub::Generate(MacroAssembler* masm) {
   __ JumpIfRoot(x4, Heap::kUninitializedSymbolRootIndex, &miss);
 
   if (!FLAG_trace_ic) {
-    // We are going megamorphic, and we don't want to visit the runtime.
+    // We are going megamorphic. If the feedback is a JSFunction, it is fine
+    // to handle it here. More complex cases are dealt with in the runtime.
+    __ AssertNotSmi(x4);
+    __ JumpIfNotObjectType(x4, x5, x5, JS_FUNCTION_TYPE, &miss);
     __ Add(x4, feedback_vector,
            Operand::UntagSmiAndScale(index, kPointerSizeLog2));
     __ LoadRoot(x5, Heap::kMegamorphicSymbolRootIndex);

src/ia32/code-stubs-ia32.cc

@@ -2367,10 +2367,16 @@ void CallIC_ArrayStub::Generate(MacroAssembler* masm) {
   __ j(not_equal, &miss);
 
   __ mov(eax, arg_count());
-  __ mov(ebx, FieldOperand(ebx, edx, times_half_pointer_size,
+  __ mov(ecx, FieldOperand(ebx, edx, times_half_pointer_size,
                            FixedArray::kHeaderSize));
+
   // Verify that ecx contains an AllocationSite
-  __ AssertUndefinedOrAllocationSite(ebx);
+  Factory* factory = masm->isolate()->factory();
+  __ cmp(FieldOperand(ecx, HeapObject::kMapOffset),
+         factory->allocation_site_map());
+  __ j(not_equal, &miss);
+
+  __ mov(ebx, ecx);
   ArrayConstructorStub stub(masm->isolate(), arg_count());
   __ TailCallStub(&stub);
 
@@ -2441,7 +2447,11 @@ void CallICStub::Generate(MacroAssembler* masm) {
   __ j(equal, &miss);
 
   if (!FLAG_trace_ic) {
-    // We are going megamorphic, and we don't want to visit the runtime.
+    // We are going megamorphic. If the feedback is a JSFunction, it is fine
+    // to handle it here. More complex cases are dealt with in the runtime.
+    __ AssertNotSmi(ecx);
+    __ CmpObjectType(ecx, JS_FUNCTION_TYPE, ecx);
+    __ j(not_equal, &miss);
     __ mov(FieldOperand(ebx, edx, times_half_pointer_size,
                         FixedArray::kHeaderSize),
            Immediate(TypeFeedbackInfo::MegamorphicSentinel(isolate)));

src/ic.cc

@@ -1835,8 +1835,13 @@ bool CallIC::DoCustomHandler(Handle<Object> receiver,
       isolate()->native_context()->array_function());
   if (array_function.is_identical_to(Handle<JSFunction>::cast(function))) {
     // Alter the slot.
-    Handle<AllocationSite> new_site = isolate()->factory()->NewAllocationSite();
-    vector->set(slot->value(), *new_site);
+    Object* feedback = vector->get(slot->value());
+    if (!feedback->IsAllocationSite()) {
+      Handle<AllocationSite> new_site =
+          isolate()->factory()->NewAllocationSite();
+      vector->set(slot->value(), *new_site);
+    }
+
     CallIC_ArrayStub stub(isolate(), state);
     set_target(*stub.GetCode());
     Handle<String> name;
@@ -1876,6 +1881,9 @@ void CallIC::HandleMiss(Handle<Object> receiver,
   State state(target()->extra_ic_state());
   Object* feedback = vector->get(slot->value());
 
+  // Hand-coded MISS handling is easier if CallIC slots don't contain smis.
+  ASSERT(!feedback->IsSmi());
+
   if (feedback->IsJSFunction() || !function->IsJSFunction()) {
     // We are going generic.
     vector->set(slot->value(),
@@ -1884,9 +1892,14 @@ void CallIC::HandleMiss(Handle<Object> receiver,
 
     TRACE_GENERIC_IC(isolate(), "CallIC", "megamorphic");
   } else {
-    // If we came here feedback must be the uninitialized sentinel,
-    // and we are going monomorphic.
-    ASSERT(feedback == *TypeFeedbackInfo::UninitializedSentinel(isolate()));
+    // The feedback is either uninitialized or an allocation site.
+    // It might be an allocation site because if we re-compile the full code
+    // to add deoptimization support, we call with the default call-ic, and
+    // merely need to patch the target to match the feedback.
+    // TODO(mvstanton): the better approach is to dispense with patching
+    // altogether, which is in progress.
+    ASSERT(feedback == *TypeFeedbackInfo::UninitializedSentinel(isolate()) ||
+           feedback->IsAllocationSite());
 
     // Do we want to install a custom handler?
     if (FLAG_use_ic &&

src/x64/code-stubs-x64.cc

@@ -2247,11 +2247,15 @@ void CallIC_ArrayStub::Generate(MacroAssembler* masm) {
   __ j(not_equal, &miss);
 
   __ movp(rax, Immediate(arg_count()));
-  __ movp(rbx, FieldOperand(rbx, rdx, times_pointer_size,
+  __ movp(rcx, FieldOperand(rbx, rdx, times_pointer_size,
                             FixedArray::kHeaderSize));
-
   // Verify that ecx contains an AllocationSite
-  __ AssertUndefinedOrAllocationSite(rbx);
+  Factory* factory = masm->isolate()->factory();
+  __ Cmp(FieldOperand(rcx, HeapObject::kMapOffset),
+         factory->allocation_site_map());
+  __ j(not_equal, &miss);
+
+  __ movp(rbx, rcx);
   ArrayConstructorStub stub(masm->isolate(), arg_count());
   __ TailCallStub(&stub);
 
@@ -2325,7 +2329,11 @@ void CallICStub::Generate(MacroAssembler* masm) {
   __ j(equal, &miss);
 
   if (!FLAG_trace_ic) {
-    // We are going megamorphic, and we don't want to visit the runtime.
+    // We are going megamorphic. If the feedback is a JSFunction, it is fine
+    // to handle it here. More complex cases are dealt with in the runtime.
+    __ AssertNotSmi(rcx);
+    __ CmpObjectType(rcx, JS_FUNCTION_TYPE, rcx);
+    __ j(not_equal, &miss);
     __ Move(FieldOperand(rbx, rdx, times_pointer_size,
                          FixedArray::kHeaderSize),
             TypeFeedbackInfo::MegamorphicSentinel(isolate));

test/mjsunit/regress/regress-392114.js

+// Copyright 2014 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.
+
+// Flags: --expose-debug-as debug --allow-natives-syntax
+
+Debug = debug.Debug;
+
+function dummy(x) {
+  return x + 100;
+}
+
+function create_closure() {
+  var f = function(arg) {
+    if (arg) { %DeoptimizeFunction(f); }
+    var a = Array(10);
+    for (var i = 0; i < a.length; i++) {
+      a[i] = i;
+    }
+  }
+  return f;
+}
+
+var c = create_closure();
+c();
+
+// c CallIC state now has custom Array handler installed.
+
+// Turn on the debugger.
+Debug.setListener(function () {});
+
+var d = create_closure();
+%OptimizeFunctionOnNextCall(d);
+// Thanks to the debugger, we recreate the full code too. We deopt and run
+// it, stomping on the unexpected AllocationSite in the type vector slot.
+d(true);
+
+// CallIC in c misinterprets type vector slot contents as an AllocationSite,
+// corrupting the heap.
+c();
+
+// CallIC MISS - crash due to corruption.
+dummy();