359151 - generate symbol files from pdb if airbag is enabled

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

This should be easy, we just need to run dump_syms on every pdb corresponding to an exe or a dynamic library.  bsmedberg suggested at one point committing a binary of dump_syms.exe to CVS since you apparently can't build it with VC2k5 Express, although you can run it.  I'm not sure where we want to put the resulting .sym files, do we just leave them in the objdir?

Uploading these symbol files to the symbol server will be a different bug.

Comment 1

[Mark Mentovai]

See also Airbag bug #52.  http://code.google.com/p/airbag/issues/detail?id=52.  I'd like a precompiled PDBSourceLineWriter library in the airbag repository, since this isn't a Mozilla-specific problem.

Comment 2

[(not currently active) Ted Mielczarek]

Attachment: work in progress

Ok, so this is just a work in progress.  It's not pretty, but it runs dump_syms on shared libraries and executables, and then pulls out the first line (the MODULE line) and uses that info to put the symbol file into dist/symbols/file.pdb/guid+age/file.sym.  That path corresponds to what the simple symbol server expects.

For reference, this is what a MODULE line looks like, hence the sed grossness:
MODULE 34bf9e18-7dba-4a60-841b-eabb4d812048 2 js3250.pdb

Comment 3

[(not currently active) Ted Mielczarek]

preed, bsmedberg, what do you think of the approach in this patch?  I'm putting the symbols in dist/symbols right now.  Do we need to have a symbol path, and create a date-named subdir in there for each build?

Comments welcome.

Comment 4

[Benjamin Smedberg]

So we really have three operations going on here:

1) generate .sym from .pdb
  this should happen in rules.mk the instant we link the dll/executable

2) copy the .sym into some special directory structure

Can we do this entirely on the server side? Does the .sym file contain all the information you need (e.g. to recreate the file.pdb/guid+age/file.sym structure)

3) upload the symbols to the symbol server (farm)

This needs to work in the face of retries and as such needs to be re-runnable by the build automation after the build has completed. preed, are there other requirements for symbol upload?

Comment 5

[(not currently active) Ted Mielczarek]

(In reply to comment #4)
> 2) copy the .sym into some special directory structure
> 
> Can we do this entirely on the server side? Does the .sym file contain all the
> information you need (e.g. to recreate the file.pdb/guid+age/file.sym
> structure)

Yes, the MODULE line is the first line in the file, and contains all that info.  The nice thing about doing this client-side is that you then don't need any server logic at all, you can just treat it as a dumb filestore.  If you think it's preferable to just upload the sym files and then rename them on the server, then we can do that as well.

Comment 6

[Benjamin Smedberg]

I think that we're going to need a dynamic interface to the symbol store so that extension authors can upload .sym files to be processed by airbag (as well as plugins, etc)

Whether we want to use that interface for the tinderbox symbol uploads, or a flatfile method, is up to the release team ;-)

Comment 7

[(not currently active) Ted Mielczarek]

This patch doesn't get us symbols for NSPR/NSS.  NSPR doesn't respect --enable-debugger-info-modules, and I think NSS just does something strange.

Comment 8

[J. Paul Reed [:preed]]

