Add fastfail to VC++ linker

Add fastfail to VC++ linker

/fastfail should get us crash dumps when the linker crashes, which it
has been doing. /maxilksize is being added for consistency with gyp
builds, and because the linker crashes are incremental linking related.

This is being added in order to investigate a particular problem but
it is a good long-term change also - crashing without a crash dump is
not helpful.

BUG=482671
Committed: https://crrev.com/1a87fbf87fc1ac4aaef80094be817c6b3a87dc1b
Cr-Commit-Position: refs/heads/master@{#382682}

[brucedawson, 2016-03-21 23:17:37]

brucedawson@chromium.org changed reviewers:
+ thakis@chromium.org

[brucedawson, 2016-03-21 23:17:37]

Consider applying this patch locally.

And maybe we should land it as well - failfast seems like a good thing.

I need to check that it works with goma, and probably add the same thing for
compilation, before landing.

[brucedawson, 2016-03-21 23:23:37]

Never mind - the .gn modification is in the wrong file. Please wait...

[brucedawson, 2016-03-21 23:50:34]

Description was changed from

==========
Add /fastfail, and /maxilksize to gn builds

/fastfail should get us crash dumps when the linker crashes, which it
has been doing. /maxilksize is being added for consistency with gyp
builds, and because the linker crashes are incremental linking related.

BUG=482671
==========

to

==========
Add fastfail to VC++ compiler and linker

/fastfail should get us crash dumps when the linker crashes, which it
has been doing. /maxilksize is being added for consistency with gyp
builds, and because the linker crashes are incremental linking related.

/d2FastFail is the compiler equivalent.

BUG=482671
==========

[brucedawson, 2016-03-21 23:51:04]

This is ready for local use now, with gn and gyp builds.

The settings are not goma compatible so I will file a bug for the goma team to
fix that.

[Nico, 2016-03-21 23:56:16]

Making maxilksize consistent looks good (but I got the crash with a gyp build
too, so it probably doesn't help with this particular issue. Still, good to be
consistent).

I don't know if we want the other flags on the bots. How big are the dumps? Do
they accumulate?

[brucedawson, 2016-03-22 00:05:08]

The crash dumps will use the default WER (Windows Error Reporting) settings, so
however those are configured. The default is, IIRC, a minidump but I don't know
where it gets stored.

We could configure the machines with whatever policies we want - save the last
ten crash dumps to %localappdata%\Crashdumps as minidumps is easy enough.
Minidumps would at most be a few MB - they mostly just save stack contents. A
full crash dump for the linker could be excessive.

I don't know how easy it is to retrieve crash dumps.

[brucedawson, 2016-03-22 17:57:45]

Description was changed from

==========
Add fastfail to VC++ compiler and linker

/fastfail should get us crash dumps when the linker crashes, which it
has been doing. /maxilksize is being added for consistency with gyp
builds, and because the linker crashes are incremental linking related.

/d2FastFail is the compiler equivalent.

BUG=482671
==========

to

==========
Add fastfail to VC++ linker

/fastfail should get us crash dumps when the linker crashes, which it
has been doing. /maxilksize is being added for consistency with gyp
builds, and because the linker crashes are incremental linking related.

This is being added in order to investigate a particular problem but
it is a good long-term change also - crashing without a crash dump is
not helpful.

BUG=482671
==========

[brucedawson, 2016-03-22 17:57:45]

brucedawson@chromium.org changed reviewers:
+ scottmg@chromium.org

[brucedawson, 2016-03-22 18:08:36]

scottmg@, any thoughts on this?

Recording crash dumps on linker crashes seems good. I don't know if we have to
change the WER settings. Could get one of the runhooks steps to set one or more
of these registry keys:

https://msdn.microsoft.com/en-us/library/windows/desktop/bb787181(v=vs.85).aspx

[scottmg, 2016-03-22 18:28:25]

I don't immediately see any docs for /fastfail
https://msdn.microsoft.com/en-us/library/y0zzbyt4.aspx is it documented
somewhere?

I assume that it won't cause a hangy-dialog-thing when it crashes?

[scottmg, 2016-03-22 18:29:44]

Otherwise, seems fine.

(OMG, Nico is away until June! wat)

[brucedawson, 2016-03-22 18:40:14]

On 2016/03/22 18:29:44, scottmg wrote:
> Otherwise, seems fine.
> 
> (OMG, Nico is away until June! wat)

/fastfail (and the compiler equivalent /d2FastFail) are flags that "are
undocumented, might not work, and whose names might change in the future." I've
asked whether a WER dialog is likely. We'll see.

[brucedawson, 2016-03-22 20:40:04]

/fastfail would normally trigger a WER dialog, but once crrev.com/1825163003
lands that will no longer happen - DontShowUI is magic!

[brucedawson, 2016-03-22 20:54:31]

crrev.com/1825163003 landed. PTAL at this change - I think it's ready to go now.

[scottmg, 2016-03-22 21:08:04]

Locally we're going to get a crashdump and a WER dialog now, is that right? I
guess that's OK, the crbugs won't be useful but they aren't really anyway.

lgtm

[Nico, 2016-03-22 21:11:38]

On 2016/03/22 18:29:44, scottmg wrote:
> Otherwise, seems fine.
> 
> (OMG, Nico is away until June! wat)

(whoopsie small typo. I meant "Mar" not "May", thanks for catching that!)

[brucedawson, 2016-03-22 21:15:03]

On 2016/03/22 21:08:04, scottmg wrote:
> Locally we're going to get a crashdump and a WER dialog now, is that right? I
> guess that's OK, the crbugs won't be useful but they aren't really anyway.
> 
> lgtm

Local crashes should get a WER dialog. Whether there is a crash dump depends on
magic local settings. On my machine I'll get a full crash dump.

[brucedawson, 2016-03-22 21:15:17]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-03-22 21:15:33]

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

