MOZ_DEBUG_FLAGS are hardcoded

RESOLVED FIXED in mozilla1.8beta3

Status

()

Core
Build Config
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Martin Mokrejs, Unassigned)

Tracking

Trunk
mozilla1.8beta3
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050304
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050304

As explained in the bug https://bugzilla.mozilla.org/show_bug.cgi?id=232748, one
needs to specify both, --enable-optimize and --disable-debug to get fully
optimzied build. Alternatively, --disable-optimize and --enable-debug must be
used to yield binaries with full debug.

The patch changes the configure --help output on those 2 respective lines.

Reproducible: Always
(Reporter)

Comment 1

12 years ago
Created attachment 176282 [details] [diff] [review]
patch 2 lines in configure --help output

applies to current cvs
Attachment #176282 - Flags: review?(benjamin)
Comment on attachment 176282 [details] [diff] [review]
patch 2 lines in configure --help output

1) The configure output should wrap at 78 columns.

2) "must" is not the correct word. Use a simple imperative sentence instead:
"Use with --disable-optimize for full debugging."
Attachment #176282 - Flags: review?(benjamin) → review-
(Reporter)

Comment 3

12 years ago
Created attachment 178101 [details] [diff] [review]
patch 2 lines in configure --help output and disable default host optimization

This patch does slightly more then the previous one. It prevents configure
zapping debug options with default optimize values. Further, forcing "-g" on
some systems turns on minimal debug info, which effciently zaps -g3 is
specified previously. Simply, using "-g" is not safe (sometimes overrides -g3
etc).
Attachment #176282 - Attachment is obsolete: true
Attachment #178101 - Flags: review?(benjamin)
Comment on attachment 178101 [details] [diff] [review]
patch 2 lines in configure --help output and disable default host optimization

Chris, can you look at this? If not, can you punt it to dbaron?
Attachment #178101 - Flags: review?(benjamin) → review?(cls)

Comment 5

12 years ago
Comment on attachment 178101 [details] [diff] [review]
patch 2 lines in configure --help output and disable default host optimization

Why is this necessary?	Host optimizations are only used for programs that are
built & used as part of the build process (eg. mkdepend and when
cross-compiling, xpidl).  On some systems, the optimization makes a signficant
difference in the performance of these programs.  If you want to debug those
programs, you can set HOST_OPTIMIZE_FLAGS=-g. 

Most of this patch doesn't make sense to me.
+if test "${HOST_OPTIMIZE_FLAGS=}"; then
+    HOST_OPTIMIZE_FLAGS="${HOST_OPTIMIZE_FLAGS=}";
+fi

If HOST_OPTIMIZE_FLAGS is set then set it?

+    if test -z "${HOST_OPTIMIZE_FLAGS}"; then
     HOST_OPTIMIZE_FLAGS="${HOST_OPTIMIZE_FLAGS=-O2}"
+    fi

HOST_OPTIMIZE_FLAGS="${HOST_OPTIMIZE_FLAGS=-O2}" implicitly does the if emtpy
test.  It doesn't need to be wrapped.

Why are you disabling MOZ_OPTIMIZE_FLAGS if MOZ_DEBUG_FLAGS isn't empty?  It's
possible to use both flags by design.  We're not going to change that.
Attachment #178101 - Flags: review?(cls) → review-
(Reporter)

Comment 6

12 years ago
Per comment #5:
<quote>
HOST_OPTIMIZE_FLAGS="${HOST_OPTIMIZE_FLAGS=-O2}" implicitly does the if emtpy
test.  It doesn't need to be wrapped.
</quote>

I didn't know this, but you're right. This part of the patch is not necessary.


The patch is about improvement of configures output and that MOZ_DEBUG_FLAGS are
blindly set regardless any other options ... I have to get rid of this as it
efectively kills my HOST_OPTIMIZE_FLAGS. Imagine in HOST_OPTIMIZE_FLAGS I ask
for some compiler options, but MOZ_DEBUG_FLAGS enforces some own options.

+    if test ! "${MOZ_DEBUG_FLAGS}"; then
     MOZ_DEBUG_FLAGS="-g -fno-inline"  # most people on linux use gcc/gdb,
                                       # and that combo is not yet good at
                                       # debugging inlined functions (even
                                       # when using DWARF2 as the debugging
                                       # format)    
