[wasm] Add guard regions to end of WebAssembly.Memory buffers

[wasm] Add guard regions to end of WebAssembly.Memory buffers

With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is now implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

Committed: https://crrev.com/eaed31c5f59abfb146c3ca82059ca1c41b1729b9
Cr-Commit-Position: refs/heads/master@{#41089}

[Eric Holk, 2016-10-08 00:15:43]

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

[commit-bot: I haz the power, 2016-10-08 00:15:56]

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-10-08 00:18:44]

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

[commit-bot: I haz the power, 2016-10-08 00:18:45]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/14212)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/14158)

[Eric Holk, 2016-10-08 00:53:56]

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

[commit-bot: I haz the power, 2016-10-08 00:54:05]

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-10-08 01:00:14]

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

[commit-bot: I haz the power, 2016-10-08 01:00:15]

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

[Eric Holk, 2016-10-12 01:02:56]

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

[commit-bot: I haz the power, 2016-10-12 01:03:07]

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-10-12 01:06:17]

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

[commit-bot: I haz the power, 2016-10-12 01:06:18]

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

[Eric Holk, 2016-10-13 18:40:36]

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

[commit-bot: I haz the power, 2016-10-13 18:40:45]

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-10-13 18:43:36]

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

[commit-bot: I haz the power, 2016-10-13 18:43:37]

Dry run: Try jobs failed on following builders:
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/14562)
  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_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/14511)

[Eric Holk, 2016-10-19 15:42:55]

Description was changed from

==========
WIP: allocate guard regions around Wasm memory

BUG=
==========

to

==========
WIP: allocate guard regions around Wasm memory

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

[Eric Holk, 2016-10-19 15:51:38]

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

[commit-bot: I haz the power, 2016-10-19 15:51:48]

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-10-19 16:57:03]

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

[commit-bot: I haz the power, 2016-10-19 16:57:03]

Dry run: Try jobs failed on following builders:
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/14854)
  v8_linux_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng_triggered/b...)

[Eric Holk, 2016-10-19 17:42:06]

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

[commit-bot: I haz the power, 2016-10-19 17:42:07]

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-10-19 18:18:20]

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

[commit-bot: I haz the power, 2016-10-19 18:18:21]

Dry run: This issue passed the CQ dry run.

[Eric Holk, 2016-10-26 18:04:17]

Description was changed from

==========
WIP: allocate guard regions around Wasm memory

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

to

==========
With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is not implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

This change no longer works with passing an ArrayBuffer to
WebAssembly.Instantiate. Instead you must pass in a legitimate
WebAssembly.Memory.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

[Eric Holk, 2016-10-26 18:12:42]

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

[Michael Lippautz, 2016-10-27 14:41:50]

JSArrayBuffer usage looks good to me. Deferring the actual review to the wasm
folks.

https://codereview.chromium.org/2396433008/diff/260001/src/base/platform/plat...
File src/base/platform/platform-posix.cc (right):

