458553 - Build NSS with debug symbols - like the rest of Firefox

Status: VERIFIED FIXED

(Core :: Security: PSM, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

In Firefox builds (and SeaMonkey, etc.) optimized builds are generally 
built WITH debug symbols.  For Windows, this is done by specifying -Zi 
on the cl command line.  In NSS's own makefiles, normally, an "optimized" 
build is built without any debug symbols.  Consequently, Mozilla browser
stack traces lack details for NSS functions in stacks, but have pretty 
complete details for non-NSS functions in stacks. 

When NSS is built as part of the Mozilla clients, it is built by one of 
the makefiles for PSM.  That PSM makefile overrides a number of things
about the wan NSS is built, so that browser builds of NSS differ from NSS's
own stand alone builds in several ways.  

I'm requesting that PSM's Makefiles introduce yet another difference in 
the way that PSM builds NSS from the way that NSS stand-alone builds 
normally work.  I'm asking that PSM build NSS with debug symbols, at 
least on Windows, by causing the cl command line to include -Zi.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Hmm.  I observe that NSS's makefile for windows bears the following lines:

        ifdef MOZ_DEBUG_SYMBOLS
                OPTIMIZER += -Zi
        endif

That was put there, long ago, as part of the resolution to this very same
issue.  This begs the question: 

Does PSM no longer define MOZ_DEBUG_SYMBOLS when it builds NSS ?

Comment 2

[(not currently active) Ted Mielczarek]

We generally leave that up to the developer (or tinderbox) to put in the mozconfig file.

Comment 3

[Kaspar]

Attachment: Proposed patch

(In reply to comment #1)
> Does PSM no longer define MOZ_DEBUG_SYMBOLS when it builds NSS ?

Yes, apparently this changed between Firefox 3.0 and Firefox 3.1/3.5 (could not yet figure out why exactly).

In any case, the attached patch (for both mozilla-central and mozilla-1.9.1) fixes this problem, by making sure that PSM's Makefile forwards MOZ_DEBUG_SYMBOLS when it builds NSS. Not sure whom I should ask for review, though.

Comment 4

[(not currently active) Ted Mielczarek]

I don't think that makefile ever had any reference to MOZ_DEBUG_SYMBOLS, see the 1.9.0 version:
http://mxr.mozilla.org/mozilla/source/security/manager/Makefile.in

See bug 468701 comment 3 for what I think happened to our symbols on Windows.

Comment 5

[(not currently active) Ted Mielczarek]

Kaspar: I think you could instead just "export MOZ_DEBUG_SYMBOLS", like we do with DLLFLAGS:
http://mxr.mozilla.org/mozilla/source/security/manager/Makefile.in#171
(although without the ifeq that that line uses)

Comment 6

[(not currently active) Ted Mielczarek]

Kaspar: if you write a patch using export, r?me on it, and we can get it on the 1.9.1 branch.

Comment 7

[Kaspar]

Attachment: Patch v2 (use export instead of DEFAULT_GMAKE_FLAGS)

(In reply to comment #6)
> Kaspar: if you write a patch using export, r?me on it, and we can get it on the
> 1.9.1 branch.

Like this? I kept it at the same position, since it's a Windows-only option.

> See bug 468701 comment 3 for what I think happened to our symbols on Windows.

It's indeed true that "the difference is due to setting MOZ_DEBUG_SYMBOLS=1 in the mozconfig [...] versus setting it in the environment in tinderbox" - if you look at the full build logs of the 3.5 nightlies (e.g. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1242938896.1242950605.22629.gz&fulltext=1), you'll notice that the environment does not include MOZ_DEBUG_SYMBOLS - in contrast to the 3.0 builds.

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 379122 [details] [diff] [review]
Patch v2 (use export instead of DEFAULT_GMAKE_FLAGS)

r=me

Comment 9

[Kaspar]

Comment on attachment 379122 [details] [diff] [review]
Patch v2 (use export instead of DEFAULT_GMAKE_FLAGS)

Requesting approval (note that afterwards I would need someone to check this in for me, I don't have commit access).

Comment 10

[Benjamin Smedberg]

Comment on attachment 379122 [details] [diff] [review]
Patch v2 (use export instead of DEFAULT_GMAKE_FLAGS)

pre-emptive approval for branch once this lands greenly in -central

Comment 11

[(not currently active) Ted Mielczarek]

Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/97fcaf4717fb
(will push to 1.9.1 once that cycles)

Comment 12

[(not currently active) Ted Mielczarek]

VERIFIED looking at the symbol package here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1243005929/firefox-3.6a1pre.en-US.win32.crashreporter-symbols.zip

Comment 13

[Johnathan Nightingale [:johnath]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c3f95850ba10