Use Structured Exception Handler on Windows to gracefully handle guard page faults

Use Structured Exception Handler on Windows to gracefully handle guard page faults

- set profile_vm flag to be true by default to make sure crashes on bots are taken care of.

R=koda@google.com

Committed: https://code.google.com/p/dart/source/detail?r=45771

[Cutch, 2015-05-11 20:38:31]

johnmccutchan@google.com changed reviewers:
+ koda@google.com, vegorov@google.com

[koda, 2015-05-12 15:53:28]

Are you confident that using __try in one method does not impact the compilation
of the rest of the code (i.e., that it triggers some global change in behavior)?

[koda, 2015-05-12 19:28:03]

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:44: DEFINE_FLAG(bool, profile_vm, true,
Please note in the CL description that you are changing this default value
(assuming you intend to commit it like this).

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:939: fault_address =
ep->ExceptionRecord->ExceptionInformation[1];
Since I suppose we only expect this to be caused by reads, consider adding an
ASSERT that the first component is 0.

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:999: DWORD newProtect = PAGE_READWRITE | PAGE_GUARD;
newProtect -> new_protect, etc.

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:1001: BOOL r =
VirtualProtect(reinterpret_cast<void*>(fault_address),
r -> success

[Vyacheslav Egorov (Google), 2015-05-12 19:46:18]

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:939: fault_address =
ep->ExceptionRecord->ExceptionInformation[1];
I wonder if you could just do VirtualProtect in the filter instead of stashing
the fault address into a global.

[Cutch, 2015-05-13 17:59:10]

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:44: DEFINE_FLAG(bool, profile_vm, true,
On 2015/05/12 19:28:02, koda wrote:
> Please note in the CL description that you are changing this default value
> (assuming you intend to commit it like this).

Done.

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:939: fault_address =
ep->ExceptionRecord->ExceptionInformation[1];
On 2015/05/12 19:28:03, koda wrote:
> Since I suppose we only expect this to be caused by reads, consider adding an
> ASSERT that the first component is 0.

Done.

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:939: fault_address =
ep->ExceptionRecord->ExceptionInformation[1];
On 2015/05/12 19:46:18, Vyacheslav Egorov (Google) wrote:
> I wonder if you could just do VirtualProtect in the filter instead of stashing
> the fault address into a global.

Acknowledged. I need an exception handler to jump to so I'd rather put the
virtual protected there.

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:999: DWORD newProtect = PAGE_READWRITE | PAGE_GUARD;
On 2015/05/12 19:28:03, koda wrote:
> newProtect -> new_protect, etc.

Done.

https://codereview.chromium.org/1131233005/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:1001: BOOL r =
VirtualProtect(reinterpret_cast<void*>(fault_address),
On 2015/05/12 19:28:02, koda wrote:
> r -> success

Done.

[koda, 2015-05-13 18:15:21]

lgtm

[Cutch, 2015-05-13 19:36:59]

Committed patchset #4 (id:60001) manually as 45771 (presubmit successful).