https://codereview.chromium.org/2396433008/diff/260001/src/base/platform/plat...
src/base/platform/platform-posix.cc:133: void OS::Unprotect(void* address, const
size_t size) {
Maybe we should have OS::Protect as a dual function to OS::Unprotect. 
You could keep OS::Guard to just call OS:Protect.

https://codereview.chromium.org/2396433008/diff/260001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/2396433008/diff/260001/src/objects-inl.h#newc...
src/objects-inl.h:6973: set_bit_field(IsExternal::update(bit_field(), value));
Please add a
  DCHECK(!value || !has_guard_region())
so that we can never internalize buffers that have been allocated using an
allocator different from 
  isolate->array_buffer_allocator()

[Eric Holk, 2016-10-28 16:20:14]

https://codereview.chromium.org/2396433008/diff/260001/src/base/platform/plat...
File src/base/platform/platform-posix.cc (right):

https://codereview.chromium.org/2396433008/diff/260001/src/base/platform/plat...
src/base/platform/platform-posix.cc:133: void OS::Unprotect(void* address, const
size_t size) {
On 2016/10/27 14:41:50, Michael Lippautz wrote:
> Maybe we should have OS::Protect as a dual function to OS::Unprotect. 
> You could keep OS::Guard to just call OS:Protect.

I think I'd probably do OS::Protect and take flags for read, write or execute to
have it basically mirror mprotect or VirtualProtect.

https://codereview.chromium.org/2396433008/diff/260001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/2396433008/diff/260001/src/objects-inl.h#newc...
src/objects-inl.h:6973: set_bit_field(IsExternal::update(bit_field(), value));
On 2016/10/27 14:41:50, Michael Lippautz wrote:
> Please add a
>   DCHECK(!value || !has_guard_region())
> so that we can never internalize buffers that have been allocated using an
> allocator different from 
>   isolate->array_buffer_allocator()

Done.

[titzer, 2016-10-28 16:24:57]

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-js.cc#ne...
src/wasm/wasm-js.cc:235: memory = i::WasmJs::GetWasmMemoryArrayBuffer(i_isolate,
mem_obj);
Can we split out the WebAssembly.Memory requirement (and changes to tests) into
another CL? That'd be easy to land first.

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.h...
src/wasm/wasm-module.h:39: #if V8_HOST_ARCH_64_BIT
Any reason not to just define this on all architectures (as an int64_t)?

https://codereview.chromium.org/2396433008/diff/300001/tools/testrunner/local...
File tools/testrunner/local/variants.py (right):

https://codereview.chromium.org/2396433008/diff/300001/tools/testrunner/local...
tools/testrunner/local/variants.py:17: "wasm_traps": [["--wasm_guard_pages",
"--invoke-weak-callbacks"]],
Not sure if this will overload the bots. Maybe loop in machenbach@?

[Eric Holk, 2016-10-28 17:08:54]

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

[commit-bot: I haz the power, 2016-10-28 17:09:00]

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-10-28 17:10:08]

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

[commit-bot: I haz the power, 2016-10-28 17:10:19]

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-10-28 17:13:29]

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

[commit-bot: I haz the power, 2016-10-28 17:13:30]

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_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...)

[Eric Holk, 2016-10-28 18:44:40]

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-js.cc#ne...
src/wasm/wasm-js.cc:235: memory = i::WasmJs::GetWasmMemoryArrayBuffer(i_isolate,
mem_obj);
On 2016/10/28 16:24:57, titzer wrote:
> Can we split out the WebAssembly.Memory requirement (and changes to tests)
into
> another CL? That'd be easy to land first.

Sure. I'll add a couple more cases to make sure we fail correctly when using a
non-WebAssembly.Memory object.

https://codereview.chromium.org/2460773003

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.h...
src/wasm/wasm-module.h:39: #if V8_HOST_ARCH_64_BIT
On 2016/10/28 16:24:57, titzer wrote:
> Any reason not to just define this on all architectures (as an int64_t)?

I was having a hard time finding the right incantation to make all the try bots
happy and finally just gave up. I'll try again though.

Done.

https://codereview.chromium.org/2396433008/diff/300001/tools/testrunner/local...
File tools/testrunner/local/variants.py (right):

https://codereview.chromium.org/2396433008/diff/300001/tools/testrunner/local...
tools/testrunner/local/variants.py:17: "wasm_traps": [["--wasm_guard_pages",
"--invoke-weak-callbacks"]],
On 2016/10/28 16:24:57, titzer wrote:
> Not sure if this will overload the bots. Maybe loop in machenbach@?

Will do.

[Eric Holk, 2016-10-28 22:11:12]

Description was changed from

==========
With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is not implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

This change no longer works with passing an ArrayBuffer to
WebAssembly.Instantiate. Instead you must pass in a legitimate
WebAssembly.Memory.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

to

==========
With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is now implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

This change no longer works with passing an ArrayBuffer to
WebAssembly.Instantiate. Instead you must pass in a legitimate
WebAssembly.Memory.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

