Array.prototype.map write error.

Array.prototype.map write error.

More care must be taken to remain on the fast path in the face of
@@species constructors.

BUG=chromium:716044
Review-Url: https://codereview.chromium.org/2846963003
Cr-Commit-Position: refs/heads/master@{#45065}
Committed: https://chromium.googlesource.com/v8/v8/+/192984ea88badc0c02e22e528b1243a9efa46f90

[mvstanton, 2017-04-28 13:49:17]

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

[commit-bot: I haz the power, 2017-04-28 13:49:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mvstanton, 2017-04-28 13:51:21]

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

[commit-bot: I haz the power, 2017-04-28 13:51:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mvstanton1, 2017-04-28 13:53:27]

mvstanton@google.com changed reviewers:
+ bmeurer@chromium.org, danno@chromium.org, mvstanton@google.com

[mvstanton1, 2017-04-28 13:53:28]

Hi Benedikt, hi Danno,

Here is the fix for the map issue we've discussed.
Map() is exposed to the bug because it makes an assumption about the length of
the output array (which it tried to set to be equal to that of the input array).

Thanks,
--Michael

[commit-bot: I haz the power, 2017-04-28 13:58:00]

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

[commit-bot: I haz the power, 2017-04-28 13:58:01]

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

[Benedikt Meurer, 2017-04-28 17:13:37]

Looks good as ducktape, but middle-term we should find a better way to deal with
the @@species case.

[mvstanton, 2017-04-29 07:25:17]

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

[commit-bot: I haz the power, 2017-04-29 07:25:26]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-29 07:55:01]

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

[commit-bot: I haz the power, 2017-04-29 07:55:02]

Dry run: This issue passed the CQ dry run.

[danno, 2017-04-29 09:27:14]

https://codereview.chromium.org/2846963003/diff/40001/src/builtins/builtins-a...
File src/builtins/builtins-array-gen.cc (right):

https://codereview.chromium.org/2846963003/diff/40001/src/builtins/builtins-a...
src/builtins/builtins-array-gen.cc:224: force_slow_.Bind(SmiConstant(1));
Would it be a little cleaner to just jump to the slow full-spec continuation
here instead? Perhaps just rename the slow label fully_spec_compliant_ and keep
it as a member on the builtin assembler. Then you can just goto it here, and you
don't have to add the force_slow_ boolean.

https://codereview.chromium.org/2846963003/diff/40001/src/builtins/builtins-a...
src/builtins/builtins-array-gen.cc:227: BIND(&done);
I am a little surprised this works... you change the value of not only the
species constructor variable but also the force_slow_ variable, but that
variable is not included as a merged variable on "done". You will need a phi
node for that, too, right? I think the way it's currently written you "force
slow" in all cases, since it's the last bound value before using it.

[mvstanton, 2017-05-01 06:29:44]

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

[commit-bot: I haz the power, 2017-05-01 06:29:49]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-01 06:31:18]

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

[commit-bot: I haz the power, 2017-05-01 06:31:18]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/25156)
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/3...)

[mvstanton, 2017-05-01 07:08:21]

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

[commit-bot: I haz the power, 2017-05-01 07:08:25]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-01 07:09:55]

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

[commit-bot: I haz the power, 2017-05-01 07:09:56]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/25249)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/25157)
  v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/40183)
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/3...)

[mvstanton, 2017-05-02 09:53:51]

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

[commit-bot: I haz the power, 2017-05-02 09:54:03]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-02 10:24:12]

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

[commit-bot: I haz the power, 2017-05-02 10:24:12]

Dry run: This issue passed the CQ dry run.

[mvstanton, 2017-05-02 10:39:52]

Hi Danno, have another look based on our discussion, thanks!

[danno, 2017-05-02 10:58:31]

lgtm with comments

https://codereview.chromium.org/2846963003/diff/100001/src/builtins/builtins-...
File src/builtins/builtins-array-gen.cc (right):

https://codereview.chromium.org/2846963003/diff/100001/src/builtins/builtins-...
src/builtins/builtins-array-gen.cc:21: // A jump to label {slow_elements} allows
the result generator to
Is this label correct?

