338224 - implement --enable-debug-symbols (nspr4.dll and --disable-debug yields "no rule to make nspr4.pdb needed by export")

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Matthew Daniel]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

This actually applies to Thunderbird CVS (where I first noticed it) and Firefox CVS, where I confirmed.

I tried (honest!) to track down the rule that "export" was complaining about, to clear the $(LIB).pdb dependency, but was stymied.

Reproducible: Always

Steps to Reproduce:
1. Checkout from CVS
2. include "ac_add_options --disable-debug" in your %MOZCONFIG%
3. make -f client.mk build
4. kaboom
5. rm -rf objdir
6. switch to "ac_add_options --enable-debug"
7. observe since .pdb file is created, build is happy; makes product REALLY verbose




Here is the build environment, FWIW:

set VCVARS=c:\Program Files\Microsoft Visual Studio 8\VC\bin\vcvars32.bat
set MSSDK=c:\Program Files\Microsoft Platform SDK for Windows Server 2003 R2\
set MOZ_TOOLS=c:\moz\moztools
set CYGWINBASE=c:\cygwin
set PATH=%CYGWINBASE%\bin;%PATH%
call "%VCVARS%"
set PATH=%MSSDK%\bin;%PATH%
set INCLUDE=%MSSDK%\include;%INCLUDE%
set LIB=%MSSDK%\lib;%LIB%
set PATH=%PATH%;%MOZ_TOOLS%\bin
set NO_MFC=1

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I can reproduce this problem.

--disable-optimize and --disable-debug ... failed
--enable-optimize  and --disable-debug ... succeeded
--disable-optimize and --enable-debug  ... succeeded

I cannot build the non-optimezed and non-debug build.

Comment 2

[timeless]

I've worked around this problem every couple of years.

wtc: link -DEBUG != BE debug. and pdbs don't have to be shipped. they should always be generated.

Comment 3

[timeless]

Attachment: always pass "create PDB flag" to linker

Comment 4

[Martijn Wargers (dead)]

Any chance of getting the patch reviewed and checked in?

Comment 5

[Martijn Wargers (dead)]

Ugh, and now I'm even getting this build error after I applied the patch.

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> I can reproduce this problem.
> 
> --disable-optimize and --disable-debug ... failed
> --enable-optimize  and --disable-debug ... succeeded
> --disable-optimize and --enable-debug  ... succeeded

That's what I get with current FF/mozilla-central (and SM/comm-central).

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a) Updated to current mozilla-central

(In reply to comment #5)
> Ugh, and now I'm even getting this build error after I applied the patch.

This patch works for me :-)

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)
> > --disable-optimize and --disable-debug ... failed
> > --enable-optimize  and --disable-debug ... succeeded
> > --disable-optimize and --enable-debug  ... succeeded

And, fwiw,
--enable-optimize  and --enable-debug ... succeeded

Comment 9

[[e-mail]]

Surely it would be better for nspr's configure to set BUILD_OPT depending on whether MOZ_DEBUG is unset rather than blindly copying MOZ_OPTIMIZE?

Comment 10

[John J. Barton]

Can someone tell me what the current status of this bug is? I need to build 
--disable-optimize and --disable-debug
so I can used the debugger when I need it. Is it possible?
Thanks,
John

Comment 11

[John J. Barton]

BTW I get the nspr4.pdb missing with
ac_add_options --disable-vista-sdk-requirements
ac_add_options --enable-application=browser
ac_add_options --disable-optimize 
ac_add_options --disable-parental-controls
ac_add_options --enable-debugger-info-modules=yes
export MOZ_DEBUG_SYMBOLS=1
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-@CONFIG_GUESS@

what can I do?

Comment 12

[John J. Barton]

downloaded second attachment to this bug as nsprbug.diff
cd src/nsprpub
patch -p1  configure.in  ../../nsprbug.diff
cd ..
make -f client.mk build

result was
 *** No rule to make target `nspr4.pdb', needed by `export'.  Stop.

Comment 13

[John J. Barton]

This .mozconfig builds (added --enable-debug)
ac_add_options --disable-vista-sdk-requirements
ac_add_options --enable-application=browser
ac_add_options --disable-optimize 
ac_add_options --disable-parental-controls
ac_add_options --enable-debug
ac_add_options --enable-debugger-info-modules=yes
export MOZ_DEBUG_SYMBOLS=1
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-@CONFIG_GUESS@

However if I delete nsprpub/pr/src/nspr4.pdb and run make, then I get the No rule message.

So in the successful case the .pdb file is not created by a rule, but rather as a side effect of the compile leading to nspr.dll. Indeed if I delete the nspr4.dll file and make, then nspr4.pdb is built. Looks like the 'link' command extracts the debug info from the .obj files and puts it into the .pdb file. The link has no explicit "/Fd" to namem the pdb, rather it seems to be using the .dll name.  Presumably some setting on the compile (Zi) leading to the .obj files will cause the .pdb to result from the link. 

