[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat

[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat

Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail.
This CL adds a fix to globally invalidate a isConcatSpreadable_protector.

Drive-by-fix: use named accessors for context variables

LOG=N
BUG=chromium:542504, v8:903
Committed: https://crrev.com/f87014ebdefc759ff58b7c9557c417e0f57749ba
Cr-Commit-Position: refs/heads/master@{#36201}

[Camillo Bruni, 2015-10-23 12:38:22]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-23 12:38:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/1

[commit-bot: I haz the power, 2015-10-23 12:49:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-23 12:49:47]

Dry run: Try jobs failed on following builders:
  v8_linux_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel/builds/10916)

[Camillo Bruni, 2015-10-26 16:03:15]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-26 16:03:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/90001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/90001

[commit-bot: I haz the power, 2015-10-26 16:57:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-26 16:57:38]

Dry run: This issue passed the CQ dry run.

[Camillo Bruni, 2015-10-27 14:29:06]

cbruni@chromium.org changed reviewers:
+ verwaest@chromium.org

[Camillo Bruni, 2015-10-27 14:29:07]

PTAL

[adamk, 2015-10-28 00:03:51]

adamk@chromium.org changed reviewers:
+ adamk@chromium.org

[adamk, 2015-10-28 00:03:52]

Can you expand on this CL description to make it clear that you're properly
handling the case where someone sets @@isConcatSpreadable on a prototype of the
array instance? Without looking at the CL that wasn't clear.

I'm also curious how sustainable this approach to well-known Symbols is. Are we
going to have to add the same thing for @@species, to avoid putting a [[Get]]
for @@species on the fast path for, say, Array.prototype.slice?

[Camillo Bruni, 2015-10-28 09:16:26]

On 2015/10/28 at 00:03:52, adamk wrote:
> Can you expand on this CL description to make it clear that you're properly
handling the case where someone sets @@isConcatSpreadable on a prototype of the
array instance? Without looking at the CL that wasn't clear.
> 
> I'm also curious how sustainable this approach to well-known Symbols is. Are
we going to have to add the same thing for @@species, to avoid putting a [[Get]]
for @@species on the fast path for, say, Array.prototype.slice?

sure :)

[Toon Verwaest, 2015-11-02 12:32:46]

Some minor comments.

https://codereview.chromium.org/1409123003/diff/90001/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/1409123003/diff/90001/src/isolate.cc#newcode2470
src/isolate.cc:2470: if (maybeValue.ToHandle(&value) && !value->IsUndefined()) {
Plus o[@@isConcatSpreadable] = undefined

https://codereview.chromium.org/1409123003/diff/90001/src/isolate.cc#newcode2489
src/isolate.cc:2489: Handle<Name> key(factory()->is_concat_spreadable_symbol());
key = factory()->is_concat_spreadable_symbol();

https://codereview.chromium.org/1409123003/diff/90001/src/isolate.h
File src/isolate.h (right):

https://codereview.chromium.org/1409123003/diff/90001/src/isolate.h#newcode963
src/isolate.h:963: // The isConcatSpreadable protector is valid if there this
property is
valid if @@isConcatSpreadable is not present on ...

https://codereview.chromium.org/1409123003/diff/90001/src/isolate.h#newcode972
src/isolate.h:972: // The elements protector is valid if there are not
additional elements
not -> no

https://codereview.chromium.org/1409123003/diff/90001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1409123003/diff/90001/src/objects.cc#newcode3908
src/objects.cc:3908: //     receiver, it->GetName());
stray change?

https://codereview.chromium.org/1409123003/diff/90001/src/objects.cc#newcode4042
src/objects.cc:4042:
it->isolate()->UpdateArrayIsConcatSpreadableProtectorOnAddProperty(
I don't think all stores go through here. Generic keyed stores to dictionary
mode receivers probably don't.

[Camillo Bruni, 2015-11-03 10:31:16]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-03 10:31:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/110001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/110001

[commit-bot: I haz the power, 2015-11-03 10:32:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-03 10:32:27]

Dry run: Try jobs failed on following builders:
  v8_linux_arm_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/9723)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/11434)

[Camillo Bruni, 2015-11-03 12:51:10]

Description was changed from

==========
[runtime] Do not check @@isConcatSpreadable for fast path
@@isConcatSpreadable has no influence on Array.prototype.concat when
invoked with Arrays only.

LOG=N
BUG=chromium:542504
==========

to

==========
[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat
Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail. This
CL adds a fix to globally invalidate a isConcatSpreadable_protector.

LOG=N
BUG=chromium:542504
==========

[Camillo Bruni, 2015-11-03 13:01:13]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-03 13:01:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/50002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/50002

[commit-bot: I haz the power, 2015-11-03 13:12:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-03 13:12:54]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
  v8_win_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel/builds/11380)

[Camillo Bruni, 2015-11-03 13:45:42]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-03 13:45:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/140001

[commit-bot: I haz the power, 2015-11-03 14:16:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-03 14:16:04]

Dry run: This issue passed the CQ dry run.

[Camillo Bruni, 2015-11-03 14:49:25]

PTAL addressed all but one comment.

https://codereview.chromium.org/1409123003/diff/90001/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/1409123003/diff/90001/src/isolate.cc#newcode2470
src/isolate.cc:2470: if (maybeValue.ToHandle(&value) && !value->IsUndefined()) {
On 2015/11/02 at 12:32:45, Toon Verwaest wrote:
> Plus o[@@isConcatSpreadable] = undefined?

?

https://codereview.chromium.org/1409123003/diff/90001/src/isolate.cc#newcode2489
src/isolate.cc:2489: Handle<Name> key(factory()->is_concat_spreadable_symbol());
On 2015/11/02 at 12:32:45, Toon Verwaest wrote:
> key = factory()->is_concat_spreadable_symbol();

right

https://codereview.chromium.org/1409123003/diff/90001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1409123003/diff/90001/src/objects.cc#newcode3908
src/objects.cc:3908: //     receiver, it->GetName());
On 2015/11/02 at 12:32:46, Toon Verwaest wrote:
> stray change?

indeed.

https://codereview.chromium.org/1409123003/diff/90001/src/objects.cc#newcode4042
src/objects.cc:4042:
it->isolate()->UpdateArrayIsConcatSpreadableProtectorOnAddProperty(
On 2015/11/02 at 12:32:45, Toon Verwaest wrote:
> I don't think all stores go through here. Generic keyed stores to dictionary
mode receivers probably don't.

I added another test case, if I understood it correctly we should be in exactly
this case (generic IC with dict mode receiver) and we still end up here.

[Camillo Bruni, 2016-03-23 13:39:24]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-23 13:39:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/240001

[commit-bot: I haz the power, 2016-03-23 14:08:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-23 14:08:05]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)

[Camillo Bruni, 2016-03-29 16:04:10]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-29 16:04:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/260001

[commit-bot: I haz the power, 2016-03-29 16:28:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-29 16:28:48]

Dry run: This issue passed the CQ dry run.

[Camillo Bruni, 2016-03-30 09:21:34]

PTAL

[adamk, 2016-03-30 16:56:10]

adamk@chromium.org changed reviewers:
- adamk@chromium.org

[adamk, 2016-03-30 16:56:11]

Look forward to seeing this land, but Toon is definitely the reviewer you want.

[Toon Verwaest, 2016-03-31 08:09:46]

Sorry for slowpoking. Here you go.

https://codereview.chromium.org/1409123003/diff/260001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1409123003/diff/260001/src/builtins.cc#newcod...
src/builtins.cc:1415: map->prototype() == *isolate->initial_array_prototype()) {
isolate->[NATIVE_CONTEXT_FIELD]() does
Handle<...>(isolate->native_context()->[NATIVE_CONTEXT_FIELD](), isolate)
internally, so that allocates a handle. Directly use
isolate->native_context()->...() instead to avoid the unnecessary handle.

https://codereview.chromium.org/1409123003/diff/260001/src/builtins.cc#newcod...
src/builtins.cc:1452: DCHECK(0 <= result_len);
DCHECK_LE(...)

https://codereview.chromium.org/1409123003/diff/260001/src/crankshaft/arm/lit...
File src/crankshaft/arm/lithium-codegen-arm.cc (right):

https://codereview.chromium.org/1409123003/diff/260001/src/crankshaft/arm/lit...
src/crankshaft/arm/lithium-codegen-arm.cc:2979: // Otherwise it needs to bail
out.
Spurious change ... :)

https://codereview.chromium.org/1409123003/diff/260001/src/elements.cc
File src/elements.cc (right):

https://codereview.chromium.org/1409123003/diff/260001/src/elements.cc#newcod...
src/elements.cc:2980: ElementsKind arg_kind =
JSArray::cast(arg)->map()->elements_kind();
JSArray::cast(arg)->GetElementsKind()

https://codereview.chromium.org/1409123003/diff/260001/src/elements.cc#newcod...
src/elements.cc:3018: DCHECK(insertion_index == result_len);
Spurious change

https://codereview.chromium.org/1409123003/diff/260001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1409123003/diff/260001/src/heap/heap.cc#newco...
src/heap/heap.cc:2912: set_array_is_concat_spreadable_protector(*cell);
Unless you want to be able to add dependencies to the cell in optimized code,
there's no need to use a PropertyCell rather than a Cell. The same is probably
true for species_cell below.

https://codereview.chromium.org/1409123003/diff/260001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1409123003/diff/260001/src/objects.cc#newcode...
src/objects.cc:4512: receiver, it->name());
This is by far not enough. Reuse dehrenberg's LookupIterator::UpdateProtector to
do the proper invalidation. Just add the new symbol case in there.

