Closed Bug 946799 Opened 6 years ago Closed 6 years ago

Write breakpad reserved-memory annotations and fix blocklist annotation

Categories

(Toolkit :: Crash Reporting, defect)

All
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox27 --- fixed

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files)

This is the client side bit of bug 946381, plus a fix to a blocklist annotation which has a typo.
Attachment #8343175 - Flags: review?(ted)
Attachment #8343176 - Flags: review?(dmajor)
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?
Attachment #8343175 - Flags: review?(ted) → review+
There is not a reason other than that's the way I already wrote the processor patch.
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!
Attachment #8343176 - Flags: review?(dmajor) → review+
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
Attachment #8343175 - Flags: approval-mozilla-beta?
Attachment #8343175 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.