[wasm] Add ProtectedStore instruction

[wasm] Add ProtectedStore instruction

This is necessary for signal-based out of bounds handling in WebAssembly.

Adds a ProtectedStore instruction that is analogous to the previously added
ProtectedLoad instruction. Rather than using bounds checks, ProtectedStore emits
an out of line section of code that throws a JavaScript exception and provides
the necessary metadata for a signal handler to be able to find the out of line
code.

BUG= https://bugs.chromium.org/p/v8/issues/detail?id=5277

Committed: https://crrev.com/4ab4bbe9b16a47a16b4f8d6bf17eb77eadbb139d
Cr-Commit-Position: refs/heads/master@{#41398}

[Eric Holk, 2016-11-22 01:36:00]

Description was changed from

==========
[wasm] add ProtectedStore instruction

This is necessary for signal-based out of bounds handling in WebAssembly.

Adds a ProtectedStore instruction that is analogous to the previously added
ProtectedLoad instruction. Rather than using bounds checks, ProtectedStore emits
an out of line section of code that throws a JavaScript exception and provides
the necessary metadata for a signal handler to be able to find the out of line
code.

BUG= https://bugs.chromium.org/p/v8/issues/detail?id=5277
==========

to

==========
[wasm] Add ProtectedStore instruction

This is necessary for signal-based out of bounds handling in WebAssembly.

Adds a ProtectedStore instruction that is analogous to the previously added
ProtectedLoad instruction. Rather than using bounds checks, ProtectedStore emits
an out of line section of code that throws a JavaScript exception and provides
the necessary metadata for a signal handler to be able to find the out of line
code.

BUG= https://bugs.chromium.org/p/v8/issues/detail?id=5277
==========

[Eric Holk, 2016-11-22 01:42:27]

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

[commit-bot: I haz the power, 2016-11-22 01:42:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Eric Holk, 2016-11-22 01:44:34]

eholk@chromium.org changed reviewers:
+ bradnelson@chromium.org, titzer@chromium.org

[Eric Holk, 2016-11-22 01:45:36]

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

[commit-bot: I haz the power, 2016-11-22 01:45:44]

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, 2016-11-22 01:50:14]

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

[commit-bot: I haz the power, 2016-11-22 01:50:15]

Dry run: Try jobs failed on following builders:
  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...)