https://codereview.chromium.org/1409123003/diff/260001/test/mjsunit/es6/array...
File test/mjsunit/es6/array-concat.js (right):

https://codereview.chromium.org/1409123003/diff/260001/test/mjsunit/es6/array...
test/mjsunit/es6/array-concat.js:27: array[Symbol.isConcatSpreadable] = false;
Make sure to add tests for all the missing cases. See the tests added by Dan for
his protectors.

https://codereview.chromium.org/1409123003/diff/260001/test/mjsunit/harmony/a...
File test/mjsunit/harmony/array-concat-array-proto.js (right):

https://codereview.chromium.org/1409123003/diff/260001/test/mjsunit/harmony/a...
test/mjsunit/harmony/array-concat-array-proto.js:1: // Copyright 2014 the V8
project authors. All rights reserved.
http://s2.quickmeme.com/img/98/98b39b21ff008afaa5cba1e79fc611b2ddb64b3f44f57e...

[Toon Verwaest, 2016-04-05 12:57:26]

https://codereview.chromium.org/1409123003/diff/340001/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/1409123003/diff/340001/src/isolate.cc#newcode...
src/isolate.cc:2489: bool Isolate::ArrayOrObjectPrototypeIsInContext(Object*
object) {
IsArrayOrObjectPrototype

https://codereview.chromium.org/1409123003/diff/340001/src/isolate.cc#newcode...
src/isolate.cc:2608: MaybeHandle<Object> maybe_value = Object::GetProperty(&it);
GetDataProperty avoids side effects. You can't just GetProperty. Not sure this
makes sense, as it won't trigger for proxies, interceptors, getters, ...

https://codereview.chromium.org/1409123003/diff/340001/src/isolate.cc#newcode...
src/isolate.cc:2625: if (!object->map()->is_prototype_map()) return;
This check is probably faster (and will generally hit) than the one above.

[Camillo Bruni, 2016-04-06 09:13:15]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-06 09:13:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/360001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/360001

[commit-bot: I haz the power, 2016-04-06 09:16:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-06 09:16:55]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/3858)

