Sync to breakpad revision 350 for breakpad fix 248, 10.5+ SDK compat

RESOLVED FIXED

Status

()

Toolkit
Crash Reporting
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Josh Aas, Assigned: Josh Aas)

Tracking

Trunk
x86_64
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(status1.9.2 final-fixed, status1.9.1 .8-fixed)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

8 years ago
I'd like to pick up the fix for breakpad bug 248 which allows building against the Mac OS X 10.5+ SDKs. Without that fix people run into build failures quite often and need to go back and disable breakpad. We also need this if we are going to move to 10.5+ SDK builds on trunk and for 64-bit builds.

http://code.google.com/p/google-breakpad/issues/detail?id=248
(Assignee)

Comment 1

8 years ago
Created attachment 397043 [details] [diff] [review]
fix v1.0
(Assignee)

Updated

8 years ago
Assignee: nobody → joshmoz
(Assignee)

Updated

8 years ago
Blocks: 501436
(Assignee)

Comment 2

8 years ago
Created attachment 397116 [details] [diff] [review]
fix v1.1

Includes my fix for the following breakpad bug:

http://code.google.com/p/google-breakpad/issues/detail?id=330
Attachment #397043 - Attachment is obsolete: true
I did some fixup on this patch, but my Linux build is still failing because some of the upstream files don't compile with -pedantic. I think the faster path to success here will be to just take a revision prior to the linux client rewrite, so we get the 10.5 SDK fixes but don't have to deal with all this Linux stuff yet. I'll update us to the latest code at a later date and deal with the issues, since they may require some upstream fixes.

Josh: can you regenerate this patch from svn revision 350? That would be prior to the linux rewrite, but would pick up my fix for http://code.google.com/p/google-breakpad/issues/detail?id=323, which would be a nice bonus.

One side note, I forgot to mention that when syncing from upstream, I leave out the "Makefile" files from the linux/ dirs, since they'd confuse our build system. I stripped a few of those out of your patch before testing.
Updating summary to narrow scope.
Summary: pull breakpad from upstream for breakpad fix 248, 10.5+ SDK compat → Sync to breakpad revision 350 for breakpad fix 248, 10.5+ SDK compat
Blocks: 514188
(Assignee)

Comment 5

8 years ago
Created attachment 398432 [details] [diff] [review]
fix v1.2

Breakpad revision 350 patch.
Attachment #397116 - Attachment is obsolete: true
Attachment #398432 - Flags: review?(ted.mielczarek)
Comment on attachment 398432 [details] [diff] [review]
fix v1.2

diff --git a/toolkit/crashreporter/google-breakpad/src/client/linux/handler/Makefile b/toolkit/crashreporter/google-breakpad/src/client/linux/handler/Makefile
new file mode 100644
diff --git a/toolkit/crashreporter/google-breakpad/src/client/solaris/handler/Makefile b/toolkit/crashreporter/google-breakpad/src/client/solaris/handler/Makefile
new file mode 100644
diff --git a/toolkit/crashreporter/google-breakpad/src/tools/linux/dump_syms/Makefile b/toolkit/crashreporter/google-breakpad/src/tools/linux/dump_syms/Makefile
new file mode 100644
diff --git a/toolkit/crashreporter/google-breakpad/src/tools/linux/symupload/Makefile b/toolkit/crashreporter/google-breakpad/src/tools/linux/symupload/Makefile
new file mode 100644
diff --git a/toolkit/crashreporter/google-breakpad/src/tools/solaris/dump_syms/Makefile b/toolkit/crashreporter/google-breakpad/src/tools/solaris/dump_syms/Makefile
new file mode 100644

Please leave these files out of the patch, they're likely to confuse our build system.

Looks fine otherwise, thanks!
Attachment #398432 - Flags: review?(ted.mielczarek) → review+
This is the fix that we have locally that's not upstreamed:
http://hg.mozilla.org/mozilla-central/rev/cfef2762ecb3
(Assignee)

Comment 8

8 years ago
That fix appears to have been upstreamed, breakpad svn revision 313 so we pick it up when updating to revision 350.
(Assignee)

Comment 9