[Mircea Trofin, 2016-10-28 22:16:29]

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-js.cc#ne...
src/wasm/wasm-js.cc:231: if (args.Length() > 2 && args[2]->IsObject()) {
please separate support for WebAssembly.Memory in a different CL.

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.cc
File src/wasm/wasm-module.cc (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:112: // addressable memory after the guard page can be
make inaccessible.
can be made

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:114: DCHECK(size % base::OS::CommitPageSize() == 0);
DCHECK_EQ(0, size % etc)

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:121: return nullptr;
because it may fail, perhaps name the API TryAllocateBackingStore

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:638: bool guard) {
guard->enable_guard_regions

also, if this flag is set, is it guaranteed you get guard regions?

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:1074: DCHECK_IMPLIES(FLAG_wasm_guard_pages,
module_->origin == kAsmJsOrigin ||
asmjs, not wasm?

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:2089: ->AdjustAmountOfExternalAllocatedMemory(pages *
WasmModule::kPageSize);
would it make sense to add this API to the internal isolate?

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.h...
src/wasm/wasm-module.h:40: const size_t kWasmMaxHeapOffset =
static_cast<size_t>(8) << 30;  // 8GB
I wonder if there is a more self-explanatory calculation, for example 2 *
std::numeric_limits<uint32_t>::max() ? or 
std::numeric_limits<uint32_t>::max() // maximum offset value
+ std::numeric_limits<uint32_t>::max() // maximum index value

probably there's an even better way.

[Eric Holk, 2016-10-29 00:04:30]

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-js.cc
File src/wasm/wasm-js.cc (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-js.cc#ne...
src/wasm/wasm-js.cc:231: if (args.Length() > 2 && args[2]->IsObject()) {
On 2016/10/28 22:16:29, Mircea Trofin wrote:
> please separate support for WebAssembly.Memory in a different CL.

Done: https://codereview.chromium.org/2460773003/

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.cc
File src/wasm/wasm-module.cc (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:112: // addressable memory after the guard page can be
make inaccessible.
On 2016/10/28 22:16:29, Mircea Trofin wrote:
> can be made

Done.

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:114: DCHECK(size % base::OS::CommitPageSize() == 0);
On 2016/10/28 22:16:29, Mircea Trofin wrote:
> DCHECK_EQ(0, size % etc)

Done.

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:121: return nullptr;
On 2016/10/28 22:16:29, Mircea Trofin wrote:
> because it may fail, perhaps name the API TryAllocateBackingStore

Done.

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:638: bool guard) {
On 2016/10/28 22:16:29, Mircea Trofin wrote:
> guard->enable_guard_regions
> 
> also, if this flag is set, is it guaranteed you get guard regions?

Done.

Yes (at least on 64-bit platforms)

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:1074: DCHECK_IMPLIES(FLAG_wasm_guard_pages,
module_->origin == kAsmJsOrigin ||
On 2016/10/28 22:16:29, Mircea Trofin wrote:
> asmjs, not wasm?

Yes. For AsmJs origin we still do the bounds checking and therefore don't need
guard pages. Some of our tests have buffers that aren't page size multiples, so
we can't add a guard region to those.

This check is making sure that if we accept an externally-allocated memory then
it was allocated with guard pages. The way this happens now is that the
allocator for WebAssembly.Memory sets up the guard pages, and using some other
object would be a TypeError.

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:2089: ->AdjustAmountOfExternalAllocatedMemory(pages *
WasmModule::kPageSize);
On 2016/10/28 22:16:29, Mircea Trofin wrote:
> would it make sense to add this API to the internal isolate?

Perhaps, but I'd rather leave it how it is.
AdjustAmountOfExternalAllocatedMemory accesses fields in the internal
embedder_data_ field, and there doesn't seem to be a lot of precedent for
accessing this through the internal API.

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.h...
src/wasm/wasm-module.h:40: const size_t kWasmMaxHeapOffset =
static_cast<size_t>(8) << 30;  // 8GB
On 2016/10/28 22:16:29, Mircea Trofin wrote:
> I wonder if there is a more self-explanatory calculation, for example 2 *
> std::numeric_limits<uint32_t>::max() ? or 
> std::numeric_limits<uint32_t>::max() // maximum offset value
> + std::numeric_limits<uint32_t>::max() // maximum index value
> 
> probably there's an even better way.

Done

[Michael Lippautz, 2016-10-29 07:39:19]

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.cc
File src/wasm/wasm-module.cc (right):

https://codereview.chromium.org/2396433008/diff/300001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:2089: ->AdjustAmountOfExternalAllocatedMemory(pages *
WasmModule::kPageSize);
On 2016/10/29 00:04:30, Eric Holk wrote:
> On 2016/10/28 22:16:29, Mircea Trofin wrote:
> > would it make sense to add this API to the internal isolate?
> 
> Perhaps, but I'd rather leave it how it is.
> AdjustAmountOfExternalAllocatedMemory accesses fields in the internal
> embedder_data_ field, and there doesn't seem to be a lot of precedent for
> accessing this through the internal API.

+1 for leaving now. This API is used by several consumers (internal and
external) and we have soft and hard limits in place that tell us when to do
which GC. So, unless you want to hunt down memory issues later on, just leave it
as is.

[Michael Achenbach, 2016-11-04 17:52:53]

machenbach@chromium.org changed reviewers:
+ machenbach@chromium.org

[Michael Achenbach, 2016-11-04 17:52:54]

Regarding testing:

You also need to add the variant here so that it's actually run on the bots:
https://cs.chromium.org/chromium/src/v8/tools/run-tests.py?q=run-test+file:%5...

The "default" set is for local testing. There we'd rather not add it. Folks who
want to use it need to specify the variant explicitly. The bots all also include
the "more" set.

https://codereview.chromium.org/2396433008/diff/400001/test/mjsunit/mjsunit.s...
File test/mjsunit/mjsunit.status (right):

https://codereview.chromium.org/2396433008/diff/400001/test/mjsunit/mjsunit.s...
test/mjsunit/mjsunit.status:722: 
What about cctest?
https://cs.chromium.org/chromium/src/v8/test/cctest/cctest.status?q=test/ccte...

Please also check the newer suites for debugger and inspector. They might miss
the skips for asm_wasm as well.

[titzer, 2016-11-07 19:54:53]

https://codereview.chromium.org/2396433008/diff/400001/src/compiler/wasm-comp...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2396433008/diff/400001/src/compiler/wasm-comp...
src/compiler/wasm-compiler.cc:2963: DCHECK(FLAG_wasm_guard_pages);
I think it's better to have FLAG_wasm_trap_handler imply FLAG_wasm_guard_pages
if possible.

https://codereview.chromium.org/2396433008/diff/400001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2396433008/diff/400001/src/flag-definitions.h...
src/flag-definitions.h:531: 
I think you should just be able to do a DEFINE_IMPLICATION(wasm_trap_handler,
wasm_guard_pages) here.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.cc
File src/wasm/wasm-module.cc (right):

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:134: if (guard) {
I think this needs to be TARGET_ARCH.

How about guard && V8_TARGET_ARCH_64_BIT?
That way, the flags FLAG_wasm_trap_handler and FLAG_wasm_guard_pages have no
effect on 32-bit systems instead of just crashing.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:669: bool enable_guard_regions) {
I like this name better. Can we make the header file match it?

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:676: bool is_external = enable_guard_regions;
What about making is_external a return value from TryAllocateBackingStore? Then
we can use it down below without an #ifdef.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:2230: if (FLAG_wasm_guard_pages &&
!old_buffer.is_null()) {
How about !old_buffer.is_null() and old_buffer.has_guard_region()?

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:2231: base::OS::Unprotect(old_buffer->backing_store(),
new_size);
Add a comment that we aren't moving the memory here, just adjusting the size.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.h...
src/wasm/wasm-module.h:567: Handle<JSArrayBuffer> NewArrayBuffer(Isolate*
isolate, size_t size, bool guard);
add_guard_pages?

[Michael Achenbach, 2016-11-08 07:51:08]

https://codereview.chromium.org/2396433008/diff/400001/test/mjsunit/mjsunit.s...
File test/mjsunit/mjsunit.status (right):

https://codereview.chromium.org/2396433008/diff/400001/test/mjsunit/mjsunit.s...
test/mjsunit/mjsunit.status:722: 
On 2016/11/04 17:52:54, machenbach (slow) wrote:
> What about cctest?
>
https://cs.chromium.org/chromium/src/v8/test/cctest/cctest.status?q=test/ccte...
> 
> Please also check the newer suites for debugger and inspector. They might miss
> the skips for asm_wasm as well.

Please also take a look at cctest as mentioned above.

[Eric Holk, 2016-11-08 23:58:15]

https://codereview.chromium.org/2396433008/diff/400001/src/compiler/wasm-comp...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2396433008/diff/400001/src/compiler/wasm-comp...
src/compiler/wasm-compiler.cc:2963: DCHECK(FLAG_wasm_guard_pages);
On 2016/11/07 19:54:52, titzer wrote:
> I think it's better to have FLAG_wasm_trap_handler imply FLAG_wasm_guard_pages
> if possible.

Done.

https://codereview.chromium.org/2396433008/diff/400001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2396433008/diff/400001/src/flag-definitions.h...
src/flag-definitions.h:531: 
On 2016/11/07 19:54:52, titzer wrote:
> I think you should just be able to do a DEFINE_IMPLICATION(wasm_trap_handler,
> wasm_guard_pages) here.

Oh awesome, this is exactly what I wanted.

done.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.cc
File src/wasm/wasm-module.cc (right):

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:134: if (guard) {
On 2016/11/07 19:54:53, titzer wrote:
> I think this needs to be TARGET_ARCH.
> 
> How about guard && V8_TARGET_ARCH_64_BIT?
> That way, the flags FLAG_wasm_trap_handler and FLAG_wasm_guard_pages have no
> effect on 32-bit systems instead of just crashing.

Done. I also updated the help text for --wasm_guard_pages to indicate that the
flag has no effect on 32-bit systems.

Making the flag have no effect on 32-bit makes me a little nervous. We might
have other code expect guard regions and do things that would be unsafe without
them. Having no effect is fine for now, but I think before we have this on by
default it would be good to revisit. I added a TODO.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:669: bool enable_guard_regions) {
On 2016/11/07 19:54:52, titzer wrote:
> I like this name better. Can we make the header file match it?

Done.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:676: bool is_external = enable_guard_regions;
On 2016/11/07 19:54:52, titzer wrote:
> What about making is_external a return value from TryAllocateBackingStore?
Then
> we can use it down below without an #ifdef.

Done, although it seems like V8_TARGET_ARCH_64_BIT is only available from the
preprocessor for some reason, so we still need an #ifdef.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:2230: if (FLAG_wasm_guard_pages &&
!old_buffer.is_null()) {
On 2016/11/07 19:54:53, titzer wrote:
> How about !old_buffer.is_null() and old_buffer.has_guard_region()?

Done.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:2231: base::OS::Unprotect(old_buffer->backing_store(),
new_size);
On 2016/11/07 19:54:53, titzer wrote:
> Add a comment that we aren't moving the memory here, just adjusting the size.

Done.

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/2396433008/diff/400001/src/wasm/wasm-module.h...
src/wasm/wasm-module.h:567: Handle<JSArrayBuffer> NewArrayBuffer(Isolate*
isolate, size_t size, bool guard);
On 2016/11/07 19:54:53, titzer wrote:
> add_guard_pages?

I went with enable_guard_regions based on your other comment.

https://codereview.chromium.org/2396433008/diff/400001/test/mjsunit/mjsunit.s...
File test/mjsunit/mjsunit.status (right):

https://codereview.chromium.org/2396433008/diff/400001/test/mjsunit/mjsunit.s...
test/mjsunit/mjsunit.status:722: 
On 2016/11/04 17:52:54, machenbach (slow) wrote:
> What about cctest?
>
https://cs.chromium.org/chromium/src/v8/test/cctest/cctest.status?q=test/ccte...
> 
> Please also check the newer suites for debugger and inspector. They might miss
> the skips for asm_wasm as well.

I originally had blocked cctests too, but I decided to enable them because they
exercise some Wasm paths that the JS tests don't and that were affected by this
change. Enabling the cctests revealed a few bugs in this change.

I added skips for inspect and debugger though.

https://codereview.chromium.org/2396433008/diff/400001/test/mjsunit/mjsunit.s...
test/mjsunit/mjsunit.status:722: 
On 2016/11/08 07:51:08, machenbach (slow) wrote:
> On 2016/11/04 17:52:54, machenbach (slow) wrote:
> > What about cctest?
> >
>
https://cs.chromium.org/chromium/src/v8/test/cctest/cctest.status?q=test/ccte...
> > 
> > Please also check the newer suites for debugger and inspector. They might
miss
> > the skips for asm_wasm as well.
> 
> Please also take a look at cctest as mentioned above.

Is there a way to invert one of the skip patterns? It'd be nice to be able to
say something like "skip all the cctests that do not has wasm in their name."

[Michael Achenbach, 2016-11-09 08:05:32]

https://codereview.chromium.org/2396433008/diff/400001/test/mjsunit/mjsunit.s...
File test/mjsunit/mjsunit.status (right):

https://codereview.chromium.org/2396433008/diff/400001/test/mjsunit/mjsunit.s...
test/mjsunit/mjsunit.status:722: 
On 2016/11/08 23:58:15, Eric Holk wrote:
> On 2016/11/08 07:51:08, machenbach (slow) wrote:
> > On 2016/11/04 17:52:54, machenbach (slow) wrote:
> > > What about cctest?
> > >
> >
>
https://cs.chromium.org/chromium/src/v8/test/cctest/cctest.status?q=test/ccte...
> > > 
> > > Please also check the newer suites for debugger and inspector. They might
> miss
> > > the skips for asm_wasm as well.
> > 
> > Please also take a look at cctest as mentioned above.
> 
> Is there a way to invert one of the skip patterns? It'd be nice to be able to
> say something like "skip all the cctests that do not has wasm in their name."

No, this feature doesn't exist. We're misusing the status files since a while
now for all kinds of things. The original intent was to temporarily silence a
small number of bugs.

I'm afraid that all cctests with the new variant will add significant runtime
and we'll run into timeouts on the bots. Why is the other wasm variant not using
cctest? If you keep it activated, could you please once run test just cctest
with linux debug and wasm_traps only and measure the time of the slowest tests,
e.g. tools/run-tests.py --variant=wasm_traps cctest --time

Then please skip the slowest tests if there are significant slow ones.

[titzer, 2016-11-09 19:23:47]

lgtm on the wasm side

https://codereview.chromium.org/2396433008/diff/480001/src/compiler/wasm-comp...
File src/compiler/wasm-compiler.cc (right):

https://codereview.chromium.org/2396433008/diff/480001/src/compiler/wasm-comp...
src/compiler/wasm-compiler.cc:2973: DCHECK(FLAG_wasm_guard_pages);
Generally want to avoid DCHECK'ing flags IMO.

https://codereview.chromium.org/2396433008/diff/480001/src/wasm/wasm-module.cc
File src/wasm/wasm-module.cc (right):

https://codereview.chromium.org/2396433008/diff/480001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:714: // down, which means we may leak memory here.
To fix this, we probably need to do something at isolate shutdown time.

https://codereview.chromium.org/2396433008/diff/480001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:2271: const bool enable_guard_regions =
EnableGuardRegions();
Should this always be false? I.e. if the memory before did not have guard
regions, then the memory after also does not.

[Eric Holk, 2016-11-09 20:02:59]

@machenbach: I pruned out a lot of the cctests. On my z840, it cut the cctest
time down from 1m19s to under 10s. Does that sound alright?

https://codereview.chromium.org/2396433008/diff/480001/src/wasm/wasm-module.cc
File src/wasm/wasm-module.cc (right):

https://codereview.chromium.org/2396433008/diff/480001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:714: // down, which means we may leak memory here.
On 2016/11/09 19:23:47, titzer wrote:
> To fix this, we probably need to do something at isolate shutdown time.

Agreed. At this point I'd prefer to address this in a later CL though.

https://codereview.chromium.org/2396433008/diff/480001/src/wasm/wasm-module.c...
src/wasm/wasm-module.cc:2271: const bool enable_guard_regions =
EnableGuardRegions();
On 2016/11/09 19:23:47, titzer wrote:
> Should this always be false? I.e. if the memory before did not have guard
> regions, then the memory after also does not.

You are correct. Done.

[Michael Achenbach, 2016-11-10 07:35:47]

On 2016/11/09 20:02:59, Eric Holk wrote:
> @machenbach: I pruned out a lot of the cctests. On my z840, it cut the cctest
> time down from 1m19s to under 10s. Does that sound alright?

sg! test infra lgtm.

[Eric Holk, 2016-11-10 17:00:52]

The CQ bit was checked by eholk@chromium.org

[Eric Holk, 2016-11-10 17:00:53]

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/2396433008/#ps500001
(title: "Pruning tests and fixing bugs")

[commit-bot: I haz the power, 2016-11-10 17:01:05]

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-10 17:02:41]

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

[commit-bot: I haz the power, 2016-11-10 17:02:42]

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_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_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_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/15996)
  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_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/16038)
  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/15957)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/12297)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/28285)

[Eric Holk, 2016-11-10 18:53:56]

The CQ bit was checked by eholk@chromium.org

[Eric Holk, 2016-11-10 18:53:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
machenbach@chromium.org, titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/2396433008/#ps520001
(title: "Merge branch 'master' of https://chromium.googlesource.com/v8/v8 into
guard-pages")

[commit-bot: I haz the power, 2016-11-10 18:54: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, 2016-11-10 19:34:43]

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

[commit-bot: I haz the power, 2016-11-10 19:34:44]

Try jobs failed on following builders:
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/17547)
  v8_win64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng_triggered/b...)

[Eric Holk, 2016-11-11 04:47:55]

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

[commit-bot: I haz the power, 2016-11-11 04:47:59]

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-11 04:50:12]

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

[commit-bot: I haz the power, 2016-11-11 04:50:14]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/16031)
  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_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  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_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/15992)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/12333)