If this is all correct, then pdb should never be required by export. If you compile with Zi you get pdb, else not.

Comment 14

[John J. Barton]

In nsprpub directory under the .mozconfig of comment 13, the compile option that causes debug output is probably "Z7" rather than Zi.

Comment 15

[John J. Barton]

client.mk shows that nsprpub has its own configure:
# 'configure' scripts generated by autoconf.
CONFIGURES := $(TOPSRCDIR)/configure
CONFIGURES += $(TOPSRCDIR)/nsprpub/configure

That may explain why this problem only occurs in the nsprpub directory.

In nsprpub/configure MOZ_DEBUG_SYMBOLS is only used for MOZ_OPTIMIZE
        if test -n "$MOZ_OPTIMIZE"; then
            if test -n "$MOZ_PROFILE"; then
                _OPTIMIZE_FLAGS="$_OPTIMIZE_FLAGS -Z7"
            fi
            if test -n "$MOZ_DEBUG_SYMBOLS"; then
                _OPTIMIZE_FLAGS="$_OPTIMIZE_FLAGS -Zi"
            fi
            if test -n "$MOZ_PROFILE" -o -n "$MOZ_DEBUG_SYMBOLS"; then
                DLLFLAGS="$DLLFLAGS -OPT:REF"
                LDFLAGS="$LDFLAGS -OPT:REF"
            fi
        fi

But I am assuming that MOZ_DEBUG_SYMBOLS should be independent of MOZ_OPTIMIZE.

Comment 16

[John J. Barton]