8 years ago
pushed to mozilla-central with ted's comments addressed

http://hg.mozilla.org/mozilla-central/rev/e8c01867056a
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 10

8 years ago
backed out because of this:

exception_handler.cc:181: error: ‘memset’ was not declared in this scope

I assume we just need to include string.h in that file but why hasn't anyone else ran into this?
(Assignee)

Comment 11

8 years ago
Created attachment 398569 [details] [diff] [review]
fix v1.3

This patch, the one I pushed to mozilla-central, compiled on the try server but failed due to the memset problem mentioned above on mozilla-central.
Attachment #398432 - Attachment is obsolete: true
Could be the switch to gcc 4.3 that changed today, but I thought we were good to go with that.
I mean, I thought jimb's patch here (already upstreamed at rev 321):
http://code.google.com/p/google-breakpad/source/detail?r=321

Would have covered this.
(Assignee)

Updated

8 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 14

8 years ago
Created attachment 398699 [details] [diff] [review]
fix v1.4

Includes Linux build fix.
Attachment #398569 - Attachment is obsolete: true
(Assignee)

Comment 15

8 years ago
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/382cb6699240
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Blocks: 429841
Can someone look at this for 1.9.0 or are we not taking updates there?
I think it's way too much for 1.9.0, and I don't think I'd take it on any branch, honestly. Just use the 10.4 SDK.

Updated

8 years ago
Duplicate of this bug: 488111
Duplicate of this bug: 522881
Duplicate of this bug: 524969
We should just take the "fix compilation with gcc 4.4" bits on the branches. They're tiny and harmless and would stop all these bugs.
Duplicate of this bug: 530072
Duplicate of this bug: 530424

Comment 24

8 years ago
Created attachment 413951 [details] [diff] [review]
Add cstdio to various files patch v0.1 [Checkin for 1.9.2: Comment 26][Checkin for 1.9.1.8: Comment 28]

Fixes for gcc 4.4
Attachment #413951 - Flags: review?(ted.mielczarek)
Comment on attachment 413951 [details] [diff] [review]
Add cstdio to various files patch v0.1 [Checkin for 1.9.2: Comment 26][Checkin for 1.9.1.8: Comment 28]

That's fine, you'll need approval for this.

Drivers: this patch has essentially zero risk, and fixes compiling Breakpad code with GCC 4.4 (which is the default in newer distros).
Attachment #413951 - Flags: review?(ted.mielczarek)
Attachment #413951 - Flags: review+
Attachment #413951 - Flags: approval1.9.1.7?
Attachment #413951 - Flags: approval1.9.2?
Attachment #413951 - Flags: approval1.9.2? → approval1.9.2+

Comment 26

8 years ago
Comment on attachment 413951 [details] [diff] [review]
Add cstdio to various files patch v0.1 [Checkin for 1.9.2: Comment 26][Checkin for 1.9.1.8: Comment 28]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/0ab07692df85
Attachment #413951 - Attachment description: Add cstdio to various files patch v0.1 → Add cstdio to various files patch v0.1 [Checkin for 1.9.2: Comment 26]
status1.9.2: --- → final-fixed
Comment on attachment 413951 [details] [diff] [review]
Add cstdio to various files patch v0.1 [Checkin for 1.9.2: Comment 26][Checkin for 1.9.1.8: Comment 28]

Approved for 1.9.1.8, a=dveditz for release-drivers
Attachment #413951 - Flags: approval1.9.1.8? → approval1.9.1.8+

Comment 28

8 years ago
Comment on attachment 413951 [details] [diff] [review]
Add cstdio to various files patch v0.1 [Checkin for 1.9.2: Comment 26][Checkin for 1.9.1.8: Comment 28]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9e91b9342677
Attachment #413951 - Attachment description: Add cstdio to various files patch v0.1 [Checkin for 1.9.2: Comment 26] → Add cstdio to various files patch v0.1 [Checkin for 1.9.2: Comment 26][Checkin for 1.9.1.8: Comment 28]

Updated

8 years ago
status1.9.1: --- → .8-fixed
You need to log in before you can comment on or make changes to this bug.