+    fi

Comment 7

12 years ago
You are correct that the MOZ_DEBUG_FLAGS are hardcoded.  Your patch should
simply modify those settings to use the shell empty test construct that
HOST_OPTIMIZE_FLAGS uses.  I don't understand why you hardcoded the use of -ggdb
in your patch either.

I still think the configure output changes are unnecessary given our discussion
in bug 232748.
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: build-config
Summary: PATCH: make configure --help descriptions clear → MOZ_DEBUG_FLAGS are hardcoded

Comment 8

12 years ago
*** Bug 268292 has been marked as a duplicate of this bug. ***

Comment 9

12 years ago
Created attachment 179906 [details] [diff] [review]
Allow --enable-debug to accept compiler flags like --enable-optimize

This patch allows --enable-debug to take compiler flags in the same fashion
that --enable-optimize does.  These flags will override the default
MOZ_DEBUG_FLAGS/_DEBUG_FLAGS settings.

Updated

12 years ago
Assignee: chase → cls
Attachment #178101 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #179906 - Flags: review?(benjamin)
Attachment #179906 - Flags: review?(benjamin) → review+

Comment 10

12 years ago
Let's also take this opportunity to standardize
the description of the --enable-debug flag in the
three configure scripts.

In NSPR, --enable-debug means:
1. Produce debugging information (for use by debuggers).
This is where the compiler flags like -g or /Zi come in.
2. Conditionally compile debugging code by defining
the DEBUG and closely related macros.

These two things are actually orthogonal, but they
are both controlled by the --enable-debug configure
option.

I hope we can come up with a concise definition of
--enable-debug that conveys both of these points.

Comment 11

12 years ago
I found that Mozilla's toplevel configure.in script
(mozilla/configure.in) distinguishes the two meanings
of "enable debugging" with the --enable-debugger-info-modules
and --enable-debug-modules options.

This means the description of --enable-debug probably
should not be the same for the three configure scripts.
The description in mozilla/configure.in should cover
its relations with --enable-debugger-info-modules
and --enable-debug-modules.

Comment 12

12 years ago
The current description of the --enable-debug-modules is wrong:

4751 MOZ_ARG_ENABLE_STRING(debug-modules,
4752 [  --enable-debug-modules  Enable/disable debug info for specific modules],
4753 [ MOZ_DEBUG_MODULES=`echo $enableval| sed 's/,/ /g'` ] )

"debug info" should be changed to "debug code" or
"debugging code" because this option controls -DDEBUG,
not -g.

Comment 13

12 years ago
The definition of --enable-debug is basically the same across all 3 configure
scripts:  enable compiler debugging + maintainer DEBUG macros.

The additional options in the toplevel configure script just give a finer
grained control over which modules use the debug flags.  These options were
added so that developers working on a small fairly self-contained piece of code
(like exthandlers) do not have to suffer through a full debug build (in
particular, the linking of a debug version of liblayout).  The use of these
additional options is not required for a debug build.

Updated

12 years ago
Attachment #179906 - Flags: approval1.8b3?
Comment on attachment 179906 [details] [diff] [review]
Allow --enable-debug to accept compiler flags like --enable-optimize

a=shaver
Attachment #179906 - Flags: approval1.8b3? → approval1.8b3+

Comment 15

12 years ago
The patch has been checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
OS: Linux → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta3
(Reporter)

Comment 16

12 years ago
Finally I had a time to re-check on this current cvs. No, it is not fixed at
least everywhere:

