Revision: 22668
Author: [email protected]
Date: Tue Jul 29 11:53:30 2014 UTC
Log: 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
[email protected]
Review URL: https://codereview.chromium.org/418023002
http://code.google.com/p/v8/source/detail?r=22668
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-392114.js
Modified:
/branches/bleeding_edge/src/arm/code-stubs-arm.cc
/branches/bleeding_edge/src/arm64/code-stubs-arm64.cc
/branches/bleeding_edge/src/ia32/code-stubs-ia32.cc
/branches/bleeding_edge/src/ic.cc
/branches/bleeding_edge/src/x64/code-stubs-x64.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-392114.js Tue Jul
29 11:53:30 2014 UTC
@@ -0,0 +1,66 @@
+// 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();
=======================================
--- /branches/bleeding_edge/src/arm/code-stubs-arm.cc Fri Jul 25 17:50:53
2014 UTC
+++ /branches/bleeding_edge/src/arm/code-stubs-arm.cc Tue Jul 29 11:53:30
2014 UTC
@@ -2958,9 +2958,14 @@
__ 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 @@
__ 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));
=======================================
--- /branches/bleeding_edge/src/arm64/code-stubs-arm64.cc Fri Jul 25
17:50:53 2014 UTC
+++ /branches/bleeding_edge/src/arm64/code-stubs-arm64.cc Tue Jul 29
11:53:30 2014 UTC
@@ -3222,15 +3222,19 @@
__ 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 x2 contains an AllocationSite
- __ AssertUndefinedOrAllocationSite(allocation_site, scratch);
+ // Verify that scratch contains an AllocationSite
+ Register map = x5;
+ __ Ldr(map, FieldMemOperand(scratch, HeapObject::kMapOffset));
+ __ JumpIfNotRoot(map, Heap::kAllocationSiteMapRootIndex, &miss);
+
+ Register allocation_site = feedback_vector;
+ __ Mov(allocation_site, scratch);
ArrayConstructorStub stub(masm->isolate(), arg_count());
__ TailCallStub(&stub);
@@ -3306,7 +3310,10 @@
__ 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);
=======================================
--- /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Fri Jul 25 17:50:53
2014 UTC
+++ /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Tue Jul 29 11:53:30
2014 UTC
@@ -2367,10 +2367,16 @@
__ 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 @@
__ 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)));
=======================================
--- /branches/bleeding_edge/src/ic.cc Mon Jul 28 10:55:32 2014 UTC
+++ /branches/bleeding_edge/src/ic.cc Tue Jul 29 11:53:30 2014 UTC
@@ -1835,8 +1835,13 @@
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 @@
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 @@
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 &&
=======================================
--- /branches/bleeding_edge/src/x64/code-stubs-x64.cc Fri Jul 25 17:50:53
2014 UTC
+++ /branches/bleeding_edge/src/x64/code-stubs-x64.cc Tue Jul 29 11:53:30
2014 UTC
@@ -2247,11 +2247,15 @@
__ 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 @@
__ 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));
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
