946799 - Write breakpad reserved-memory annotations and fix blocklist annotation

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Benjamin Smedberg]

This is the client side bit of bug 946381, plus a fix to a blocklist annotation which has a typo.

Comment 1

[Benjamin Smedberg]

Attachment: bug946799-partA-reserve-annotation

Comment 2

[Benjamin Smedberg]

Attachment: bug946799-partB-blocklist

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 8343175 [details] [diff] [review]
bug946799-partA-reserve-annotation

Review of attachment 8343175 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/crashreporter/nsExceptionHandler.cpp
@@ +567,5 @@
> +      int bufferLen;
> +
> +      if (gBreakpadReservedVM) {
> +        WriteFile(hFile, kBreakpadReserveAddressParameter, kBreakpadReserveAddressParameterLen, &nBytes, nullptr);
> +        _ui64toa(uintptr_t(gBreakpadReservedVM), buffer, 10);

Is there a reason you're writing this out in base-10 instead of in hex like we do most other addresses?

Comment 4

[Benjamin Smedberg]

There is not a reason other than that's the way I already wrote the processor patch.

Comment 5

[(Away)]

Comment on attachment 8343176 [details] [diff] [review]
bug946799-partB-blocklist

Review of attachment 8343176 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/build/WindowsDllBlocklist.cpp
@@ +149,5 @@
>  static const char kBlockedDllsParameter[] = "BlockedDllList=";
>  static const int kBlockedDllsParameterLen =
>    sizeof(kBlockedDllsParameter) - 1;
>  
> +static const char kBlocklistInitFailedParameter[] = "BlocklistInitFailed=1\n";

I kept these split in order to maintain the key/value pattern from nsExceptionHandler.cpp, but I guess collapsing them is reasonable.

@@ +158,2 @@
>  static const int kUser32BeforeBlocklistParameterLen =
> +  sizeof(kUser32BeforeBlocklistParameter) - 1;

Good catch, thanks!

Comment 6

[Benjamin Smedberg]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dccf3c3e7125
https://hg.mozilla.org/integration/mozilla-inbound/rev/14c40f80f7e2

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dccf3c3e7125
https://hg.mozilla.org/mozilla-central/rev/14c40f80f7e2

Comment 8

[Benjamin Smedberg]

Comment on attachment 8343175 [details] [diff] [review]
bug946799-partA-reserve-annotation

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 943051 made memory reservation work but made the vmblock analysis less useful, this fixes it
User impact if declined: worse data on OOM crashes
Testing completed (on m-c, etc.): verified via crash-stats
Risk to taking this patch (and alternatives if risky): not risky
String or IDL/UUID changes made by this patch: none

Comment 9

[Benjamin Smedberg]

https://hg.mozilla.org/releases/mozilla-beta/rev/591ed7a98ced
https://hg.mozilla.org/releases/mozilla-beta/rev/79e2ad13871d