[turbofan] Fix ASAN bug in escape analysis

[turbofan] Fix ASAN bug in escape analysis

BUG=566253
LOG=n

Committed: https://crrev.com/07cc8d598b155e5c59585f544e190c38f4ba8b2e
Cr-Commit-Position: refs/heads/master@{#32942}

[sigurds, 2015-12-16 13:43:33]

Description was changed from

==========
[turbofan] Fix ASAN bug in escape analysis

BUG=566253
LOG=n
==========

to

==========
[turbofan] Fix ASAN bug in escape analysis

BUG=566253
LOG=n
==========

[sigurds, 2015-12-16 13:43:33]

sigurds@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[sigurds, 2015-12-16 14:25:51]

sigurds@chromium.org changed reviewers:
+ bmeurer@chromium.org
- mstarzinger@chromium.org

[sigurds, 2015-12-16 14:26:12]

PTAL, this fixes a clusterfuzz issue.

[Benedikt Meurer, 2015-12-16 17:53:56]

https://codereview.chromium.org/1530143002/diff/1/src/compiler/escape-analysi...
File src/compiler/escape-analysis.cc (right):

https://codereview.chromium.org/1530143002/diff/1/src/compiler/escape-analysi...
src/compiler/escape-analysis.cc:457: info_.resize(graph()->NodeCount());
How about

if (node->id() >= info_.size()) return false;

as in IsVirtual below?

https://codereview.chromium.org/1530143002/diff/1/src/compiler/escape-analysi...
src/compiler/escape-analysis.cc:471: return info_[node->id()] == kEscaped;
Isn't this subject to the same issue?

[sigurds, 2015-12-17 10:16:33]

Please take a look again :)

https://codereview.chromium.org/1530143002/diff/1/src/compiler/escape-analysi...
File src/compiler/escape-analysis.cc (right):

https://codereview.chromium.org/1530143002/diff/1/src/compiler/escape-analysi...
src/compiler/escape-analysis.cc:457: info_.resize(graph()->NodeCount());
On 2015/12/16 17:53:56, Benedikt Meurer wrote:
> How about
> 
> if (node->id() >= info_.size()) return false;
> 
> as in IsVirtual below?

I want to get rid of the checks here, see comments in new patchset.

https://codereview.chromium.org/1530143002/diff/1/src/compiler/escape-analysi...
src/compiler/escape-analysis.cc:471: return info_[node->id()] == kEscaped;
On 2015/12/16 17:53:56, Benedikt Meurer wrote:
> Isn't this subject to the same issue?

See comments in new patch set.

https://codereview.chromium.org/1530143002/diff/20001/src/compiler/escape-ana...
File src/compiler/escape-analysis.cc (left):

https://codereview.chromium.org/1530143002/diff/20001/src/compiler/escape-ana...
src/compiler/escape-analysis.cc:463: return false;
I'm removing this check here, because the class has the invariant that info_ has
entries for all nodes in the graph (even phis that get created by the analysis).

https://codereview.chromium.org/1530143002/diff/20001/src/compiler/escape-ana...
File src/compiler/escape-analysis.cc (right):

https://codereview.chromium.org/1530143002/diff/20001/src/compiler/escape-ana...
src/compiler/escape-analysis.cc:1079: return false;
IsVirtual and IsEscaped below are public methods that may get called after the
graph has been further modified, hence the check is needed here.

[Benedikt Meurer, 2015-12-17 10:30:22]

LGTM

[sigurds, 2015-12-17 10:58:25]

The CQ bit was checked by sigurds@chromium.org

[commit-bot: I haz the power, 2015-12-17 10:58:37]

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

[commit-bot: I haz the power, 2015-12-17 11:21:51]

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

[commit-bot: I haz the power, 2015-12-17 11:21:52]

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

[sigurds, 2015-12-17 11:29:07]

The CQ bit was checked by sigurds@chromium.org

[sigurds, 2015-12-17 11:29:07]

The patchset sent to the CQ was uploaded after l-g-t-m from bmeurer@chromium.org
Link to the patchset: https://codereview.chromium.org/1530143002/#ps40001
(title: "Don't write code in DCHECK :)")

[commit-bot: I haz the power, 2015-12-17 11:29:22]

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

[commit-bot: I haz the power, 2015-12-17 11:46:49]

Description was changed from

==========
[turbofan] Fix ASAN bug in escape analysis

BUG=566253
LOG=n
==========

to

==========
[turbofan] Fix ASAN bug in escape analysis

BUG=566253
LOG=n
==========

[commit-bot: I haz the power, 2015-12-17 11:46:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-17 11:47:24]

Description was changed from

==========
[turbofan] Fix ASAN bug in escape analysis

BUG=566253
LOG=n
==========

to

==========
[turbofan] Fix ASAN bug in escape analysis

BUG=566253
LOG=n

Committed: https://crrev.com/07cc8d598b155e5c59585f544e190c38f4ba8b2e
Cr-Commit-Position: refs/heads/master@{#32942}
==========

[commit-bot: I haz the power, 2015-12-17 11:47:24]

Patchset 3 (id:??) landed as
https://crrev.com/07cc8d598b155e5c59585f544e190c38f4ba8b2e
Cr-Commit-Position: refs/heads/master@{#32942}