Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(165)

Issue 1340543002: Fix Mac Breakpad host tools to build in Linux cross-compile (Closed)

Created:
5 years, 3 months ago by Ted Mielczarek
Modified:
5 years ago
Reviewers:
Mark Mentovai
CC:
google-breakpad-dev_googlegroups.com
Base URL:
https://chromium.googlesource.com/breakpad/breakpad.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Fix Mac Breakpad host tools to build in Linux cross-compile We're working on building our Firefox Mac builds as a Linux cross-compile (https://bugzilla.mozilla.org/show_bug.cgi?id=921040) and we need symbol dumping to work. This change ports the Mac dump_syms tool to build and work on Linux. I've tested it and it produces identical output to running the tool on Mac. The bulk of the work here was converting src/common/mac/dump_syms.mm and src/tools/mac/dump_syms/dump_syms_tool.mm from ObjC++ to C++ and removing their use of Foundation classes in favor of standard C/C++. This won't compile out-of-the-box on Linux, it requires some Mac system headers that are not included in this patch. I have those tentatively in a separate patch to land in Gecko (http://hg.mozilla.org/users/tmielczarek_mozilla.com/mc/rev/5fb8da23c83c), but I wasn't sure if you'd be interested in having them in the Breakpad tree. We could almost certainly pare down the set of headers included there, I didn't spend too much time trying to minimize them (we primarily just need the Mach-O structs and a few associated bits). I just realized that this patch is missing updating the XCode project files (ugh). I'll fix that up in a bit. R=mark@chromium.org BUG=https://bugzilla.mozilla.org/show_bug.cgi?id=543111 Committed: https://chromium.googlesource.com/breakpad/breakpad/+/6cee755e09223833137cefa589037c1448e44fa2

Patch Set 1 #

Total comments: 25

Patch Set 2 : Fixed review comments, other fixup #

Total comments: 7