[Camillo Bruni, 2016-04-06 13:14:31]

cbruni@chromium.org changed reviewers:
+ ulan@chromium.org

[Camillo Bruni, 2016-04-06 13:14:32]

PTAL again.
ulan@ please have a look at heap/*

https://codereview.chromium.org/1409123003/diff/260001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1409123003/diff/260001/src/heap/heap.cc#newco...
src/heap/heap.cc:2912: set_array_is_concat_spreadable_protector(*cell);
On 2016/03/31 at 08:09:46, Toon Verwaest wrote:
> Unless you want to be able to add dependencies to the cell in optimized code,
there's no need to use a PropertyCell rather than a Cell. The same is probably
true for species_cell below.

ok.

[Camillo Bruni, 2016-04-06 13:14:35]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-06 13:14:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/400001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/400001

[ulan, 2016-04-06 13:17:21]

heap lgtm

[commit-bot: I haz the power, 2016-04-06 13:24:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-06 13:24:35]

Dry run: Try jobs failed on following builders:
  v8_win_compile_dbg on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/1...)

[Camillo Bruni, 2016-04-06 13:35:56]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-06 13:36:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/420001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/420001

[commit-bot: I haz the power, 2016-04-06 13:44:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-06 13:44:58]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/...)
  v8_linux64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/3880)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)

[Michael Achenbach, 2016-04-06 13:52:53]

The CQ bit was checked by machenbach@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-06 13:53:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/420001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/420001

[commit-bot: I haz the power, 2016-04-06 14:09:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-06 14:09:41]

Dry run: Try jobs failed on following builders:
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/18025)

[Camillo Bruni, 2016-04-08 09:29:08]

Description was changed from

==========
[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat
Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail. This
CL adds a fix to globally invalidate a isConcatSpreadable_protector.

LOG=N
BUG=chromium:542504
==========

to

==========
[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat
Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail. This
CL adds a fix to globally invalidate a isConcatSpreadable_protector.

LOG=N
BUG=chromium:542504, v8:903
==========

[Toon Verwaest, 2016-04-11 09:30:56]

Getting there :)

https://codereview.chromium.org/1409123003/diff/440001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1409123003/diff/440001/src/builtins.cc#newcod...
src/builtins.cc:1334: if
(!isolate->IsArrayIsConcatSpreadableLookupChainIntact()) {
This only guarantees that Array.prototype and Object.prototype don't have it. It
doesn't guarantee that the receiver itself doesn't have it. Also, it could be
installed on a non-array or array with non-default .__proto__. In all those
cases you need to slow-fetch the property.
Please add tests for all these currently unsupported cases.
Basically you have to
if (IsSimpleArray(obj) && IsArrayIsConcatSpreadableLookupChainIntact()) return
Object::IsArray(obj);

[Camillo Bruni, 2016-04-11 11:55:12]

https://codereview.chromium.org/1409123003/diff/440001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1409123003/diff/440001/src/builtins.cc#newcod...
src/builtins.cc:1334: if
(!isolate->IsArrayIsConcatSpreadableLookupChainIntact()) {
On 2016/04/11 at 09:30:56, Toon Verwaest wrote:
> This only guarantees that Array.prototype and Object.prototype don't have it.
It doesn't guarantee that the receiver itself doesn't have it. Also, it could be
installed on a non-array or array with non-default .__proto__. In all those
cases you need to slow-fetch the property.
> Please add tests for all these currently unsupported cases.
> Basically you have to
> if (IsSimpleArray(obj) && IsArrayIsConcatSpreadableLookupChainIntact()) return
Object::IsArray(obj);

I think the naming for the protector is confusing. 
Currently I invalidate the protector in lookup.cc whenever you use
@@IsConcatSpreadable, independently from the receiver.


test/mjsunit/es6/array-concat.js added further down already covers the case of
@@IsConcatSpreadable on the receiver.

[Toon Verwaest, 2016-04-13 14:26:54]

Ok in that case just change the name

[Michael Achenbach, 2016-04-13 14:28:45]

Description was changed from

==========
[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat
Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail. This
CL adds a fix to globally invalidate a isConcatSpreadable_protector.

LOG=N
BUG=chromium:542504, v8:903
==========

to

==========
[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat
Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail. This
CL adds a fix to globally invalidate a isConcatSpreadable_protector.

LOG=N
BUG=chromium:542504, v8:903
==========

[Michael Achenbach, 2016-04-13 14:28:45]

machenbach@chromium.org changed reviewers:
- machenbach@chromium.org

[Camillo Bruni, 2016-04-15 09:32:11]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-15 09:32:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/460001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/460001

[commit-bot: I haz the power, 2016-04-15 09:48:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-15 09:49:00]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
  v8_win_nosnap_shared_rel_ng_triggered on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Camillo Bruni, 2016-04-15 11:36:39]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-15 11:36:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/480001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/480001

[commit-bot: I haz the power, 2016-04-15 11:52:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-15 11:52:38]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[Camillo Bruni, 2016-04-15 13:50:30]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-15 13:50:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/500001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/500001

[commit-bot: I haz the power, 2016-04-15 14:23:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-15 14:23:08]

Dry run: This issue passed the CQ dry run.

[Camillo Bruni, 2016-05-06 09:28:03]

Description was changed from

==========
[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat
Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail. This
CL adds a fix to globally invalidate a isConcatSpreadable_protector.

LOG=N
BUG=chromium:542504, v8:903
==========

to

==========
[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat

Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail.
This CL adds a fix to globally invalidate a isConcatSpreadable_protector.

Drive-by-fix: use named accessors for context variables

LOG=N
BUG=chromium:542504, v8:903
==========

[Camillo Bruni, 2016-05-06 10:48:47]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-06 10:48:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/560001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/560001

[commit-bot: I haz the power, 2016-05-06 11:19:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-06 11:19:51]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2016-05-11 13:01:02]

lgtm

[Camillo Bruni, 2016-05-11 13:02:08]

The CQ bit was checked by cbruni@chromium.org

[Camillo Bruni, 2016-05-11 13:02:09]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org
Link to the patchset: https://codereview.chromium.org/1409123003/#ps560001
(title: "reverting accidental changes")

[commit-bot: I haz the power, 2016-05-11 13:02:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/560001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/560001

[commit-bot: I haz the power, 2016-05-11 13:03:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-11 13:03:55]

Try jobs failed on following builders:
  v8_linux64_rel_ng on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/5602)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)

[Camillo Bruni, 2016-05-11 13:44:44]

The CQ bit was checked by cbruni@chromium.org

[Camillo Bruni, 2016-05-11 13:44:45]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org,
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/1409123003/#ps580001
(title: "merging with master")

[commit-bot: I haz the power, 2016-05-11 13:44:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/580001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/580001

[commit-bot: I haz the power, 2016-05-11 15:45:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-11 15:45:26]

Try jobs failed on following builders:
  v8_linux_arm_rel_ng on tryserver.v8 (JOB_TIMED_OUT, no build URL)

[Camillo Bruni, 2016-05-12 08:27:23]

The CQ bit was checked by cbruni@chromium.org

[commit-bot: I haz the power, 2016-05-12 08:27:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1409123003/580001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1409123003/580001

[commit-bot: I haz the power, 2016-05-12 08:50:27]

Committed patchset #31 (id:580001)

[commit-bot: I haz the power, 2016-05-12 08:52:45]

Description was changed from

==========
[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat

Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail.
This CL adds a fix to globally invalidate a isConcatSpreadable_protector.

Drive-by-fix: use named accessors for context variables

LOG=N
BUG=chromium:542504, v8:903
==========

to

==========
[runtime] Avoid @@isConcatSpreadable lookup for fast path Array.prototype.concat

Currently we do not check for @@isConcatSpreadable properly. If the Symbol is
set on the Array.prototype or Object.prototype the current fast paths fail.
This CL adds a fix to globally invalidate a isConcatSpreadable_protector.

Drive-by-fix: use named accessors for context variables

LOG=N
BUG=chromium:542504, v8:903

Committed: https://crrev.com/f87014ebdefc759ff58b7c9557c417e0f57749ba
Cr-Commit-Position: refs/heads/master@{#36201}
==========

[commit-bot: I haz the power, 2016-05-12 08:52:47]

Patchset 31 (id:??) landed as
https://crrev.com/f87014ebdefc759ff58b7c9557c417e0f57749ba
Cr-Commit-Position: refs/heads/master@{#36201}