[Eric Holk, 2016-11-11 04:59:45]

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

[commit-bot: I haz the power, 2016-11-11 04:59:51]

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-11 05:05:40]

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

[commit-bot: I haz the power, 2016-11-11 05:05:42]

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-11 05:08:46]

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

[commit-bot: I haz the power, 2016-11-11 05:08:50]

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-11 05:35:48]

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

[commit-bot: I haz the power, 2016-11-11 05:35:49]

Dry run: This issue passed the CQ dry run.

[Eric Holk, 2016-11-11 05:45:57]

eholk@chromium.org changed reviewers:
+ jochen@chromium.org

[Eric Holk, 2016-11-11 05:45:58]

Jochen - it'd probably be good to have your input on the changes in
base/platform as well.

[jochen (gone - plz use gerrit), 2016-11-12 21:05:45]

can you please reformat your CL description to use < 80c per line.

also, the CL title needs to be repeated as first line of the CL description

https://codereview.chromium.org/2396433008/diff/600001/src/base/platform/plat...
File src/base/platform/platform-posix.cc (right):

https://codereview.chromium.org/2396433008/diff/600001/src/base/platform/plat...
src/base/platform/platform-posix.cc:106: if (allocated != requested) {
why not allocated >= requested?

[jochen (gone - plz use gerrit), 2016-11-12 21:07:21]

+machenbach for test question

https://codereview.chromium.org/2396433008/diff/600001/tools/run-tests.py
File tools/run-tests.py (right):

https://codereview.chromium.org/2396433008/diff/600001/tools/run-tests.py#new...
tools/run-tests.py:114: "wasm_traps",
is there no better way to run all wasm tests with another flag but adding a new
variant?

Now everytime smebody adds some new test directory, they need to remember to
skip it form wasm_traps

[Michael Achenbach, 2016-11-14 07:41:50]

https://codereview.chromium.org/2396433008/diff/600001/tools/run-tests.py
File tools/run-tests.py (right):

https://codereview.chromium.org/2396433008/diff/600001/tools/run-tests.py#new...
tools/run-tests.py:114: "wasm_traps",
On 2016/11/12 21:07:20, jochen (travelling til Nov 18) wrote:
> is there no better way to run all wasm tests with another flag but adding a
new
> variant?
> 
> Now everytime smebody adds some new test directory, they need to remember to
> skip it form wasm_traps

Not a simple one. Neither the status file nor the testsuites were designed for
staging new features. But we're using them that way.

It's not too bad if a folder is forgotten. That means it'd be tested more. Worst
case we run into timeouts. The "MORE_VARIANT" tests don't run on dev machines by
default anyways and therefore won't increase time there.

[Eric Holk, 2016-11-14 18:37:32]

Description was changed from

==========
With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is now implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

This change no longer works with passing an ArrayBuffer to
WebAssembly.Instantiate. Instead you must pass in a legitimate
WebAssembly.Memory.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

to

==========
[wasm] Add guard regions to end of WebAssembly.Memory buffers

With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is now implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

[Eric Holk, 2016-11-14 19:05:56]

https://codereview.chromium.org/2396433008/diff/600001/src/base/platform/plat...
File src/base/platform/platform-posix.cc (right):

https://codereview.chromium.org/2396433008/diff/600001/src/base/platform/plat...
src/base/platform/platform-posix.cc:106: if (allocated != requested) {
On 2016/11/12 21:05:45, jochen (travelling til Nov 18) wrote:
> why not allocated >= requested?

The goal is to allocate all or none of the requested range. If that didn't
happen, we free what was allocated and return nullptr to indicate the failure. I
suppose `allocated < requested` would be okay, but I think it makes more sense
to require the exact amount.

That said, I noticed in the call to Free below we should probably use allocated
instead of requested as the size, so I will fix that.

[jochen (gone - plz use gerrit), 2016-11-17 02:00:34]

lgtm

[Eric Holk, 2016-11-17 17:55:38]

The CQ bit was checked by eholk@chromium.org

[Eric Holk, 2016-11-17 17:55:39]

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

[commit-bot: I haz the power, 2016-11-17 17:55:51]

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-17 17:57:19]

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

[commit-bot: I haz the power, 2016-11-17 17:57:21]

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_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  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_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/16379)
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/18086)

