1286005 - Don't include the PID in the NS_DebugBreak crash annotation

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

NS_DebugBreak includes the PID at the start of the abort message. This is fine for printing to the console in a debug build, but it prevents anybody from using the abort message as a facet in crash-stats searches, as seen in bug 1273770.

One solution would be to generate a separate string for printing that includes the pid (or only include the PID when doing the final printout). Another solution would be to just never include the PID in a non-debug build.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Don't include the PID in the NS_DebugBreak crash annotation.

I tested locally and it seems to produce the same output for buf, and the appropriate non-pid output for nonPIDBuf.

Including the PID makes it impossible to aggregate crash reports on crash-stats.

This also reduces buffer size to ensure that having two buffers does not increase total stack size, though that is unlikely to matter. 1000 characters is likely excessive in any event.

I changed the print macro to ALL_CAPS because I think it is confusing to have a macro with an unbound variable in it that looks like a regular function call.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e50eb5912d50
Don't include the PID in the NS_DebugBreak crash annotation. r=froydnj

Comment 3

[Andrew McCreight [:mccr8]]

Nick, since you've been looking at MOZ_CRASH() messages, you might find this interesting, in case you start looking at NS_DebugBreak crash messages.

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e50eb5912d50

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Oops, I had more extensive patches in bug 1277448 that I hadn't gotten to addressing review comments on that did this and some other things...

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(I think the right thing to do will be to just back this out when I land those, but I'll look more closely first.)

Comment 7

[Andrew McCreight [:mccr8]]

Comment on attachment 8769865 [details] [diff] [review]
Don't include the PID in the NS_DebugBreak crash annotation.

Approval Request Comment
[Feature/regressing bug #]: old
[User impact if declined]: This bug makes it easier to understand why certain crashes are happening.
[Describe test coverage new/current, TreeHerder]: We have some testing of crashing, but not for this particular detail of behavior.
[Risks and why]: Low. It has been on trunk for a while, and it only affects crash behavior.
[String/UUID change made/needed]: Low.

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8769865 [details] [diff] [review]
Don't include the PID in the NS_DebugBreak crash annotation.

Diagnostic patch for crashes, please uplift to beta.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1d80b9b0c4d9