https://codereview.chromium.org/2846963003/diff/100001/src/builtins/builtins-...
src/builtins/builtins-array-gen.cc:165: void MapResultGenerator() {
Perhaps call this FastMapResultGenerator?

https://codereview.chromium.org/2846963003/diff/100001/src/builtins/builtins-...
src/builtins/builtins-array-gen.cc:221: Label fast(this);
nit: Any particular reason for this change?

[mvstanton, 2017-05-03 12:19:05]

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

[mvstanton, 2017-05-03 12:19:12]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2017-05-03 12:19:13]

The patchset sent to the CQ was uploaded after l-g-t-m from danno@chromium.org
Link to the patchset: https://codereview.chromium.org/2846963003/#ps120001
(title: "REBASE/nits.")

[commit-bot: I haz the power, 2017-05-03 12:19:18]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mvstanton, 2017-05-03 12:19:35]

mvstanton@chromium.org changed reviewers:
- mvstanton@google.com

[mvstanton, 2017-05-03 12:19:36]

Thanks! Submitting.

https://codereview.chromium.org/2846963003/diff/100001/src/builtins/builtins-...
File src/builtins/builtins-array-gen.cc (right):

https://codereview.chromium.org/2846963003/diff/100001/src/builtins/builtins-...
src/builtins/builtins-array-gen.cc:21: // A jump to label {slow_elements} allows
the result generator to
On 2017/05/02 10:58:31, danno wrote:
> Is this label correct?

Outdated comment, thanks! Done.

https://codereview.chromium.org/2846963003/diff/100001/src/builtins/builtins-...
src/builtins/builtins-array-gen.cc:165: void MapResultGenerator() {
On 2017/05/02 10:58:31, danno wrote:
> Perhaps call this FastMapResultGenerator?

Done.

https://codereview.chromium.org/2846963003/diff/100001/src/builtins/builtins-...
src/builtins/builtins-array-gen.cc:221: Label fast(this);
On 2017/05/02 10:58:31, danno wrote:
> nit: Any particular reason for this change?

Nope, just a rebase thing - I've restored the original.

[mvstanton, 2017-05-03 12:31:15]

The CQ bit was unchecked by mvstanton@chromium.org

[mvstanton, 2017-05-03 12:34:23]

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

[commit-bot: I haz the power, 2017-05-03 12:34:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mvstanton, 2017-05-03 12:34:30]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2017-05-03 12:34:30]

The patchset sent to the CQ was uploaded after l-g-t-m from danno@chromium.org
Link to the patchset: https://codereview.chromium.org/2846963003/#ps140001
(title: "Better nit repair!")

[commit-bot: I haz the power, 2017-05-03 12:34:40]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-03 12:39:23]

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

[commit-bot: I haz the power, 2017-05-03 12:39:25]

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

[mvstanton, 2017-05-03 14:06:01]

The CQ bit was checked by mvstanton@chromium.org

[commit-bot: I haz the power, 2017-05-03 14:06:08]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-03 14:11:45]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1493820361501110,
"parent_rev": "40d01184a18a599392691c0eb931720628a44e80", "commit_rev":
"192984ea88badc0c02e22e528b1243a9efa46f90"}

[commit-bot: I haz the power, 2017-05-03 14:11:50]

Description was changed from

==========
Array.prototype.map write error.

More care must be taken to remain on the fast path in the face of
@@species constructors.

BUG=chromium:716044
==========

to

==========
Array.prototype.map write error.

More care must be taken to remain on the fast path in the face of
@@species constructors.

BUG=chromium:716044

Review-Url: https://codereview.chromium.org/2846963003
Cr-Commit-Position: refs/heads/master@{#45065}
Committed:
https://chromium.googlesource.com/v8/v8/+/192984ea88badc0c02e22e528b1243a9efa...
==========

[commit-bot: I haz the power, 2017-05-03 14:11:51]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/v8/v8/+/192984ea88badc0c02e22e528b1243a9efa...