Hook up custom Win 64 SEH to v8

Hook up custom Win 64 SEH to v8

BUG=v8:3597
R=cpu@chromium.org,wfh@chromium.org

Committed: https://crrev.com/284435c33e33b0bcb765671fca98f52567671fe7
Cr-Commit-Position: refs/heads/master@{#297820}

[jochen (gone - plz use gerrit), 2014-09-30 07:39:13]

ptal

[cpu_(ooo_6.6-7.5), 2014-09-30 19:20:49]

looking good, just one question on my side.

https://codereview.chromium.org/619543002/diff/40001/components/crash/app/bre...
File components/crash/app/breakpad_win.cc (right):

https://codereview.chromium.org/619543002/diff/40001/components/crash/app/bre...
components/crash/app/breakpad_win.cc:616: if (g_breakpad) {
why not call here CrashForException() ?

https://codereview.chromium.org/619543002/diff/40001/components/crash/app/bre...
components/crash/app/breakpad_win.cc:636: reinterpret_cast<DWORD64>(start) |
0x3,
or 3 ?

this needs a comment.

[jochen (gone - plz use gerrit), 2014-09-30 19:23:26]

https://codereview.chromium.org/619543002/diff/40001/components/crash/app/bre...
File components/crash/app/breakpad_win.cc (right):

https://codereview.chromium.org/619543002/diff/40001/components/crash/app/bre...
components/crash/app/breakpad_win.cc:616: if (g_breakpad) {
On 2014/09/30 19:20:49, cpu wrote:
> why not call here CrashForException() ?

i don't have exception pointers.

I could create a runtime function with unwind info that has CrashForException as
exception handler, but the unwind info needs to be in the same memory range as
the crash that occurred :-/ which i can't generally write to

https://codereview.chromium.org/619543002/diff/40001/components/crash/app/bre...
components/crash/app/breakpad_win.cc:636: reinterpret_cast<DWORD64>(start) |
0x3,
On 2014/09/30 19:20:49, cpu wrote:
> or 3 ?


well, because that's what the API says. First parameter should be the address |
0x3, second parameter should be the address. what else would you expect?

[jochen (gone - plz use gerrit), 2014-09-30 20:06:08]

ptal

[Will Harris, 2014-09-30 20:40:00]

https://codereview.chromium.org/619543002/diff/60001/components/crash/app/bre...
File components/crash/app/breakpad_win.cc (right):

https://codereview.chromium.org/619543002/diff/60001/components/crash/app/bre...
components/crash/app/breakpad_win.cc:638: static_cast<DWORD>(size_in_bytes),
check limits here during size_t -> DWORD conversion

[jochen (gone - plz use gerrit), 2014-09-30 21:18:32]

done, ptal

[cpu_(ooo_6.6-7.5), 2014-09-30 22:42:15]

You are going to hate me but I recant. This approach does not give us the
exception info and a crash without the exception info is unusable.

I should have caught that, terribly sorry.

basically we need to be calling CrashForException.

Maybe we can continue the review on the older patch?

[jochen (gone - plz use gerrit), 2014-10-01 07:24:14]

On 2014/09/30 at 22:42:15, cpu wrote:
> You are going to hate me but I recant. This approach does not give us the
exception info and a crash without the exception info is unusable.
> 
> I should have caught that, terribly sorry.
> 
> basically we need to be calling CrashForException.
> 
> Maybe we can continue the review on the older patch?

that approach was really too hacky (and wasted a lot of memory - I had to commit
something like 6MB for a 64byte data structure)

Mozilla uses a vectored exception handler and tries to manually unwind the
stack. If that fails, it calls CrashForException() with the exception pointers
it got from the VEH.

I know you said that this might be too brittle for production, but it looks not
too complicated.

wdyt?

[jochen (gone - plz use gerrit), 2014-10-01 12:45:58]

ptal

now we get a real exception.

depends on v8 bleeding_edge@24353 to reserve some space in the code range for
this.

[cpu_(ooo_6.6-7.5), 2014-10-02 00:23:55]

lgtm

[jochen (gone - plz use gerrit), 2014-10-02 09:59:48]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-10-02 10:00:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/619543002/140001

[jochen (gone - plz use gerrit), 2014-10-02 11:29:24]

The CQ bit was unchecked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2014-10-02 11:35:28]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-10-02 11:35:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/619543002/160001

[commit-bot: I haz the power, 2014-10-02 13:08:32]

Committed patchset #9 (id:160001) as 1208342536b0a59dee1c8626ebcc4cade4766189

[commit-bot: I haz the power, 2014-10-02 13:09:13]

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