[bradnelson, 2016-11-22 02:02:36]

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.cc:3012: if (!FLAG_wasm_trap_handler) {
The fuzzers are going to throw up a lot of issues if this doesn't fail
gracefully with this flag on.

Don't we need to gate all of this off in the case the arch doesn't support the
handler?

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.cc:3040: DCHECK(!FLAG_wasm_trap_handler);
A TODO to support this?

[titzer, 2016-11-22 10:33:20]

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.cc:3012: if (!FLAG_wasm_trap_handler) {
On 2016/11/22 02:02:36, bradnelson wrote:
> The fuzzers are going to throw up a lot of issues if this doesn't fail
> gracefully with this flag on.
> 
> Don't we need to gate all of this off in the case the arch doesn't support the
> handler?

I think we should (eventually) implement ProtectedLoad and ProtectedStore on all
architectures, because it might be useful even on 32-bit for another purpose
(e.g. stack checks).

So we need to add a configuration mechanism for whether protected memories are
supported on the platform.

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/x64/instru...
File src/compiler/x64/instruction-selector-x64.cc (right):

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:187: DCHECK(!protect);
We'll need to support all of these types with protected loads and stores.

Maybe we should encode whether the load/store can trap with another bit in the
instruction opcode?

[Eric Holk, 2016-11-22 23:16:18]

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

[commit-bot: I haz the power, 2016-11-22 23:16:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Eric Holk, 2016-11-22 23:16:25]

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/wasm-compi...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.cc:3012: if (!FLAG_wasm_trap_handler) {
On 2016/11/22 10:33:20, titzer wrote:
> On 2016/11/22 02:02:36, bradnelson wrote:
> > The fuzzers are going to throw up a lot of issues if this doesn't fail
> > gracefully with this flag on.
> > 
> > Don't we need to gate all of this off in the case the arch doesn't support
the
> > handler?
> 
> I think we should (eventually) implement ProtectedLoad and ProtectedStore on
all
> architectures, because it might be useful even on 32-bit for another purpose
> (e.g. stack checks).
> 
> So we need to add a configuration mechanism for whether protected memories are
> supported on the platform.

I actually added something similar in the guard pages CL. I changed it to do the
same here.

Done.

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/wasm-compi...
src/compiler/wasm-compiler.cc:3040: DCHECK(!FLAG_wasm_trap_handler);
On 2016/11/22 02:02:36, bradnelson wrote:
> A TODO to support this?

Done, and for ProtectedLoad.

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/x64/instru...
File src/compiler/x64/instruction-selector-x64.cc (right):

https://codereview.chromium.org/2516413003/diff/40001/src/compiler/x64/instru...
src/compiler/x64/instruction-selector-x64.cc:187: DCHECK(!protect);
On 2016/11/22 10:33:20, titzer wrote:
> We'll need to support all of these types with protected loads and stores.
> 
> Maybe we should encode whether the load/store can trap with another bit in the
> instruction opcode?

I think this is a better idea. It looks like I can put it in the misc field.

[commit-bot: I haz the power, 2016-11-22 23:20:51]

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

[commit-bot: I haz the power, 2016-11-22 23:20:52]

Dry run: Try jobs failed on following builders:
  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...)

[Eric Holk, 2016-11-22 23:34:11]

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

[commit-bot: I haz the power, 2016-11-22 23:34:21]

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, 2016-11-22 23:59:11]

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

[commit-bot: I haz the power, 2016-11-22 23:59:12]

Dry run: This issue passed the CQ dry run.

[Eric Holk, 2016-11-30 17:11:16]

ping

[titzer, 2016-11-30 17:23:23]

lgtm

https://codereview.chromium.org/2516413003/diff/120001/src/compiler/x64/code-...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/2516413003/diff/120001/src/compiler/x64/code-...
src/compiler/x64/code-generator-x64.cc:305: void MaybeEmitOOLTrap(Zone* zone,
CodeGenerator* codegen,
EmitOOLTrapIfNeeded?

[Eric Holk, 2016-11-30 18:58:59]

The CQ bit was checked by eholk@chromium.org

[Eric Holk, 2016-11-30 18:58:59]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/2516413003/#ps160001
(title: "Merging with master")

[commit-bot: I haz the power, 2016-11-30 18:59:06]

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

[Eric Holk, 2016-11-30 18:59:08]

Thanks!

https://codereview.chromium.org/2516413003/diff/120001/src/compiler/x64/code-...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/2516413003/diff/120001/src/compiler/x64/code-...
src/compiler/x64/code-generator-x64.cc:305: void MaybeEmitOOLTrap(Zone* zone,
CodeGenerator* codegen,
On 2016/11/30 17:23:23, titzer wrote:
> EmitOOLTrapIfNeeded?

Done.

[commit-bot: I haz the power, 2016-11-30 19:51:14]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1480532339143850,
"parent_rev": "51c40417edb519d64873945fa859aff3af9f2802", "commit_rev":
"1e063993c6bb488c974c6d318f0eddec7da84d7f"}

[commit-bot: I haz the power, 2016-11-30 19:51:21]

Description was changed from

==========
[wasm] Add ProtectedStore instruction

This is necessary for signal-based out of bounds handling in WebAssembly.

Adds a ProtectedStore instruction that is analogous to the previously added
ProtectedLoad instruction. Rather than using bounds checks, ProtectedStore emits
an out of line section of code that throws a JavaScript exception and provides
the necessary metadata for a signal handler to be able to find the out of line
code.

BUG= https://bugs.chromium.org/p/v8/issues/detail?id=5277
==========

to

==========
[wasm] Add ProtectedStore instruction

This is necessary for signal-based out of bounds handling in WebAssembly.

Adds a ProtectedStore instruction that is analogous to the previously added
ProtectedLoad instruction. Rather than using bounds checks, ProtectedStore emits
an out of line section of code that throws a JavaScript exception and provides
the necessary metadata for a signal handler to be able to find the out of line
code.

BUG= https://bugs.chromium.org/p/v8/issues/detail?id=5277
==========

[commit-bot: I haz the power, 2016-11-30 19:51:23]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-11-30 19:52:06]

Description was changed from

==========
[wasm] Add ProtectedStore instruction

This is necessary for signal-based out of bounds handling in WebAssembly.

Adds a ProtectedStore instruction that is analogous to the previously added
ProtectedLoad instruction. Rather than using bounds checks, ProtectedStore emits
an out of line section of code that throws a JavaScript exception and provides
the necessary metadata for a signal handler to be able to find the out of line
code.

BUG= https://bugs.chromium.org/p/v8/issues/detail?id=5277
==========

to

==========
[wasm] Add ProtectedStore instruction

This is necessary for signal-based out of bounds handling in WebAssembly.

Adds a ProtectedStore instruction that is analogous to the previously added
ProtectedLoad instruction. Rather than using bounds checks, ProtectedStore emits
an out of line section of code that throws a JavaScript exception and provides
the necessary metadata for a signal handler to be able to find the out of line
code.

BUG= https://bugs.chromium.org/p/v8/issues/detail?id=5277

Committed: https://crrev.com/4ab4bbe9b16a47a16b4f8d6bf17eb77eadbb139d
Cr-Commit-Position: refs/heads/master@{#41398}
==========

[commit-bot: I haz the power, 2016-11-30 19:52:07]

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