[interpreter] Remove BytecodeArrayBuilder::Illegal().

[interpreter] Remove BytecodeArrayBuilder::Illegal().

BUG=v8:4280
LOG=N

Committed: https://crrev.com/1818a2f278fcf09f2cc7c64297b5adcf907e281f
Cr-Commit-Position: refs/heads/master@{#36249}

[oth, 2016-05-13 09:03:15]

oth@chromium.org changed reviewers:
+ rmcilroy@chromium.org, verwaest@chromium.org

[oth, 2016-05-13 09:03:15]

Ross, follow on from the peephole optimizer change. PTAL.

Toon, this change adds a new error to signify an index into a jump table is
invalid. If we hit this, execution has deviated from all expected paths. WDYT?

Thanks

[Toon Verwaest, 2016-05-13 10:54:34]

Don't you just want to crash (e.g., PushStackTraceAndDie) in that case? By
crashing you can get this feedback through chromecrash, rather than just
randomly getting (probably catchable) userland exceptions in the wild. At least
I very much prefer this over "illegal access" exceptions.

[rmcilroy, 2016-05-13 10:57:00]

On 2016/05/13 10:54:34, Toon Verwaest wrote:
> Don't you just want to crash (e.g., PushStackTraceAndDie) in that case? By
> crashing you can get this feedback through chromecrash, rather than just
> randomly getting (probably catchable) userland exceptions in the wild. At
least
> I very much prefer this over "illegal access" exceptions.

+1 this should be a call to Abort with a new bailout reason specifying the
reason for the crash.

[oth, 2016-05-13 11:39:09]

On 2016/05/13 10:57:00, rmcilroy wrote:
> On 2016/05/13 10:54:34, Toon Verwaest wrote:
> > Don't you just want to crash (e.g., PushStackTraceAndDie) in that case? By
> > crashing you can get this feedback through chromecrash, rather than just
> > randomly getting (probably catchable) userland exceptions in the wild. At
> least
> > I very much prefer this over "illegal access" exceptions.
> 
> +1 this should be a call to Abort with a new bailout reason specifying the
> reason for the crash.

Aha, got it. Thanks both. Revised to call Abort, much better.

[rmcilroy, 2016-05-13 12:41:27]

Could you also update bytecode-graph-builder to have VisitIllegal being
UNREACHABLE.

After that, lgtm

https://codereview.chromium.org/1979523002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/1979523002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-array-builder.h:339: 
We should just remove the function, right?

https://codereview.chromium.org/1979523002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1979523002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:659: RegisterAllocationScope
register_scope(this);
Nit - put this in its own block.

[rmcilroy, 2016-05-13 12:41:28]

Could you also update bytecode-graph-builder to have VisitIllegal being
UNREACHABLE.

After that, lgtm

[oth, 2016-05-13 15:26:48]

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

[oth, 2016-05-13 15:26:48]

mstarzinger@chromium.org: Please review changes in compiler making the illegal
bytecode unreachable in the graph builder. The intention is we never emit the
illegal bytecode in legal bytecode :-D.

[oth, 2016-05-13 15:28:23]

+cc neis

[Michael Starzinger, 2016-05-13 15:35:08]

LGTM.

[oth, 2016-05-13 15:59:34]

The CQ bit was checked by oth@chromium.org

[oth, 2016-05-13 15:59:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1979523002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-05-13 15:59:38]

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

[commit-bot: I haz the power, 2016-05-13 16:01:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-13 16:01:53]

Description was changed from

==========
[interpreter] Remove BytecodeArrayBuilder::Illegal().

BUG=v8:4280
LOG=N
==========

to

==========
[interpreter] Remove BytecodeArrayBuilder::Illegal().

BUG=v8:4280
LOG=N

Committed: https://crrev.com/1818a2f278fcf09f2cc7c64297b5adcf907e281f
Cr-Commit-Position: refs/heads/master@{#36249}
==========

[commit-bot: I haz the power, 2016-05-13 16:01:55]

Patchset 4 (id:??) landed as
https://crrev.com/1818a2f278fcf09f2cc7c64297b5adcf907e281f
Cr-Commit-Position: refs/heads/master@{#36249}