Reviewers: Hannes Payer, machenbach,
Message:
Here is the change we discussed. ArrayConstructorStub::Generate was not
handling
the case where the type cell had a function pointer in it. The test was
updated
to provoke the missing case, as well.
Thanks,
--Michael
Description:
Bugfix: The general array constructor stub did not handle the case
properly when it is called with a function pointer in the type cell,
instead assuming that an AllocationSite object should be present. The
case where this can happen is if the cell is uninitialized, then the
first constructor call made is to the Array function of a different
context. In that case, we'll store the function pointer in the cell,
and then go ahead and call the array constructor stub too. The bug is
fixed by checking for the AllocationSite object map. If not found, the
constructor stub goes forward with a default ElementsKind, just as in
several other cases.
A test in allocation-site-info.js was beefed up to make sure the state
chain described above is traversed.
BUG=
Please review this at https://codereview.chromium.org/18277006/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/arm/code-stubs-arm.cc
M src/ia32/code-stubs-ia32.cc
M src/x64/code-stubs-x64.cc
M test/mjsunit/allocation-site-info.js
Index: src/arm/code-stubs-arm.cc
diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc
index
58678e78a1a80e5502d5f63b96948344ecd95b27..2919f5768f9b2cf9338c78d9d1f36b0c2ce6ec89
100644
--- a/src/arm/code-stubs-arm.cc
+++ b/src/arm/code-stubs-arm.cc
@@ -7076,13 +7076,10 @@ void ArrayConstructorStub::Generate(MacroAssembler*
masm) {
__ CompareRoot(r3, Heap::kUndefinedValueRootIndex);
__ b(eq, &no_info);
- // We should have an allocation site object
- if (FLAG_debug_code) {
- __ push(r3);
- __ ldr(r3, FieldMemOperand(r3, 0));
- __ CompareRoot(r3, Heap::kAllocationSiteMapRootIndex);
- __ Assert(eq, "Expected AllocationSite object in register edx");
- }
+ // The type cell has either an AllocationSite or a JSFunction
+ __ ldr(r4, FieldMemOperand(r3, 0));
+ __ CompareRoot(r4, Heap::kAllocationSiteMapRootIndex);
+ __ b(ne, &no_info);
__ ldr(r3, FieldMemOperand(r3, AllocationSite::kPayloadOffset));
__ SmiUntag(r3);
Index: src/ia32/code-stubs-ia32.cc
diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index
093794bdcc834c95250812103db57412ab08403e..293b929dde03f657d78569c0c5016e00c19b47bb
100644
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -7628,13 +7628,10 @@ void ArrayConstructorStub::Generate(MacroAssembler*
masm) {
__ cmp(edx, Immediate(undefined_sentinel));
__ j(equal, &no_info);
- // We should have an allocation site object
- if (FLAG_debug_code) {
- __ cmp(FieldOperand(edx, 0),
- Immediate(Handle<Map>(
- masm->isolate()->heap()->allocation_site_map())));
- __ Assert(equal, "Expected AllocationSite object in register edx");
- }
+ // The type cell has either an AllocationSite or a JSFunction
+ __ cmp(FieldOperand(edx, 0), Immediate(Handle<Map>(
+ masm->isolate()->heap()->allocation_site_map())));
+ __ j(not_equal, &no_info);
__ mov(edx, FieldOperand(edx, AllocationSite::kPayloadOffset));
__ SmiUntag(edx);
Index: src/x64/code-stubs-x64.cc
diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc
index
3338013be8b0f178e6859c5c091de0a6068579d7..98c396166e8d7f16d05b74d4276738e0fc0acbdd
100644
--- a/src/x64/code-stubs-x64.cc
+++ b/src/x64/code-stubs-x64.cc
@@ -6673,12 +6673,10 @@ void ArrayConstructorStub::Generate(MacroAssembler*
masm) {
__ Cmp(rdx, undefined_sentinel);
__ j(equal, &no_info);
- // We should have an allocation site object
- if (FLAG_debug_code) {
- __ Cmp(FieldOperand(rdx, 0),
- Handle<Map>(masm->isolate()->heap()->allocation_site_map()));
- __ Assert(equal, "Expected AllocationSite object in register rdx");
- }
+ // The type cell has either an AllocationSite or a JSFunction
+ __ Cmp(FieldOperand(rdx, 0),
+ Handle<Map>(masm->isolate()->heap()->allocation_site_map()));
+ __ j(not_equal, &no_info);
__ movq(rdx, FieldOperand(rdx, AllocationSite::kPayloadOffset));
__ SmiToInteger32(rdx, rdx);
Index: test/mjsunit/allocation-site-info.js
diff --git a/test/mjsunit/allocation-site-info.js
b/test/mjsunit/allocation-site-info.js
index
1369a2d2ec801b1861d7b4f91d9fcd5936ec1746..72df772b0c11986940ec61b0f6e17ca5c0d951f7
100644
--- a/test/mjsunit/allocation-site-info.js
+++ b/test/mjsunit/allocation-site-info.js
@@ -297,9 +297,26 @@ if (support_smi_only_arrays) {
assertTrue(new type(1,2,3) instanceof type);
}
+ function instanceof_check2(type) {
+ assertTrue(new type() instanceof type);
+ assertTrue(new type(5) instanceof type);
+ assertTrue(new type(1,2,3) instanceof type);
+ }
+
var realmBArray = Realm.eval(realmB, "Array");
instanceof_check(Array);
instanceof_check(realmBArray);
+
+ // instanceof_check2 is here because the call site goes through a state.
+ // Since instanceof_check(Array) was first called with the current
context
+ // Array function, it went from (uninit->Array) then
(Array->megamorphic).
+ // We'll get a different state traversal if we start with realmBArray.
+ // It'll go (uninit->realmBArray) then (realmBArray->megamorphic).
Recognize
+ // that state "Array" implies an AllocationSite is present, and code is
+ // configured to use it.
+ instanceof_check2(realmBArray);
+ instanceof_check2(Array);
+
%OptimizeFunctionOnNextCall(instanceof_check);
// No de-opt will occur because HCallNewArray wasn't selected, on
account of
--
--
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/groups/opt_out.
