[left-trimming] Avoid creating duplicate handles in builtins.cc

[left-trimming] Avoid creating duplicate handles in builtins.cc
EnsureJSArrayWithWritableFastElements

Having several handles pointing to the backing store of an array that gets
left-trimmed might cause the gc to start marking a stale-handle still pointing
to the old backing-store start. By introducing a separate handle scope for
EnsureJSArrayWithWritableFastElements we avoid this issue. Additionally a
SLOW_DCHECK in Heap::LeftTrimFixedArray ensurse that there are no more than one
active handle pointing to the backing store.

BUG=chr:585787
LOG=n

Committed: https://crrev.com/6dd16e8eb1440dcce91062ba92b8d1c3df3a121b
Cr-Commit-Position: refs/heads/master@{#34022}

[Camillo Bruni, 2016-02-15 15:29:51]

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

[commit-bot: I haz the power, 2016-02-15 15:29:57]

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

[Camillo Bruni, 2016-02-15 15:30:09]

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

[Camillo Bruni, 2016-02-15 15:30:09]

Hi Ulan, PTAL the proposed fix.

[commit-bot: I haz the power, 2016-02-15 15:32:55]

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

[commit-bot: I haz the power, 2016-02-15 15:32:56]

Dry run: 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/1393)
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/1...)
  v8_linux_arm_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/13777)
  v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)

[ulan, 2016-02-15 15:33:09]

One comments, otherwise looks good.

https://codereview.chromium.org/1699733003/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1699733003/diff/1/src/builtins.cc#newcode270
src/builtins.cc:270: if (args == NULL || array->HasFastObjectElements()) return
elms;
I think CloseAndEscape should be used here and in other return paths.

[Camillo Bruni, 2016-02-15 17:16:50]

On 2016/02/15 at 15:33:09, ulan wrote:
> One comments, otherwise looks good.
> 
> https://codereview.chromium.org/1699733003/diff/1/src/builtins.cc
> File src/builtins.cc (right):
> 
> https://codereview.chromium.org/1699733003/diff/1/src/builtins.cc#newcode270
> src/builtins.cc:270: if (args == NULL || array->HasFastObjectElements())
return elms;
> I think CloseAndEscape should be used here and in other return paths.

doh! right :D

[Camillo Bruni, 2016-02-15 17:26:38]

ptal again

[ulan, 2016-02-15 17:28:34]

lgtm!

[Camillo Bruni, 2016-02-16 09:20:46]

The CQ bit was checked by cbruni@chromium.org

[commit-bot: I haz the power, 2016-02-16 09:20:50]

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

[commit-bot: I haz the power, 2016-02-16 09:50:34]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-16 09:51:12]

Description was changed from

==========
[left-trimming] Avoid creating duplicate handles in builtins.cc
EnsureJSArrayWithWritableFastElements

Having several handles pointing to the backing store of an array that gets
left-trimmed might cause the gc to start marking a stale-handle still pointing
to the old backing-store start. By introducing a separate handle scope for
EnsureJSArrayWithWritableFastElements we avoid this issue. Additionally a
SLOW_DCHECK in Heap::LeftTrimFixedArray ensurse that there are no more than one
active handle pointing to the backing store.

BUG=chr:585787
LOG=n
==========

to

==========
[left-trimming] Avoid creating duplicate handles in builtins.cc
EnsureJSArrayWithWritableFastElements

Having several handles pointing to the backing store of an array that gets
left-trimmed might cause the gc to start marking a stale-handle still pointing
to the old backing-store start. By introducing a separate handle scope for
EnsureJSArrayWithWritableFastElements we avoid this issue. Additionally a
SLOW_DCHECK in Heap::LeftTrimFixedArray ensurse that there are no more than one
active handle pointing to the backing store.

BUG=chr:585787
LOG=n

Committed: https://crrev.com/6dd16e8eb1440dcce91062ba92b8d1c3df3a121b
Cr-Commit-Position: refs/heads/master@{#34022}
==========

[commit-bot: I haz the power, 2016-02-16 09:51:13]

Patchset 2 (id:??) landed as
https://crrev.com/6dd16e8eb1440dcce91062ba92b8d1c3df3a121b
Cr-Commit-Position: refs/heads/master@{#34022}