[Eric Holk, 2016-11-17 18:15:30]

The CQ bit was checked by eholk@chromium.org

[Eric Holk, 2016-11-17 18:15:31]

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

[commit-bot: I haz the power, 2016-11-17 18:15:42]

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-17 18:48:20]

Description was changed from

==========
[wasm] Add guard regions to end of WebAssembly.Memory buffers

With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is now implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

to

==========
[wasm] Add guard regions to end of WebAssembly.Memory buffers

With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is now implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

[commit-bot: I haz the power, 2016-11-17 18:48:22]

Committed patchset #34 (id:660001)

[commit-bot: I haz the power, 2016-11-17 22:38:53]

Description was changed from

==========
[wasm] Add guard regions to end of WebAssembly.Memory buffers

With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is now implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

to

==========
[wasm] Add guard regions to end of WebAssembly.Memory buffers

With this change, WebAssembly.Memory objects have backing stores allocated as an
8GB region where everything beyond the size of the Wasm heap is inaccessible.

GrowMemory is now implemented by changing the protection on the guard regions to
make the new portions of the heap accessible.

Guard pages are not enabled by default, but this change adds a flag and a test
variant to make sure we get test coverage on them.

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

Committed: https://crrev.com/eaed31c5f59abfb146c3ca82059ca1c41b1729b9
Cr-Commit-Position: refs/heads/master@{#41089}
==========

[commit-bot: I haz the power, 2016-11-17 22:38:56]

Patchset 34 (id:??) landed as
https://crrev.com/eaed31c5f59abfb146c3ca82059ca1c41b1729b9
Cr-Commit-Position: refs/heads/master@{#41089}