Last Comment Bug 510920 - Sync to breakpad revision 350 for breakpad fix 248, 10.5+ SDK compat
: Sync to breakpad revision 350 for breakpad fix 248, 10.5+ SDK compat
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Breakpad Integration (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- normal (vote)
: ---
Assigned To: Josh Aas
:
Mentors:
: 488111 522881 524969 530072 530424 (view as bug list)
Depends on:
Blocks: 429841 501436 514188
  Show dependency treegraph
 
Reported: 2009-08-17 10:41 PDT by Josh Aas
Modified: 2009-12-21 16:49 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
final-fixed
.8-fixed


Attachments
fix v1.0 (1.75 MB, patch)
2009-08-27 10:31 PDT, Josh Aas
no flags Details | Diff | Review
fix v1.1 (1.75 MB, patch)
2009-08-27 14:51 PDT, Josh Aas
no flags Details | Diff | Review
fix v1.2 (1.31 MB, patch)
2009-09-03 12:04 PDT, Josh Aas
ted: review+
Details | Diff | Review
fix v1.3 (1.30 MB, patch)
2009-09-03 21:06 PDT, Josh Aas
no flags Details | Diff | Review
fix v1.4 (1.30 MB, patch)
2009-09-04 10:00 PDT, Josh Aas
no flags 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] (3.07 KB, patch)
2009-11-22 13:32 PST, Ian Neal
ted: review+
benjamin: approval1.9.2+
dveditz: approval1.9.1.8+
Details | Diff | Review

Description Josh Aas 2009-08-17 10:41:58 PDT
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
Comment 1 Josh Aas 2009-08-27 10:31:02 PDT
Created attachment 397043 [details] [diff] [review]
fix v1.0
Comment 2 Josh Aas 2009-08-27 14:51:17 PDT
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
Comment 3 Ted Mielczarek [:ted.mielczarek] 2009-09-02 06:31:51 PDT
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.
Comment 4 Ted Mielczarek [:ted.mielczarek] 2009-09-02 08:03:52 PDT
Updating summary to narrow scope.
Comment 5 Josh Aas 2009-09-03 12:04:44 PDT
Created attachment 398432 [details] [diff] [review]
fix v1.2

Breakpad revision 350 patch.
Comment 6 Ted Mielczarek [:ted.mielczarek] 2009-09-03 14:11:05 PDT
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!
Comment 7 Ted Mielczarek [:ted.mielczarek] 2009-09-03 15:55:24 PDT
This is the fix that we have locally that's not upstreamed:
http://hg.mozilla.org/mozilla-central/rev/cfef2762ecb3
Comment 8 Josh Aas 2009-09-03 17:41:26 PDT
That fix appears to have been upstreamed, breakpad svn revision 313 so we pick it up when updating to revision 350.
Comment 9 Josh Aas 2009-09-03 20:42:54 PDT
pushed to mozilla-central with ted's comments addressed

http://hg.mozilla.org/mozilla-central/rev/e8c01867056a
Comment 10 Josh Aas 2009-09-03 21:04:59 PDT
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?
Comment 11 Josh Aas 2009-09-03 21:06:49 PDT
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.
Comment 12 Ted Mielczarek [:ted.mielczarek] 2009-09-04 03:36:14 PDT
Could be the switch to gcc 4.3 that changed today, but I thought we were good to go with that.
Comment 13 Ted Mielczarek [:ted.mielczarek] 2009-09-04 03:58:20 PDT
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.
Comment 14 Josh Aas 2009-09-04 10:00:55 PDT
Created attachment 398699 [details] [diff] [review]
fix v1.4

Includes Linux build fix.
Comment 15 Josh Aas 2009-09-04 10:16:03 PDT
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/382cb6699240
Comment 16 [On PTO until 6/29] 2009-09-10 15:34:38 PDT
Can someone look at this for 1.9.0 or are we not taking updates there?
Comment 17 Ted Mielczarek [:ted.mielczarek] 2009-09-10 15:47:28 PDT
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.
Comment 18 Makoto Kato [:m_kato] (PTO 6/20-21, 6/24) 2009-09-28 03:19:21 PDT
*** Bug 488111 has been marked as a duplicate of this bug. ***
Comment 19 Benjamin Smedberg [:bsmedberg] 2009-10-17 09:02:22 PDT
*** Bug 522881 has been marked as a duplicate of this bug. ***
Comment 20 Benjamin Smedberg [:bsmedberg] 2009-10-28 08:19:09 PDT
*** Bug 524969 has been marked as a duplicate of this bug. ***
Comment 21 Ted Mielczarek [:ted.mielczarek] 2009-10-28 08:25:05 PDT
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.
Comment 22 Ted Mielczarek [:ted.mielczarek] 2009-11-20 07:05:27 PST
*** Bug 530072 has been marked as a duplicate of this bug. ***
Comment 23 Ted Mielczarek [:ted.mielczarek] 2009-11-22 11:37:57 PST
*** Bug 530424 has been marked as a duplicate of this bug. ***
Comment 24 Ian Neal 2009-11-22 13:32:46 PST
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
Comment 25 Ted Mielczarek [:ted.mielczarek] 2009-11-22 14:15:10 PST
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).
Comment 26 Ian Neal 2009-12-16 07:25:20 PST
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
Comment 27 Daniel Veditz [:dveditz] 2009-12-21 15:46:03 PST
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
Comment 28 Ian Neal 2009-12-21 16:49:06 PST
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

Note You need to log in before you can comment on or make changes to this bug.