Closed
Bug 378463
Opened 17 years ago
Closed 17 years ago
build time logic to extract debug symbols on OS X
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(1 file, 4 obsolete files)
18.32 KB,
patch
|
Details | Diff | Splinter Review |
The current build logic only works on Win32. Shebs, I know you looked at this, do you have anything you could post here as a starting point?
Assignee | ||
Comment 1•17 years ago
|
||
Ok, this patch works for me. It makes us build the dump_syms tool, and use it in |make buildsymbols|. I had to tweak a few things in make_symbol_store.pl, since the .sym file MODULE line changed a bit. That means I'll have to checkin a new dump_syms.exe for Windows as well. I don't know how this interacts with Universal builds. I do know that the OS X dump_syms offers an architecture flag, so you can call it with |-a ppc| or |-a x86|, so it might be that we need to run it one per arch. Mark, any insight you can offer on that?
Comment 2•17 years ago
|
||
A universal build is really just two thin builds. Figure out if you're targeting x86 or ppc and always call dump_syms with an architecture argument. You should also make sure that your dump_syms is built with the host tools, not the target tools, to ensure that it'll run on a ppc build host during the x86 cross portion of a universal build.
Assignee | ||
Comment 3•17 years ago
|
||
Yeah, I figured I needed to change that to a HOST_PROGRAM. My only concern is that we're running dump_syms in a post-build step: http://mxr.mozilla.org/mozilla/source/tools/tinderbox/post-mozilla-rel.pl#1298 So how do we know what objdir to make in?
Assignee | ||
Comment 4•17 years ago
|
||
Nevermind, bsmedberg pointed out this lovely bit of Tinderbox code to me: http://mxr.mozilla.org/mozilla/source/tools/tinderbox/post-mozilla-rel.pl#1213 So my current plan is: 1) In mozilla/Makefile.in, detect universal builds, and pass -a ppc,i386 to make-symbol-store.pl 2) In make-symbol-store.pl, loop running dump_syms for each arch specified after -a, and pass that to dump_syms. If someone is doing a non-universal build, and I don't pass -a to dump_syms, it will just do the right thing, right?
Assignee | ||
Comment 5•17 years ago
|
||
Ok, this adds universal build support. dump_syms is built as a HOST_PROGRAM, the list of architectures is passed to make_symbol_store.pl, and it calls dump_syms once per arch. In addition, this patch changes the UB flight.mk to not strip the binaries when they are merged into a UB, so we can dump the UB. They will still get stripped in the final packaging.
Attachment #262878 -
Attachment is obsolete: true
Attachment #263047 -
Flags: review?(mark)
Assignee | ||
Comment 6•17 years ago
|
||
Whoops, forgot to diff new files. As I mentioned before, I will also need to checkin a new dump_syms.exe along with this patch.
Attachment #263047 -
Attachment is obsolete: true
Attachment #263142 -
Flags: review?(mark)
Attachment #263047 -
Flags: review?(mark)
Comment 7•17 years ago
|
||
Comment on attachment 263142 [details] [diff] [review] with universal build support >+SYM_FIND_CMD := /bin/find . -path "./dist" -prune -o -name "*.exe" \ You don't need quotes around ./dist any more than you'd need them around, say, -prune. You also don't need the ./ . >+ -o -name "*.dll" -o -name "*.EXE" | sed "s/\.[^\.]*$$/\.pdb/" Indent a bit more to make it clearer that it's a continuation? >+# we want to copy PDB files on Windows >+MAKE_SYM_STORE_ARGS := -c >+DUMP_SYMS_BIN := $(topsrcdir)/toolkit/airbag/tools/win32/dump_syms.exe >+endif >+ifeq ($(OS_ARCH),Darwin) >+# need to pass arch flags for universal builds >+ifdef UNIVERSAL_BINARY >+MAKE_SYM_STORE_ARGS := -a "ppc i386" >+SYM_DIST := $(DIST)/universal >+else >+MAKE_SYM_STORE_ARGS := >+SYM_DIST := $(DIST) >+endif This might not work with a pure cross build - say you're on a ppc host building for x86, the ppc dump_syms will try to dump ppc, but there is no ppc code to dump. How about setting MAKE_SYM_STORE_ARGS to -a whatever depending on the target architecture in the non-UNIVERSAL_BINARY case? >Index: toolkit/airbag/tools/make_symbol_store.pl > # file.pdb/GUID+age/file.sym We've taken to referring to what we used to call guid+age or uuid+age as "identifier" now, since there might not be uuid or guid components at all. You may want to say so somewhere.
Attachment #263142 -
Flags: review?(mark) → review+
Assignee | ||
Comment 8•17 years ago
|
||
Addressed Mark's comments, I use -a $(OS_TEST) in the non-universal case. I need to make sure this still works on Win32, and then I'll check it in.
Attachment #263142 -
Attachment is obsolete: true
Assignee | ||
Comment 9•17 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 10•17 years ago
|
||
I backed this out due to bustage. ~/firefox/mozilla> cvs commit -m "backout patch for bug 378463 due to bustage" toolkit/airbag/airbag/src/tools/mac/dump_syms toolkit/xre/Makefile.in build/macosx/universal/flight.mk toolkit/airbag/tools/make_symbol_store.pl toolkit/airbag/airbag/src/common/mac/Makefile.in toolkit/airbag/airbag/src/common toolkit/airbag/Makefile.in Makefile.in toolkit/airbag/tools/win32/dump_syms.exe cvs commit: Examining toolkit/airbag/airbag/src/tools/mac/dump_syms cvs commit: Examining toolkit/airbag/airbag/src/tools/mac/dump_syms/dump_syms.xcodeproj cvs commit: Examining toolkit/airbag/airbag/src/common cvs commit: Examining toolkit/airbag/airbag/src/common/linux cvs commit: Examining toolkit/airbag/airbag/src/common/mac cvs commit: Examining toolkit/airbag/airbag/src/common/windows Checking in toolkit/xre/Makefile.in; /cvsroot/mozilla/toolkit/xre/Makefile.in,v <-- Makefile.in new revision: 1.71; previous revision: 1.70 done Checking in build/macosx/universal/flight.mk; /cvsroot/mozilla/build/macosx/universal/flight.mk,v <-- flight.mk new revision: 1.6; previous revision: 1.5 done Checking in toolkit/airbag/tools/make_symbol_store.pl; /cvsroot/mozilla/toolkit/airbag/tools/make_symbol_store.pl,v <-- make_symbol_store.pl new revision: 1.4; previous revision: 1.3 done Checking in toolkit/airbag/airbag/src/common/mac/Makefile.in; /cvsroot/mozilla/toolkit/airbag/airbag/src/common/mac/Makefile.in,v <-- Makefile.in new revision: 1.3; previous revision: 1.2 done Checking in toolkit/airbag/Makefile.in; /cvsroot/mozilla/toolkit/airbag/Makefile.in,v <-- Makefile.in new revision: 1.8; previous revision: 1.7 done Checking in Makefile.in; /cvsroot/mozilla/Makefile.in,v <-- Makefile.in new revision: 1.372; previous revision: 1.371 done Checking in toolkit/airbag/tools/win32/dump_syms.exe; /cvsroot/mozilla/toolkit/airbag/tools/win32/dump_syms.exe,v <-- dump_syms.exe new revision: 1.3; previous revision: 1.2 done Removing toolkit/airbag/airbag/src/tools/mac/dump_syms/Makefile.in; /cvsroot/mozilla/toolkit/airbag/airbag/src/tools/mac/dump_syms/Makefile.in,v <-- Makefile.in new revision: delete; previous revision: 1.1 done Removing toolkit/airbag/airbag/src/common/Makefile.in; /cvsroot/mozilla/toolkit/airbag/airbag/src/common/Makefile.in,v <-- Makefile.in new revision: delete; previous revision: 1.1 done
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 11•17 years ago
|
||
This fixes the test Makefile as well, which is what caused the bustage last night. Checked in again.
Attachment #263363 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•