[scottmg, 2016-03-22 21:22:07]

On 2016/03/22 21:11:38, Nico (away until Mon Mar 27) wrote:
> On 2016/03/22 18:29:44, scottmg wrote:
> > Otherwise, seems fine.
> > 
> > (OMG, Nico is away until June! wat)
> 
> (whoopsie small typo. I meant "Mar" not "May", thanks for catching that!)

Purely self-preservation driven on tools/ and build/ reviews. :)

[commit-bot: I haz the power, 2016-03-22 21:25:20]

Description was changed from

==========
Add fastfail to VC++ linker

/fastfail should get us crash dumps when the linker crashes, which it
has been doing. /maxilksize is being added for consistency with gyp
builds, and because the linker crashes are incremental linking related.

This is being added in order to investigate a particular problem but
it is a good long-term change also - crashing without a crash dump is
not helpful.

BUG=482671
==========

to

==========
Add fastfail to VC++ linker

/fastfail should get us crash dumps when the linker crashes, which it
has been doing. /maxilksize is being added for consistency with gyp
builds, and because the linker crashes are incremental linking related.

This is being added in order to investigate a particular problem but
it is a good long-term change also - crashing without a crash dump is
not helpful.

BUG=482671
==========

[commit-bot: I haz the power, 2016-03-22 21:25:22]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-22 21:27:35]

Description was changed from

==========
Add fastfail to VC++ linker

/fastfail should get us crash dumps when the linker crashes, which it
has been doing. /maxilksize is being added for consistency with gyp
builds, and because the linker crashes are incremental linking related.

This is being added in order to investigate a particular problem but
it is a good long-term change also - crashing without a crash dump is
not helpful.

BUG=482671
==========

to

==========
Add fastfail to VC++ linker

/fastfail should get us crash dumps when the linker crashes, which it
has been doing. /maxilksize is being added for consistency with gyp
builds, and because the linker crashes are incremental linking related.

This is being added in order to investigate a particular problem but
it is a good long-term change also - crashing without a crash dump is
not helpful.

BUG=482671

Committed: https://crrev.com/1a87fbf87fc1ac4aaef80094be817c6b3a87dc1b
Cr-Commit-Position: refs/heads/master@{#382682}
==========

[commit-bot: I haz the power, 2016-03-22 21:27:36]

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