Closed Bug 514188 Opened 10 years ago Closed 10 years ago

sync to breakpad revision 463 to pick up Linux client rewrite

Categories

(Toolkit :: Crash Reporting, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- .4-fixed

People

(Reporter: ted, Assigned: ted)

References

Details

(Whiteboard: [fixed-lorentz])

Attachments

(2 files, 1 obsolete file)

In bug 510920 josh was trying to sync up to a newer breakpad to pick up some OS X fixes. However, in Breakpad revision 384, a rewrite of the Linux client code was landed, so taking that code from upstream requires some work. I got some of it working, but it didn't seem worthwhile to block the OS X fixes on that, so I'm spinning that off into a separate bug. I think the Linux client rewrite probably has some things we want, and it's generally good practice to sync up with upstream every once in a while anyway.
One of the advantage of the linux client rewrite, is that it seems to get the crash address correct. i.e. instead of reporting the eip for SIGSEGV it reports the faulting memory address.
I'm going to fix this before I get started on bug 525849, doesn't make sense to try to use the (now replaced) Linux Breakpad code as a starting point.
Assignee: nobody → ted.mielczarek
Status: NEW → ASSIGNED
Summary: sync to breakpad tip (revision 392 or later) to pick up Linux client rewrite → sync to breakpad revision 434 to pick up Linux client rewrite
Just to document the steps I'm using, since I never remember to do it and always waste a bunch of time.
Starting from a clean mozilla-central tree:
rm -rf toolkit/crashreporter/google-breakpad
svn export --ignore-externals -r 434 ../google-breakpad ./toolkit/crashreporter/google-breakpad
hg st -un | grep "Makefile$" | xargs rm # to remove Breakpad Makefiles that we aren't using
hg st -n | grep "Makefile\.in$" | xargs hg revert --no-backup # restore our Makefile.ins
hg addremove toolkit/crashreporter/google-breakpad/

(I should put this on a wiki somewhere.)
Summary: sync to breakpad revision 434 to pick up Linux client rewrite → sync to breakpad revision 437 to pick up Linux client rewrite
Here's what the Breakpad changes look like, I'm not going to bother trying to attach them here:
http://hg.mozilla.org/try/rev/e3b777e00767
Blocks: 427210
It looks like we'll get 64-bit Linux client support out of this as well.
The exception handler is busted for us, we crash in its signal handler. My unittests pass, so I suspect this is due to us having a chained signal handler.
This fixes nsProfileLock so it chains to Breakpad's signal handler properly.
Attachment #416284 - Flags: review?(benjamin)
Attached patch build system changes (obsolete) — Splinter Review
These are the build system changes needed to build with the latest Breakpad SVN (r437).
Attachment #416285 - Flags: review?(benjamin)
Attachment #416284 - Flags: review?(benjamin) → review+
Attachment #416285 - Flags: review?(benjamin) → review+
Comment on attachment 416285 [details] [diff] [review]
build system changes

I'm assuming that this review also applies to the Breakpad patch that I didn't attach here (but linked to above).
I didn't review it: it's a straight import of the project code, right? (In which case review isn't necessary).
Yes.
Blocks: 532713
Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/2138d22ff018
http://hg.mozilla.org/mozilla-central/rev/09e8d73fda8b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
OK. I am assuming it is way too late to get this into Firefox 3.6.  However, I think this needs to be in 3.6.1.  I suspect many Linux users have AMD64 processors.
Flags: wanted1.9.2?
This isn't going to land on the branch. It's too much churn.
<soapbox>So yet another reason I will never run a 3.6.x version and recommend other just stay at 3.5 until 3.7 or whatever is next appears because 3.6 seems to be  no useful new features or fixes release.</soapbox>
Please leave content-free advocacy out of bugzilla. Thanks.
Well it is interesting in learning that you have zero interest in supporting Linux users.
All of our Linux users use distro builds anyway. But this bug is fixed.
Hmm.I am a linux user.  I don't use a distro build.  So, it would seem your previous assertion is incorrect.

But to stop this stupid argument, I realize this is a big change and that is why I said for 3.6 it was not a good idea and suggested it for 3.6.1.

And perhaps for 3.6.1 or even .2 it is not ready.  If you think all Linux users use distro builds, then since they all have crashreporter disabled you will never get any information about Linux only crashes.

The point is that those Linux users who do NOT use distro builds, are the same class of user who most likely have an AMD64 processor and have not been able to give useful crash information since we stopped using talkbalk (which worked just fine by the way) and replaced it with breakpad/crashreporter which had this issue in that it does not work at all with an AMD64 processor.

So, the point is do we really want to go through the entire 3.6 cycle with only getting useful crash reports from people using Intel processors (which I contend is the minority of Linux users)?
(In reply to comment #20)
> If you think all Linux users
> use distro builds, then since they all have crashreporter disabled you will
> never get any information about Linux only crashes.

There are attempts (see bug 447771) to allow distribution builds to report crashes to crash-stats.mozilla.org
I backed the update patches out due to bug 535071:
http://hg.mozilla.org/mozilla-central/rev/5694f93345f7
http://hg.mozilla.org/mozilla-central/rev/088fc4c3f9df
http://hg.mozilla.org/mozilla-central/rev/35df81a48484
http://hg.mozilla.org/mozilla-central/rev/bd072f4c6a78

(I left the signal handler patch in.)
Status: RESOLVED → REOPENED
Flags: wanted1.9.2? → wanted1.9.2-
Resolution: FIXED → ---
I have a patch up for review upstream to fix the bustage we saw:
http://breakpad.appspot.com/49008
(In reply to comment #3)
> (I should put this on a wiki somewhere.)

I did, so I don't forget next time:
https://wiki.mozilla.org/Breakpad/Updating_to_latest_Breakpad_from_SVN
I backed out the nsProfileLock change, I think without the Breakpad update it actually broke things worse:
http://hg.mozilla.org/mozilla-central/rev/d303e54445e7
http://hg.mozilla.org/mozilla-central/rev/a3dea6c607dd
Ok, fixed the upstream issue:
http://code.google.com/p/google-breakpad/source/detail?r=461

I'll get my patches refreshed and relanded ASAP.
Blocks: 522788
Summary: sync to breakpad revision 437 to pick up Linux client rewrite → sync to breakpad revision 461 to pick up Linux client rewrite
A few changes to accommodate upstream changes since the previous patch. Interdiff should show it's not a lot.
Attachment #416285 - Attachment is obsolete: true
Attachment #419893 - Flags: review?(benjamin)
Actually synced to 463, I misled myself somewhere.

bsmedberg said to carry his review forward since there aren't any major changes.

Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/3975716a6eb4 (nsProfileLock changes)
http://hg.mozilla.org/mozilla-central/rev/5e311ec4e775 (breakpad sync)
http://hg.mozilla.org/mozilla-central/rev/defc32dee2f8 (build system changes)
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Summary: sync to breakpad revision 461 to pick up Linux client rewrite → sync to breakpad revision 463 to pick up Linux client rewrite
Attachment #419893 - Flags: review?(benjamin) → review+
Does this mean that you'll be enabling the crash reporter on the trunk x86_64 Linux nightlies soonish?
I'm not sure. It works there, but we don't have the ability to walk the stack on the server side yet. That will depend on bug 464750, which is in progress.
Depends on: 537950
Target Milestone: --- → mozilla1.9.3a1
Blocks: 530041
Blocks: 552909
Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed
Blocks: 411392
You need to log in before you can comment on or make changes to this bug.