(In reply to comment #4)

> This needs to work in the face of retries and as such needs to be re-runnable
> by the build automation after the build has completed. preed, are there other
> requirements for symbol upload?

Couple of things:

-- A directory structure that makes it easy [for humans] to associate symbols with build IDs.

-- There are actually some weird tunneling things going on in the current symbols version, because we have to be able to upload symbols from machines that are outside the firewall. I *think* we'll need to have some sort of support for this kind of functionality as well.

We'd be happy to review patches as necessary.

Comment 9

[(not currently active) Ted Mielczarek]

(In reply to comment #8)
> -- A directory structure that makes it easy [for humans] to associate symbols
> with build IDs.

Do we need to have a configure option to specify a directory to place the symbols?

Then we can put things in $SYMBOLDIR/$BUILDID/foo 

Comment 10

[(not currently active) Ted Mielczarek]

So yeah, this patch isn't going to cut it, since it doesn't work for NSPR/NSS.  I think I'm going to look into hooking in where the talkback work gets done:
http://lxr.mozilla.org/seamonkey/source/Makefile.in#120

Comment 11

[(not currently active) Ted Mielczarek]

Attachment: more work in progress

This is actually even uglier, but I at least think this is the right place to do this.  It hooks into the 'make splitsymbols' target, which copies pdbs to $(DIST)/$(BUILDID).  This patch makes a dir $(DIST)/$(BUILDID)/airbag-symbols, and then runs dump_syms on each pdb file, and moves the result into a directory named properly for the filesystem symbol server.  Of course I did this all in bash script inside the Makefile, so it's horrible and awful.  I need to find a less ugly way to do what I want.

Comment 12

[(not currently active) Ted Mielczarek]

Attachment: perl script to process pdbs into symbol store

I'm going to include this as part of the patch, but I thought I'd throw it up here separately for the time being.  You can pass this script a directory and then a bunch of pdb files on the command line and it will run dump_syms on each one, and place it into the proper directory structure in the directory you specified.  You can then use this directory as the symbol store for airbag's processor.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Will this be usable for those of us who create our own builds or will it only be usable with Tinderbox builds like Talkback?

Comment 14

[(not currently active) Ted Mielczarek]

This build work will be usable by anyone.  Airbag as a whole should be usable by anyone who sets up a server.  The server code will all be open source as it's developed.

Comment 15

[(not currently active) Ted Mielczarek]

Attachment: generate airbag symbol store in "make splitsymbols"

Ok, this is what I want.  This patch includes the perl script from above, with a few additions, and some logic in Makefile.in under the "splitsymbols" target.  It makes a dist/airbag-symbols/$BUILD_ID/ directory, and builds an airbag-compatible symbol store in that path.  In addition, I've made it copy the pdb files there, so we can setup the airbag symbol server to be compatible with the Microsoft Symbol Server, so our builds would become magically debuggable on Windows.  Finally, the list of .sym/.pdb files in the symbol store gets echoed to a text file in that directory, so that we could do cleanup of symbols from old builds in the future.

This patch doesn't include anything for uploading symbols to a server.  I'm not sure if it's worth getting this much checked in without that or not.  I have some idea of what I'd like to do, but it needs some work.

Comment 16

[(not currently active) Ted Mielczarek]

Attachment: generate and upload airbag symbol store

Ok, I wrote a short script based roughly on how the talkback script worked.  It takes a directory as a parameter and attempts to upload the contents of that directory to a symbol server.  preed, can you take a look and see if this would work?

Comment 17

[J. Paul Reed [:preed]]

Comment on attachment 246363 [details] [diff] [review]
generate and upload airbag symbol store


>+# This script expects the following environment variables to be set:
>+# AIRBAG_SYMBOL_SERVER  : host to upload symbols to
>+# AIRBAG_SYMBOL_USER    : username on that host
>+# AIRBAG_SYMBOL_PATH    : path on that host to put symbols in
>+#

Who's responsible for setting these environment variables?

>Index: toolkit/airbag/tools/win32/dump_syms.exe
>===================================================================
>RCS file: toolkit/airbag/tools/win32/dump_syms.exe
>diff -N toolkit/airbag/tools/win32/dump_syms.exe
>Binary files /dev/null and dump_syms.exe differ

Where does dump_syms come from?

The rest of it looks good, so this isn't an r-; just curious about the above.

Comment 18

[(not currently active) Ted Mielczarek]

(In reply to comment #17)
> Who's responsible for setting these environment variables?

I was thinking they'd be set in the tinderbox config.  It looks like the talkback upload script has this stuff hardcoded, but we want this to be usable by others as well.

> >Index: toolkit/airbag/tools/win32/dump_syms.exe
> Where does dump_syms come from?
 
This is a compiled binary from the Airbag source:
http://airbag.googlecode.com/svn/trunk/src/tools/windows/dump_syms/

I'm going to check it in compiled for convenience, and also because you can't compile it with VC Express, see comment 1.

Comment 19

[J. Paul Reed [:preed]]

(In reply to comment #18)
> (In reply to comment #17)
> > Who's responsible for setting these environment variables?
> 
> I was thinking they'd be set in the tinderbox config.  It looks like the
> talkback upload script has this stuff hardcoded, but we want this to be usable
> by others as well.

Sure, makes sense; I just wanted to know whose responsibility it was to set those variables.

When this is ready to be turned on, you can submit a patch to the tinder-configs, then; they're in mozilla/tools/tinderbox-configs/
 
> > >Index: toolkit/airbag/tools/win32/dump_syms.exe
> > Where does dump_syms come from?
> 
> This is a compiled binary from the Airbag source:
> http://airbag.googlecode.com/svn/trunk/src/tools/windows/dump_syms/
> 
> I'm going to check it in compiled for convenience, and also because you can't
> compile it with VC Express, see comment 1.

Alright; could you please include some info about how to reproduce this binary in the checkin comment, then? Which compiler you used, on what OS, etc., etc., etc. I'm fine with checking it in for convenience, but it'd be good to have some information about where it came from, in case we need to reproduce it or some such.

Comment 20

[(not currently active) Ted Mielczarek]

Checked in.  We'll deal with tinderbox integration in another bug.

You can use this now, |make splitsymbols| in the top of your source tree (or objdir) will generate the symbol store, and |make uploadsymbols| will upload them,  assuming you have the environment variables specified in upload_symbols.sh defined.