(In reply to comment #8)
> (In reply to comment #6)
> > > --disable-optimize and --disable-debug ... failed
> > > --enable-optimize  and --disable-debug ... succeeded
> > > --disable-optimize and --enable-debug  ... succeeded
> 
> And, fwiw,
> --enable-optimize  and --enable-debug ... succeeded

That makes sense because nsprpub/configure says:
if test -n "$MOZ_OPTIMIZE"; then
    CFLAGS="$CFLAGS $_OPTIMIZE_FLAGS"
    CXXFLAGS="$CXXFLAGS $_OPTIMIZE_FLAGS"
fi

if test -n "$MOZ_DEBUG"; then
    CFLAGS="$CFLAGS $_DEBUG_FLAGS"
    CXXFLAGS="$CXXFLAGS $_DEBUG_FLAGS"
fi
So if you disable both, then Z7 is not added. If you want pdb you have to optimize or debug.

Comment 17

[Serge Gautherie (:sgautherie)]

wtc, ping ?

Comment 18

[John J. Barton]

I found a solution that allows debugging with Visual Studio 2005 but no stream of tracing on startup:

ac_add_options --disable-vista-sdk-requirements
ac_add_options --enable-application=browser
ac_add_options --disable-optimize
ac_add_options --disable-parental-controls
export MOZ_DEBUG_SYMBOLS=1
ac_add_options --enable-debugger-info-modules=yes
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-@CONFIG_GUESS@

The only difference between here and Comment 11 is the position of the export. I don't remember reading anything about order of the mozconfig but if it is essentially a script it makes sense that the constants have to be defined first.

My guess is that some people have the export in their environment variables so they never have this bug. (More evidence that environment variables should never be used in a shared build system).

To come to this conclusion I did:
make -f client.mk clean
make -f client.mk build
If the clean is not perfect it is possible that the conclusion would be incorrect.

Comment 19

[Nelson Bolyard (seldom reads bugmail)]

I'm not sure I consider this bug valid.
A .pdb file is a file for debugging.  It contains the debugging symbols.
When you say --disable-debug, you're saying "I don't want debugging stuff"
and that includes debugging symbols.

Comment 20

[John J. Barton]

I don't agree. I want some debugging stuff: symbols. I don't want other debugging stuff: tracing. Various attempts to accomplish this include trying --disable-debug as in the title, but it is not in the end needed. 

In addition the build should not fail with "no rule ..." with any combination of options.  Its a bug in the make system.  Maybe too hard to fix that but I in my opinion no target should depend upon .pdb since it is a side-effect of compile, not a result of any command.

I think the minimal resolution is to ensure that 
export MOZ_DEBUG_SYMBOLS=1
is set early in the .mozconfig file.

Comment 21

[Nelson Bolyard (seldom reads bugmail)]

You didn't tell us what makefile reports the error you cited in comment 12

Comment 22

[John J. Barton]

I'm not sure what you are asking. The makefile was, as I said in comment 12, "client.mk". The .mozconfig I used in comment 12 was the one in comment 11. The error occurs when make enters nsprpub as a describe in comment 15.

Comment 23

[Nelson Bolyard (seldom reads bugmail)]

John, Make invokes itself recursively as it traverses the tree of source 
file directories.  In each directory, it invokes that directory's makefile.
The error you reported occurred in some makefile for some directory.
It seems very unlikely that the error occurred in client.mk, but more 
likely that it occurred in some subordinate makefile.  The question is:
which one?

Comment 24

[John J. Barton]

Since the error occurs in nsprpub, I presume the subordinate make file is nsprpub/Makefile. But since these are generated make files, you have to look into the config system to fix the problem.

Comment 25

[Nelson Bolyard (seldom reads bugmail)]

John, there's no need to presume anything.  The output of the make should 
make it very explicitly clear where the problem occurs.  There should be a 
LOT more lines of output surrounding the one you quoted:
>  *** No rule to make target `nspr4.pdb', needed by `export'.  Stop.
please share them with us.

Comment 26

[John J. Barton]

Sorry Nelson, I already did a lot of investigation on this problem and posted my findings. I have a working solution now so I don't want to undo it all unless there is a good reason.  If you try the .mozconfig files I posted and get different results, I'll be happy to try to sort that out.

Comment 27

[John J. Barton]

(In reply to comment #25)
> John, there's no need to presume anything.  The output of the make should 
> make it very explicitly clear where the problem occurs.  There should be a 
> LOT more lines of output surrounding the one you quoted:
> >  *** No rule to make target `nspr4.pdb', needed by `export'.  Stop.
> please share them with us.
Here is the make -d output
$ tail -40 make.out
    Trying pattern rule with stem `nspr4.pdb.cpp'.
    Trying implicit prerequisite `RCS/nspr4.pdb.cpp,v'.
    Trying pattern rule with stem `nspr4.pdb.cpp'.
    Trying implicit prerequisite `RCS/nspr4.pdb.cpp'.
    Trying pattern rule with stem `nspr4.pdb.cpp'.
    Trying implicit prerequisite `s.nspr4.pdb.cpp'.
    Trying pattern rule with stem `nspr4.pdb.cpp'.
    Trying implicit prerequisite `SCCS/s.nspr4.pdb.cpp'.
   Trying pattern rule with stem `nspr4.pdb'.
   Trying implicit prerequisite `nspr4.pdb.s'.
   Looking for a rule with intermediate file `nspr4.pdb.s'.
    Avoiding implicit rule recursion.
    Trying pattern rule with stem `nspr4.pdb.s'.
    Trying implicit prerequisite `nspr4.pdb.s,v'.
    Trying pattern rule with stem `nspr4.pdb.s'.
    Trying implicit prerequisite `RCS/nspr4.pdb.s,v'.
    Trying pattern rule with stem `nspr4.pdb.s'.
    Trying implicit prerequisite `RCS/nspr4.pdb.s'.
    Trying pattern rule with stem `nspr4.pdb.s'.
    Trying implicit prerequisite `s.nspr4.pdb.s'.
    Trying pattern rule with stem `nspr4.pdb.s'.
    Trying implicit prerequisite `SCCS/s.nspr4.pdb.s'.
   No implicit rule found for `nspr4.pdb'.
   Finished prerequisites of target file `nspr4.pdb'.
  Must remake target `nspr4.pdb'.
make[5]: Leaving directory `/g/mozilla/mozilla-central/src/obj-i686-pc-mingw32/n
sprpub/pr/src'
Reaping losing child 0x0a0334f0 PID 3288
Removing child 0x0a0334f0 PID 3288 from chain.
make[4]: Leaving directory `/g/mozilla/mozilla-central/src/obj-i686-pc-mingw32/n
sprpub/pr'
Reaping losing child 0x0a034600 PID 2776
Removing child 0x0a034600 PID 2776 from chain.
make[3]: Leaving directory `/g/mozilla/mozilla-central/src/obj-i686-pc-mingw32/n
sprpub'
Reaping losing child 0x0a056948 PID 3092
Removing child 0x0a056948 PID 3092 from chain.
make[2]: Leaving directory `/g/mozilla/mozilla-central/src/obj-i686-pc-mingw32'
Reaping losing child 0x0a0567e8 PID 2428
Removing child 0x0a0567e8 PID 2428 from chain.
make[1]: Leaving directory `/g/mozilla/mozilla-central/src/obj-i686-pc-mingw32'
Reaping losing child 0x0a0445e0 PID 2628
Removing child 0x0a0445e0 PID 2628 from chain.

Comment 28

[John J. Barton]

(In reply to comment #20)
...
> I think the minimal resolution is to ensure that 
> export MOZ_DEBUG_SYMBOLS=1
> is set early in the .mozconfig file.

I re-cloned hg mozilla-central in a new directory and copied the .mozconfig from comment 18 and the build fails with the same pdb message.

So something I changed in the config files fixed the problem, not the change to .mozconfig. 

And once again I am not able to build.

Comment 29

[John J. Barton]

This error message means the .pdb file with debug symbols has not been built.
   (Based on the error message and MS docs)
The setting that creates the .pdb is -Zi to C compiler
   (Based on the MS docs and line 1583 of src/nsprpub/configure.in
   http://mxr.mozilla.org/mozilla-central/source/nsprpub/configure.in#1562)
The C compiler gets the setting from either $_OPTIMIZE_FLAGS *or* $_DEBUG_FLAGS in configure.in, line 2661
    http://mxr.mozilla.org/mozilla-central/source/nsprpub/configure.in#2661

MOZ_OPTIMIZE controls the application of _OPTIMIZE_FLAGS; MOZ_DEBUG controls _DEBUG_FLAGS. In the .mozconfig files I use, *neither* value is set.

The flag to control debug symbols in other modules is MOZ_DEBUG_SYMBOLS
    http://mxr.mozilla.org/mozilla-central/source/config/config.mk#253

My goal is to set the -Zi in the case that  
   1) MOZ_OPTIMIZE is not set,
   2) MOZ_DEBUG is not set,
   3) MOZ_DEBUG_SYMBOLS is set,
for nsprpub directories

It looks to me as if I should change two places in src/nsprpub/configure.in. 
1) In the "Override of system specific target options", case
*-mingw*|*-cygwin*|*-msvc*|*-mks*)
...
        # want symbols but not debug tracing or optimize
        if test -n "$MOZ_DEBUG_SYMBOLS"; then  
            if test -z "$MOZ_OPTIMIZE" -a -z "$MOZ_DEBUG"; then
                _DEBUG_SYMBOLS = "$_DEBUG_SYMBOLS -Zi"
            fi
        fi
...
And then near the bottom of the file, use the new _DEBUG_SYMBOLS:

if test -z "$MOZ_DEBUG" -a -z "$MOZ_OPTIMIZE"; then 
    if test -n "$_DEBUG_SYMBOLS"; then
        CFLAGS="$CFLAGS $_DEBUG_SYMBOLS"
        CXXFLAGS="$CXXFLAGS $_DEBUG_SYMBOLS"
    fi
fi      

But I can't get configure to work, the second bit fails with
/g/mozilla/mozilla-central/src/nsprpub/configure: _DEBUG_SYMBOLS: command not fo
und

Comment 30

[John J. Barton]

Attachment: Patch to nsprpub/config/rules.mk

only tested with the .mozconfig in comment 18 and only on windows

Comment 31

[John J. Barton]

Ok I've found a solution that works.  In nsprpub/config/rules.mk we can postpend the flags to create the .pdb files with the correct names:
OS_CFLAGS:=$(OS_CFLAGS) -Zi -Fd$(SHARED_LIB_PDB)

This can be done in the 'if' block that applies to Windows and OS/2 only, just after the value of SHARED_LIB_PDB is created.

The reset of OS_CFLAGS can be done only within if MOZ_DEBUG_SYMBOLS, however no other use of similar variable appears in rules.mk.

If any code after this point changes components of OS_CFLAGS, you lose.

Comment 32

[timeless]

Attachment: fix --enable-debugger-info-modules

nelson: your view is wrong.

gecko supports:
--enable-debugging-info-modules --disable-optimize --disable-debug

PDBs let you debug things. this includes optimized things and non optimized things.

NSPR (and sadly NSS which is why i keep trying to fix NSPR) have this broken view of the world which is basically described by your incorrect comments.

i should be able to debug *anything*.

in mozilla, --enable-debug means two things:

--enable-debugging-info
--enable-debugging-code

where info is stuff the compiler/linker provide so that a debugger will work
and code is arbitrary stuff hidden behind #ifdef DEBUG and the like.

wtc: please review this. i've had review requests to you for nearly 2 years on this bug. that's entirely unacceptable.

Comment 33

[timeless]

Comment on attachment 354732 [details] [diff] [review]
fix --enable-debugger-info-modules

ted: if this doesn't get a review in a reasonable time. i'd like permission to commit this to Mercurial *ANYWAY* and just merge it each time nspr lands.

Comment 34

[(not currently active) Ted Mielczarek]

Comment on attachment 354732 [details] [diff] [review]
fix --enable-debugger-info-modules

We don't take local patches to NSPR in m-c. We'll have to ping wtc about these, he's been pretty responsive as of late, these might just have fallen off his radar.

Comment 35

[John J. Barton]

Jan Odvarko just succeeded with the patch in comment 30 and 
export MOZ_DEBUG_SYMBOLS=1

Comment 36

[Wan-Teh Chang]

Comment on attachment 354732 [details] [diff] [review]
fix --enable-debugger-info-modules

Ted, could you review this patch?

Some background: NSPR developers only do two kinds of build:
- debug build (the default): --enable-debug --disable-optimize
- optimized build: --disable-debug --enable-optimize

We don't use other combinations of these two configure options,
and we don't use the MOZ_PROFILE and MOZ_DEBUG_SYMBOLS
environment variables.  The support of those two environment
variables was added to NSPR at Mozilla developers' request,
and over time I've forgot what they meant, and I couldn't
figure out by inspecting Mozilla's build system how Mozilla
uses them.

So, if the patch doesn't break how the NSPR developers build
NSPR, it's fine.  It'd be nice to add some comments to document
the meaning of MOZ_PROFILE and MOZ_DEBUG_SYMBOLS and how they
should be set (as environment variables, at configure time, or
at build time).  Thanks.

Comment 37

[Nelson Bolyard (seldom reads bugmail)]

Actually, I build with MOZ_DEBUG_SYMBOLS all the time.  But I can be 
flexible there, if needs be.

Comment 38

[Wan-Teh Chang]

Nelson, you're relying on an unintended effect of MOZ_DEBUG_SYMBOLS
to turn off the -PDB:NONE linker flag, which is not supported by
the latest versions of Visual C++.  The proper fix is to check the
Visual C++ version before adding the -PDB:NONE linker flag.  I
implemented the fix in nsprpub/configure.in but not in
security/coreconf:
http://mxr.mozilla.org/nspr/source/nsprpub/configure.in#1591

1591         if test "$MSC_VER" -le "1200" -a -z "$MOZ_DEBUG_SYMBOLS"; then
1592             OS_DLLFLAGS="$OS_DLLFLAGS -PDB:NONE"
1593         fi

So you don't need to set MOZ_DEBUG_SYMBOLS when building NSPR.

Contrast that with the corresponding code in security/coreconf:

http://mxr.mozilla.org/security/source/security/coreconf/WIN954.0.mk#70

70 ifndef MOZ_DEBUG_SYMBOLS
71         OS_DLLFLAGS += -PDB:NONE
72 endif

http://mxr.mozilla.org/security/source/security/coreconf/WIN32.mk#162

162 ifndef MOZ_DEBUG_SYMBOLS
163         LDFLAGS    += -PDB:NONE 
164 endif

Comment 39

[(not currently active) Ted Mielczarek]

(In reply to comment #37)
> Actually, I build with MOZ_DEBUG_SYMBOLS all the time.  But I can be 
> flexible there, if needs be.

As do all of our nightly builds. I can't be flexible about that. :) However, I did remove MOZ_PROFILE from the Mozilla build system, as it was duplicate functionality.

Comment 40

[[e-mail] (:bhearsum)]

Comment on attachment 354732 [details] [diff] [review]
fix --enable-debugger-info-modules

fwiw, this fixes things for me.

Comment 41

[Wan-Teh Chang]

Ted: MXR shows that MOZ_PROFILE still exists in mozilla-central.
When you say you removed MOZ_PROFILE from the Mozilla build system,
did you mean that you stopped using MOZ_PROFILE, rather than
removing it from the source tree?

Should we remove MOZ_PROFILE from the NSPR and NSS source trees?
The fewer (poorly documented) build variables, the better.

Comment 42

[(not currently active) Ted Mielczarek]

I only see two actual references to it:
http://mxr.mozilla.org/mozilla-central/source/configure.in#7958
http://mxr.mozilla.org/mozilla-central/source/Makefile.in#137

These are just leftovers that were missed, I suspect. We might as well clean it out of NSPR/NSS as well, if it was only added for Mozilla use anyway.

Comment 43

[Wan-Teh Chang]

Yes, those are the two references to MOZ_PROFILE I found. There is
one more reference at
http://mxr.mozilla.org/mozilla-central/source/config/autoconf.mk.in#590

MOZ_PROFILE is also mentioned in this comment:
http://mxr.mozilla.org/mozilla-central/source/config/config.mk#246

MOZ_PROFILE and MOZ_DEBUG_SYMBOLS were only added to NSPR/NSS for Mozilla
use.  Nelson has since found a side effect of MOZ_DEBUG_SYMBOLS to
turn off the obsolete -PDB:NONE linker flag, but we're not using
MOZ_PROFILE.

Comment 44

[(not currently active) Ted Mielczarek]

I filed bug 479978 on removing the last traces of MOZ_PROFILE from the Mozilla build system.

Comment 45

[(not currently active) Ted Mielczarek]

Comment on attachment 354732 [details] [diff] [review]
fix --enable-debugger-info-modules

+    if test -n "$NSPR_DEBUG_INFO"; then
+        if test -z "$MOZ_DEBUG_SYMBOLS"; then
+            export MOZ_DEBUG_SYMBOLS=1

The problem with this is that this will make --enable-debugger-info-modules=nspr work on Win32, but nowhere else. That doesn't seem great to me. You can already accomplish this (on Windows) by exporting MOZ_DEBUG_SYMBOLS=1.

The other part seems fine, since it's the actual fix to the bug here.

I wonder if we shouldn't just make nspr's configure understand --enable-debugger-info-modules instead? (on all platforms)

Comment 46

[Wan-Teh Chang]

Ted: NSPR doesn't have submodules.  Does the --enable-debugger-info-modules
option still make sense for NSPR?

Comment 47

[(not currently active) Ted Mielczarek]

I think we could just make it turn on debug symbols, like MOZ_DEBUG_SYMBOLS does. Except MOZ_DEBUG_SYMBOLS only works for Windows.

Comment 48

[Wan-Teh Chang]

OK, for NSPR I think the option should be named
--enable-debugger-info because NSPR consists of only
one module (itself).

--enable-debug implies --enable-debugger-info but also
turns on code ifdef'd with DEBUG.

Do I understand this correctly?

Comment 49

[(not currently active) Ted Mielczarek]

Yes. It feels like we should combine the codepaths for MOZ_DEBUG_SYMBOLS, too, since they accomplish that same task.

The only reason I suggested using --enable-debugger-info-modules is so that it would silently pass from the top-level configure and work, but it's not a big deal either way.

Comment 51

[Sergey Pastukhov]

Attachment: fix build rules for creating pdb files when needed

Comment 52

[(not currently active) Ted Mielczarek]

Comment on attachment 418861 [details] [diff] [review]
fix build rules for creating pdb files when needed

I don't think this is correct. If you compile with this patch and --enable-optimize and MOZ_DEBUG_SYMBOLS=1, you'll wind up with -DEBUG twice on the link line, because of this:
http://mxr.mozilla.org/mozilla-central/source/nsprpub/configure.in#1565

I think the configure part of timeless' patch above is actually correct.

Comment 53

[(not currently active) Ted Mielczarek]

bug 517097 is adding --enable-debug-symbols to the top-level configure, we should just port that to NSPR, refactor all these codepaths, and fix it right.

Comment 54

[Mitchell Field [:Mitch]]

Attachment: --enable-debug-symbols

Port of bug 517097's patch. This patch depends on the aforementioned patch.

Comment 55

[Mitchell Field [:Mitch]]

Comment on attachment 426395 [details] [diff] [review]
--enable-debug-symbols

This is broken.

Comment 56

[Mitchell Field [:Mitch]]

Attachment: --enable-debug-symbols (v2)

Comment 57

[Julian Seward [:jseward]]

(In reply to comment #32)

> i should be able to debug *anything*.
> 
> in mozilla, --enable-debug means two things:
> 
> --enable-debugging-info
> --enable-debugging-code
> 
> where info is stuff the compiler/linker provide so that a debugger will work
> and code is arbitrary stuff hidden behind #ifdef DEBUG and the like.

I absolutely agree.  Enabling debugging symbols etc (so that debuggers
can navigate) and enabling assertions/#ifdef-DEBUG code should be
independent.  Glomming them together is a mistake.

Comment 58

[Julian Seward [:jseward]]

(In reply to comment #56)
> Created an attachment (id=432001) [details]
> --enable-debug-symbols (v2)

Does not work for me, WinXP, VC2008 Express, mozilla-central.

Comment 59

[Robert Sayre]

(In reply to comment #57)
> (In reply to comment #32)
> 
> > i should be able to debug *anything*.
> > 
> > in mozilla, --enable-debug means two things:
> > 
> > --enable-debugging-info
> > --enable-debugging-code
> > 
> > where info is stuff the compiler/linker provide so that a debugger will work
> > and code is arbitrary stuff hidden behind #ifdef DEBUG and the like.
> 
> I absolutely agree.  Enabling debugging symbols etc (so that debuggers
> can navigate) and enabling assertions/#ifdef-DEBUG code should be
> independent.  Glomming them together is a mistake.

Doesn't --enable-debugger-info-modules already provide symbols without DEBUG code?

Comment 60

[(not currently active) Ted Mielczarek]

NSPR doesn't actually support that (and this is an NSPR bug). We replaced that with --enable-debug-symbols in the Mozilla configure in bug 517097, Mitch's patch here adds it to NSPR's configure as well. (but apparently doesn't fix the current bug here.)

Comment 61

[Mitchell Field [:Mitch]]

Attachment: --enable-debug-symbols (v3)

Comment 62

[Mitchell Field [:Mitch]]

Attachment: --enable-debug-symbols (v4)

Comment 63

[Mitchell Field [:Mitch]]

(In reply to comment #62)
> Created an attachment (id=435592) [details]
> --enable-debug-symbols (v4)

Tested on Windows 7, with all -debug, -debug-symbols and -optimize enable/disable combinations.

Comment 64

[(not currently active) Ted Mielczarek]

Comment on attachment 435592 [details] [diff] [review]
--enable-debug-symbols (v4)

>diff --git a/nsprpub/config/autoconf.mk.in b/nsprpub/config/autoconf.mk.in
>--- a/nsprpub/config/autoconf.mk.in
>+++ b/nsprpub/config/autoconf.mk.in
>@@ -32,16 +32,18 @@ MOD_MAJOR_VERSION = @MOD_MAJOR_VERSION@
> MOD_MINOR_VERSION = @MOD_MINOR_VERSION@
> MOD_PATCH_VERSION = @MOD_PATCH_VERSION@
> 
> LIBNSPR		= @LIBNSPR@
> LIBPLC		= @LIBPLC@
> 
> CROSS_COMPILE	= @CROSS_COMPILE@
> BUILD_OPT	= @MOZ_OPTIMIZE@
>+MOZ_DEBUG	= @MOZ_DEBUG@
>+MOZ_DEBUG_SYMBOLS	= @MOZ_DEBUG_SYMBOLS@

Do these line up in a sensible way in the actual file? Hard to tell with actual tab characters in a diff.

>diff --git a/nsprpub/configure.in b/nsprpub/configure.in
>--- a/nsprpub/configure.in
>+++ b/nsprpub/configure.in
>@@ -176,39 +176,58 @@ AC_ARG_WITH(mozilla,
> 	    else
> 	        MOZILLA_CLIENT=
> 	    fi],
>     [	if test -n "$MOZILLA_CLIENT"; then
> 	        AC_DEFINE(MOZILLA_CLIENT)
> 	    fi])
> 
> AC_ARG_ENABLE(optimize,
>-    [  --enable-optimize(=val) Enable code optimizations (val, ie. -O2) ],
>+    [  --enable-optimize[=OPT] Enable code optimizations (ie. -O2) ],
>     [ if test "$enableval" != "no"; then
>-        MOZ_OPTIMIZE=1
>-        if test -n "$enableval" && test "$enableval" != "yes"; then
>-    	    _OPTIMIZE_FLAGS=`echo $enableval | sed -e 's|\\\ | |g'`
>+          MOZ_OPTIMIZE=1
>+          if test -n "$enableval" -a "$enableval" != "yes"; then
>+            _OPTIMIZE_FLAGS=`echo $enableval | sed -e 's|\\\ | |g'`
>             _SAVE_OPTIMIZE_FLAGS=$_OPTIMIZE_FLAGS
>-        fi
>+          fi

Is this all just reindentation? Is this for consistency with something else? I'm not a fan of indentation changes unless you're already touching the code for another reason.

>+if test -n "$MOZ_DEBUG" -o -n "$MOZ_DEBUG_SYMBOLS"; then
>+    MOZ_DEBUG_SYMBOLS=1
>+fi

Why not just set MOZ_DEBUG_SYMBOLS=1 in the ENABLE(debug block where you set MOZ_DEBUG=1? Also, is there a reason you check that MOZ_DEBUG_SYMBOLS is set and then set it to 1 here?

>-        if test -n "$MOZ_OPTIMIZE"; then
>-            if test -n "$MOZ_DEBUG_SYMBOLS"; then
>-                _OPTIMIZE_FLAGS="$_OPTIMIZE_FLAGS -Zi"
>+        if test -n "$MOZ_DEBUG_SYMBOLS" -o -n "$MOZ_DEBUG"; then

Since you're setting MOZ_DEBUG_SYMBOLS if MOZ_DEBUG is defined up above, I think you should only have to test MOZ_DEBUG_SYMBOLS here. (And in the other places below.)

r=me with those things fixed. Thanks for the patch!

Comment 65

[Mitchell Field [:Mitch]]

Attachment: --enable-debug-symbols (v5)

(In reply to comment #64)
> (From update of attachment 435592 [details] [diff] [review])
> Do these line up in a sensible way in the actual file? Hard to tell with actual
> tab characters in a diff.
The whole file is a mess. I've tried to line them up better now.

> Is this all just reindentation? Is this for consistency with something else?
> I'm not a fan of indentation changes unless you're already touching the code
> for another reason.
I reindented and inverted the logic, to be consistent with the --enable-debug and --enable-debug-symbols options nearby.

> Why not just set MOZ_DEBUG_SYMBOLS=1 in the ENABLE(debug block where you set
> MOZ_DEBUG=1? Also, is there a reason you check that MOZ_DEBUG_SYMBOLS is set
> and then set it to 1 here?
Fixed.

> Since you're setting MOZ_DEBUG_SYMBOLS if MOZ_DEBUG is defined up above, I
> think you should only have to test MOZ_DEBUG_SYMBOLS here. (And in the other
> places below.)
Done.

Comment 66

[(not currently active) Ted Mielczarek]

Comment on attachment 435933 [details] [diff] [review]
--enable-debug-symbols (v5)

Ok, this looks good. Thanks! I'll land it for you shortly.

Comment 67

[(not currently active) Ted Mielczarek]

Landed in CVS:
Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.272; previous revision: 1.271
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.276; previous revision: 1.275
done
Checking in config/autoconf.mk.in;
/cvsroot/mozilla/nsprpub/config/autoconf.mk.in,v  <--  autoconf.mk.in
new revision: 1.39; previous revision: 1.38
done
Checking in config/rules.mk;
/cvsroot/mozilla/nsprpub/config/rules.mk,v  <--  rules.mk
new revision: 3.77; previous revision: 3.76
done

Comment 68

[John J. Barton]

Today for the first time in more than a month I did hg pull on 1.9.2 and updated. Then I set my .mozconfig to avoid 'debug':

export MOZ_DEBUG_SYMBOLS=1
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-ff-dbg
ac_add_options --enable-application=browser
mk_add_options MOZ_CO_PROJECT=browser
ac_add_options --disable-optimize
ac_add_options --disable-debug
ac_add_options --disable-tests
ac_add_options --enable-debugger-info-modules=yes

 My build fails again with " *** No rule to make target `nspr4.pdb', needed by `export'."

Is the patch here on 1.9.2?  If not will it work on 1.9.2? If yes, how to activate it?

Comment 69

[John J. Barton]

(In reply to comment #68)

> Is the patch here on 1.9.2?  If not will it work on 1.9.2? If yes, how to
> activate it?

I applied the patch to 1.9.2 but two hunks fail. I could not see how to fix them, I could not find the lines specified by the diff. If I run with the partial patch I still fail.

Comment 70

[John J. Barton]

My old workaround in comment 30 still works for me. So hopefully this problem will be fixed on 1.9.3.

Comment 71

[(not currently active) Ted Mielczarek]

This has only landed in NSPR CVS (and mozilla-central) so far. It's possible that we'll pick up a NSPR update on 1.9.2 at some point, but this isn't high-priority enough to drive an update there.

Comment 72

[Wan-Teh Chang]

Mitchell, Ted: thank you for the patch.  I'd like you to make
one improvement to your patch: --enable-debug-symbols should
have a "smart" default value for backward compatibility.

Recall that the NSPR and NSS developers build NSPR with only
these two combinations of the debug/optimize configure options:

    --enable-debug --disable-optimize (called the "debug" build)
    --disable-debug --enable-optimize (called the "optimized" build)

I'd like --enable-debug-symbols to be the default in debug builds,
and --disable-debug-symbols to be the default in optimized builds.

You can tie the default of the debug-symbols configure option
to either the debug or the optimize configure option.

Looking at your patch v5 (attachment 435933 [details] [diff] [review]), I think you intended
to tie the default of the debug-symbols option to the debug configure
option:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/configure.in&rev=1.277&mark=261,263-264#261

For some reason, it's not working well.  If I do a debug build
on Linux (just run "nsprpub/configure" without any options), I
don't get any debug symbols.  In fact, none of BUILD_OPT,
MOZ_DEBUG, and MOZ_DEBUG_SYMBOLS is defined in the generated
nsprpub/config/autoconf.mk:

    CROSS_COMPILE           =
    BUILD_OPT                       =
    MOZ_DEBUG                       =
    MOZ_DEBUG_SYMBOLS       =

Perhaps you didn't know that debug build is the default for
NSPR's configure?

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/configure.in&rev=1.277&mark=67-68#65

Comment 73

[Mitchell Field [:Mitch]]

Attachment: Followup

> I'd like --enable-debug-symbols to be the default in debug builds,
> and --disable-debug-symbols to be the default in optimized builds.

That's what I was aiming for.
 
> Looking at your patch v5 (attachment 435933 [details] [diff] [review]), I think you intended
> to tie the default of the debug-symbols option to the debug configure
> option:

Correct.

> Perhaps you didn't know that debug build is the default for
> NSPR's configure?

I did know, but I simply misunderstood AC_ARG_ENABLE'S behaviour when an option is not specified.

This patch sets the defaults as I originally intended, and also renames the few BUILD_OPT instances to MOZ_OPTIMIZE, as BUILD_OPT was set to @MOZ_OPTIMIZE@ anyway.

Comment 74

[(not currently active) Ted Mielczarek]

Comment on attachment 441688 [details] [diff] [review]
Followup

Sorry, I guess I missed those on reviewing the first patch.

Comment 75

[Wan-Teh Chang]

Attachment: Followup by Mitchell Field (as checked in)

I edited Mitchell's Followup patch a bit:
- Rely on the default value of MOZ_DEBUG set at the top of configure.in.
- Fix TABs; in NSPR the default tab stop is 8.

I checked in the patch on the NSPR trunk (NSPR 4.8.6).

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.282; previous revision: 1.281
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.286; previous revision: 1.285
done
Checking in config/autoconf.mk.in;
/cvsroot/mozilla/nsprpub/config/autoconf.mk.in,v  <--  autoconf.mk.in
new revision: 1.42; previous revision: 1.41
done
Checking in config/config.mk;
/cvsroot/mozilla/nsprpub/config/config.mk,v  <--  config.mk
new revision: 3.33; previous revision: 3.32
done
Checking in pr/src/md/windows/Makefile.in;
/cvsroot/mozilla/nsprpub/pr/src/md/windows/Makefile.in,v  <--  Makefile.in
new revision: 1.21; previous revision: 1.20
done

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.283; previous revision: 1.282
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.287; previous revision: 1.286
done

Comment 77

[Alex Vincent [:WeirdAl]]

(In reply to comment #71)
> This has only landed in NSPR CVS (and mozilla-central) so far. It's possible
> that we'll pick up a NSPR update on 1.9.2 at some point, but this isn't
> high-priority enough to drive an update there.

Requesting 1.9.2 support on the basis of duplicate bug 590032, which bit me unexpectedly.  (I really, really hate it when I can't build in some variant of configure options.)

Comment 78

[Benjamin Smedberg]

No, this has too many odd dependencies to backport to 1.9.2