gmake[3]: Entering directory `/scratch/mozilla/dbm/src'
db.c
gcc -o db.o -c -I../../dist/include/system_wrappers -include
../../config/gcc_hidden.h -DOSTYPE=\"Linux2.6\" -DOSARCH=\"Linux\"
-DBUILD_ID=0000000000 -DMEMMOVE -D__DBINTERFACE_PRIVATE  -I./../include 
-I../../dist/include -I../../dist/include -I../../dist/include/nspr   
-I../../dist/sdk/include    -fPIC  -Wall -W -Wno-unused -Wpointer-arith
-Wcast-align -Wno-long-long -pedantic -g3 -O0 -ggdb -pthread -pipe  -DDEBUG
-D_DEBUG -DDEBUG_root -DTRACING -g -fno-inline   -include ../../mozilla-config.h
-DMOZILLA_CLIENT -Wp,-MD,.deps/db.pp db.c

Do you see the "-g -fno-inline" overrding the former "-g3 -O0 -ggdb" flags?
Please see comment #6.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 17

12 years ago
WFM. How are you trying to override the debug flags?  You'll have to use the
--enable-debug=<flags> option.
(Reporter)

Comment 18

12 years ago
I configure like this:

MOZ_DEBUG_FLAGS="-g3 -O0 -ggdb" CFLAGS="-g3 -O0 -ggdb" CXXFLAGS="-g3 -O0 -ggdb"
./configure --enable-debug --disable-optimize 
--enable-debug-modules=all --enable-debugger-info-modules
--enable-detect-webshell-leaks --enable-svg --enable-svg-renderer-libart
--enable-image-decoders=all --with-qtdir=/usr/qt/3 --enable-application=suite
--disable-freetype2 --enable-jprof --enable --default-toolkit=gtk2 --enable-xft

With overrding I meant that the latter values override the former.
So "-g3 -O0 -ggdb -g -fno-inline" gives "-O0 -ggdb -g2 -fno-inline"



       -glevel
       -ggdblevel
       -gstabslevel
       -gcofflevel
       -gxcofflevel
       -gvmslevel
           Request debugging information and also use level to specify how much
           information.  The default level is 2.

           Level 1 produces minimal information, enough for making backtraces
           in parts of the program that you don't plan to debug.  This includes
           descriptions of functions and external variables, but no information 
           about local variables and no line numbers.

           Level 3 includes extra information, such as all the macro definitions
     
           present in the program.  Some debuggers support macro expansion when
           you use -g3.


       -ggdb
           Produce debugging information for use by GDB.  This means to use the
       
           most expressive format available (DWARF 2, stabs, or the native
           format if neither of those are supported), including GDB extensions
           if at all possible.


I think if "-g" is already set in CFLAGS/CXXFLAGS but if you don't aggree then
consider at least when set in MOZ_DEBUG_FLAGS (as was in my case) ... then "-g
-no-inline" defining code should be cautious and append only "-no-inline".
Assignee: cls → nobody
Status: REOPENED → NEW

Comment 19

12 years ago
Sorry, but we don't suport setting MOZ_DEBUG_FLAGS via the environment.  We
don't support setting MOZ_OPTIMIZE_FLAGS via the environment either.  We
extended --enable-debug & --enable-optimize so that people wouldn't have to set
the environment variables that correspond to those options.  Is there some
reason why you keep avoiding those options?  

Btw, setting CFLAGS & CXXFLAGS should be completely unnecessary at this point.

(Reporter)

Comment 20

12 years ago
My apologies then. I thought I still have to use them to set "-ggdb" for example.
So what's the proposed method then? ;-)

Comment 21

12 years ago
./configure --disable-optimize --enable-debug='-g3 -O0 -ggdb'

(Reporter)

Comment 22

12 years ago
I see, you are right, that works. ;)

gmake[3]: Entering directory `/scratch/mozilla/nsprpub/config'
gcc -o now.o -c      -pipe -ansi -Wall -pthread -g3 -O0 -ggdb -fPIC  -UNDEBUG
-DDEBUG_root  -DMOZILLA_CLIENT=1 -DDEBUG=1 -DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE=1
-DHAVE_VISIBILITY_PRAGMA=1 -DXP_UNIX=1 -D_POSIX_SOURCE=1 -D_BSD_SOURCE=1
-D_SVID_SOURCE=1 -D_LARGEFILE64_SOURCE=1 -DHAVE_FCNTL_FILE_LOCKING=1 -DLINUX=1
-Di386=1 -DHAVE_LCHOWN=1 -DHAVE_STRERROR=1 -D_REENTRANT=1  -DFORCE_PR_LOG
-D_PR_PTHREADS -UHAVE_CVAR_BUILT_ON_SEM   now.c
Status: NEW → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.