Patch Set 3 : Move mac-headers to mac_headers #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+4533 lines, -1061 lines) Patch
M Makefile.am View 1 2 6 chunks +65 lines, -3 lines 0 comments Download
M Makefile.in View 1 2 32 chunks +972 lines, -8 lines 0 comments Download
M src/common/common.gyp View 1 1 chunk +1 line, -1 line 0 comments Download
M src/common/mac/arch_utilities.cc View 1 2 chunks +108 lines, -10 lines 0 comments Download
M src/common/mac/byteswap.h View 1 2 chunks +25 lines, -0 lines 0 comments Download
M src/common/mac/dump_syms.h View 1 4 chunks +5 lines, -12 lines 0 comments Download
A + src/common/mac/dump_syms.cc View 1 9 chunks +98 lines, -75 lines 2 comments Download
D src/common/mac/dump_syms.mm View 1 chunk +0 lines, -603 lines 0 comments Download
M src/common/mac/file_id.cc View 1 2 chunks +2 lines, -1 line 0 comments Download
M src/common/mac/macho_id.cc View 1 9 chunks +17 lines, -19 lines 0 comments Download
M src/common/mac/macho_reader_unittest.cc View 1 10 chunks +15 lines, -11 lines 0 comments Download
M src/common/mac/macho_utilities.h View 1 chunk +19 lines, -8 lines 0 comments Download
M src/common/mac/macho_utilities.cc View 3 chunks +77 lines, -12 lines 0 comments Download
M src/common/mac/macho_walker.cc View 1 7 chunks +13 lines, -15 lines 0 comments Download
M src/common/stabs_reader.h View 1 1 chunk +2 lines, -3 lines 0 comments Download
A src/third_party/mac_headers/README View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/architecture/byte_order.h View 1 2 1 chunk +45 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/i386/_types.h View 1 2 1 chunk +34 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach-o/arch.h View 1 2 1 chunk +105 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach-o/fat.h View 1 2 1 chunk +64 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach-o/loader.h View 1 2 1 chunk +1402 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach-o/nlist.h View 1 2 1 chunk +312 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/boolean.h View 1 2 1 chunk +88 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/i386/boolean.h View 1 2 1 chunk +74 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/i386/vm_param.h View 1 2 1 chunk +157 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/i386/vm_types.h View 1 2 1 chunk +140 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/machine.h View 1 2 1 chunk +346 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/machine/boolean.h View 1 2 1 chunk +40 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/machine/thread_state.h View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/machine/thread_status.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/machine/vm_types.h View 1 2 1 chunk +40 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/thread_status.h View 1 2 1 chunk +94 lines, -0 lines 0 comments Download
A src/third_party/mac_headers/mach/vm_prot.h View 1 2 1 chunk +140 lines, -0 lines 0 comments Download
M src/tools/mac/dump_syms/dump_syms.xcodeproj/project.pbxproj View 1 5 chunks +8 lines, -8 lines 0 comments Download
A + src/tools/mac/dump_syms/dump_syms_tool.cc View 1 6 chunks +12 lines, -14 lines 0 comments Download
D src/tools/mac/dump_syms/dump_syms_tool.mm View 1 chunk +0 lines, -257 lines 0 comments Download
M src/tools/mac/tools_mac.gypi View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 17 (0 generated)
Ted Mielczarek
5 years, 3 months ago (2015-09-11 12:41:48 UTC) #1
Mark Mentovai
I think we could take the headers in-tree in a location off to the side ...
5 years, 3 months ago (2015-09-11 15:49:51 UTC) #2
Ted Mielczarek
Ok, I will pull the headers in in an updated patch. If I do that ...
5 years, 3 months ago (2015-09-11 15:58:01 UTC) #3
Mark Mentovai
Ted Mielczarek wrote: > https://codereview.chromium.org/1340543002/diff/1/src/common/mac/arch_utilities.cc#newcode116 > src/common/mac/arch_utilities.cc:116: #ifndef __APPLE__ > On 2015/09/11 15:49:50, Mark Mentovai ...
5 years, 3 months ago (2015-09-11 16:19:35 UTC) #4
Ted Mielczarek
I think I addressed all your review comments. I added src/tools/mac/dump_syms to the autotools build ...
5 years, 3 months ago (2015-09-15 12:58:36 UTC) #5
Mark Mentovai
https://codereview.chromium.org/1340543002/diff/20001/Makefile.am File Makefile.am (right): https://codereview.chromium.org/1340543002/diff/20001/Makefile.am#newcode572 Makefile.am:572: -I$(top_srcdir)/src/third_party/mac-headers \ You don’t want to do this on ...
5 years, 3 months ago (2015-09-15 16:27:52 UTC) #6
Ted Mielczarek
https://codereview.chromium.org/1340543002/diff/20001/Makefile.am File Makefile.am (right): https://codereview.chromium.org/1340543002/diff/20001/Makefile.am#newcode572 Makefile.am:572: -I$(top_srcdir)/src/third_party/mac-headers \ On 2015/09/15 16:27:52, Mark Mentovai - August ...
5 years, 3 months ago (2015-09-15 16:51:15 UTC) #7
Mark Mentovai
LGTM https://codereview.chromium.org/1340543002/diff/20001/Makefile.am File Makefile.am (right): https://codereview.chromium.org/1340543002/diff/20001/Makefile.am#newcode572 Makefile.am:572: -I$(top_srcdir)/src/third_party/mac-headers \ On 2015/09/15 16:51:15, Ted Mielczarek wrote: ...
5 years, 3 months ago (2015-09-15 18:24:23 UTC) #8
Mark Mentovai
Update the CL description too.
5 years, 3 months ago (2015-09-15 18:24:58 UTC) #9
Ted Mielczarek
https://codereview.chromium.org/1340543002/diff/20001/Makefile.am File Makefile.am (right): https://codereview.chromium.org/1340543002/diff/20001/Makefile.am#newcode572 Makefile.am:572: -I$(top_srcdir)/src/third_party/mac-headers \ On 2015/09/15 18:24:23, Mark Mentovai - August ...
5 years, 3 months ago (2015-09-16 10:45:32 UTC) #10
Ted Mielczarek
Committed patchset #3 (id:40001) manually as 6cee755e09223833137cefa589037c1448e44fa2 (presubmit successful).
5 years, 3 months ago (2015-09-16 10:47:04 UTC) #11
Ted Mielczarek
On 2015/09/15 18:24:58, Mark Mentovai - August is over wrote: > Update the CL description ...
5 years, 3 months ago (2015-09-16 10:48:55 UTC) #12
Mark Mentovai
https://codereview.chromium.org/1340543002/diff/20001/Makefile.am File Makefile.am (right): https://codereview.chromium.org/1340543002/diff/20001/Makefile.am#newcode572 Makefile.am:572: -I$(top_srcdir)/src/third_party/mac-headers \ On 2015/09/16 10:45:32, Ted Mielczarek wrote: > ...
5 years, 3 months ago (2015-09-16 14:06:24 UTC) #13
labath
Hi, this change breaks "make install" on linux. It complains about having two executables named ...
5 years, 1 month ago (2015-11-17 16:24:38 UTC) #14
Ted Mielczarek
On 2015/11/17 16:24:38, labath wrote: > Hi, > > this change breaks "make install" on ...
5 years, 1 month ago (2015-11-17 20:52:57 UTC) #15
Ted Mielczarek
On 2015/11/17 20:52:57, Ted Mielczarek wrote: > I did notice this and forgot to fix ...
5 years ago (2015-11-25 14:14:24 UTC) #16
labath
5 years ago (2015-11-25 16:30:22 UTC) #17
Message was sent while issue was closed.
On 2015/11/25 14:14:24, Ted Mielczarek wrote:
> On 2015/11/17 20:52:57, Ted Mielczarek wrote:
> > I did notice this and forgot to fix it. I'm OK with either putting it in
> > noinst_PROGRAMS or renaming it (we could call it dump_syms_mac).
> 
> I submitted a CL to rename the binary:
> https://codereview.chromium.org/1474673004/

Thanks a lot! :)

(I am sorry for the late response, I only noticed your comments now. I was
hoping I'd get notifications by email, but apparently commenting does not
auto-subscribe me here. :) )

Powered by Google App Engine
This is Rietveld 408576698