567424 - sync to Breakpad revision 619 to pick up OS X symbol dumping changes (64-bit support + DWARF CFI support)

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

As requested, filing separate bug to track bringing in-tree breakpad into sync with upstream fixes. This is needed for 10.6 64bit symbols. Further details in bug#548025.

nom'd as this blocks osx10.6 64bit release.

Comment 1

[Josh Aas]

Marking this as blocking, though in the future I'd prefer to have a bug about the specific problem we're hoping to solve be a blocker.

Comment 2

[Jim Blandy :jimb]

Google got around to reviewing the patches.  They were large patches, adding significant amounts of new code, and Google had a bunch of changes they wanted.  Ted offered to take care of updating the patches for me.  I have iterated with Google on the review comments so that there's a consensus on each suggested change.

Links to Rietveld issues for the two patches:
http://breakpad.appspot.com/96001/show
http://breakpad.appspot.com/93001/show

Most of the changes Google asked for are small and local, except for these:

- I used C++ iostreams for some error reporting; Google prefers stderr.  It's in the style guide, so I should have known. I was not using any interesting properties of C++ iostreams, so this is not technically significant.

- I used GNU style in my comments, where references to function arguments are CAPITALIZED; Google prefers that they be |piped|.  Big change, because I comment every data member and public function; not technically significant.

Comment 3

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Ted, pushing this your way as you are already working in that area, and the beta is looming closer. If you are not the right person for this, can you let me know who should be?

Comment 4

[(not currently active) Ted Mielczarek]

I am currently working on this. It will probably take me a few more days.

Comment 5

[Johnathan Nightingale [:johnath]]

Ted - does this still block beta1? Should we hold the release for it?

Comment 6

[(not currently active) Ted Mielczarek]

It's close, but you probably shouldn't hold the beta for it.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

--> beta2+, we decided that we wouldn't block the first beta on this.

Choffman asked if we wanted to turn off OOPP so that Socorro had a proper baseline, but Shaver and I agreed that we'd rather get the bug reports and UX feedback from users about OOPP and figure out how to deal with the crash stats afterwards.

Comment 8

[(not currently active) Ted Mielczarek]

All the necessary changes landed upstream. We need to sync to r619.

Comment 9

[(not currently active) Ted Mielczarek]

Attachment: Build system changes (to r619)

As usual, I had to tweak our Makefiles a bit to get this to build, due to files being moved around in Breakpad. I tested this on Linux and OS X (64-bit). I'll probably build on Windows too before I land it.

I found one compile error on 64-bit OS X that I rolled into this patch, and submitted upstream for review:
http://breakpad.appspot.com/124001/show

The Breakpad update itself is a huge patch, and as usual I'm not actually asking for review on that.

Comment 10

[(not currently active) Ted Mielczarek]

Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/9a685a2e4f50
http://hg.mozilla.org/mozilla-central/rev/058117f13cc5

Comment 11

[(not currently active) Ted Mielczarek]

bustage fix:
http://hg.mozilla.org/mozilla-central/rev/2db1618de875
(not sure how that snuck by)

Comment 12

[(not currently active) Ted Mielczarek]

Another bustage fix:
http://hg.mozilla.org/mozilla-central/rev/360f7ec9f2fe

Apparently I don't hit these with gcc 4.4, but tinderbox with gcc 4.3.3 does.

Comment 13

[(not currently active) Ted Mielczarek]

What a train wreck. One more bustage fix for OS X x86-64 (which built fine locally without this patch, wtf):
http://hg.mozilla.org/mozilla-central/rev/f4ca183be77c

Comment 14

[John P Baker]

This seems to have broken on Windows 2000 (yes I know!) with

RtlCaptureContext could not be loaded in dynamic link library KERNEL32.dll

on attempted start.

Comment 15

[(not currently active) Ted Mielczarek]

I pushed those bustage fixes upstream:
http://code.google.com/p/google-breakpad/source/detail?r=622

Comment 16

[Jim Mathies [:jimm]]

(In reply to comment #10)
> Pushed to m-c:
> http://hg.mozilla.org/mozilla-central/rev/9a685a2e4f50
> http://hg.mozilla.org/mozilla-central/rev/058117f13cc5

This added a call to RtlCaptureContext which currently isn't available on 2K.

http://msdn.microsoft.com/en-us/library/ms680591%28VS.85%29.aspx

Would disabling the crash reporter address this?

Comment 17

[Jim Mathies [:jimm]]

(In reply to comment #16)
> (In reply to comment #10)
> > Pushed to m-c:
> > http://hg.mozilla.org/mozilla-central/rev/9a685a2e4f50
> > http://hg.mozilla.org/mozilla-central/rev/058117f13cc5
> 
> This added a call to RtlCaptureContext which currently isn't available on 2K.
> 
> http://msdn.microsoft.com/en-us/library/ms680591%28VS.85%29.aspx
> 
> Would disabling the crash reporter address this?

Also, I'm assuming this makes 4.0b2 inoperable on 2K. I was testing a standard release build of m-c when I ran into this.

Comment 18

[timeless]

it's possible to fix this w/o disabling crash reporter. it shouldn't require too much code...