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

RESOLVED FIXED

Status

()

Toolkit
Crash Reporting
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: joduinn, Assigned: ted)

Tracking

unspecified
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta2+)

Details

Attachments

(1 attachment)

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

8 years ago
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.
blocking2.0: ? → beta1+

Comment 2

8 years ago
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.
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?
Assignee: nobody → ted.mielczarek
(Assignee)

Comment 4

8 years ago
I am currently working on this. It will probably take me a few more days.
(Assignee)

Updated

8 years ago
Blocks: 571367
Ted - does this still block beta1? Should we hold the release for it?
(Assignee)

Comment 6

8 years ago
It's close, but you probably shouldn't hold the beta for it.
--> 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.
blocking2.0: beta1+ → beta2+
(Assignee)

Comment 8

8 years ago
All the necessary changes landed upstream. We need to sync to r619.
Summary: update Breakpad to include upstream Breakpad fixes → sync to Breakpad revision 619 to pick up OS X symbol dumping changes (64-bit support + DWARF CFI support)
(Assignee)

Updated

8 years ago
Blocks: 564632
(Assignee)

Updated

8 years ago
No longer depends on: 558947
(Assignee)

Updated

8 years ago
Blocks: 576053
(Assignee)

Comment 9

8 years ago
Created attachment 455442 [details] [diff] [review]
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.
Attachment #455442 - Flags: review?(mitchell.field)
Attachment #455442 - Flags: review?(mitchell.field) → review+
(Assignee)

Comment 10

8 years ago
Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/9a685a2e4f50
http://hg.mozilla.org/mozilla-central/rev/058117f13cc5
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

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

Comment 12

8 years ago
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.
(Assignee)

Comment 13

8 years ago
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

8 years ago
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.

Updated

8 years ago
Depends on: 577486
(Assignee)

Comment 15

8 years ago
I pushed those bustage fixes upstream:
http://code.google.com/p/google-breakpad/source/detail?r=622
(Assignee)

Updated

8 years ago
Blocks: 553365
(Assignee)

Updated

8 years ago
Blocks: 491774

Comment 16

8 years ago
(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

8 years ago
(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

8 years ago
it's possible to fix this w/o disabling crash reporter. it shouldn't require too much code...
You need to log in before you can comment on or make changes to this bug.