[PATCH v3] Separate runtime debug output from debug symbols

[PATCH v3] Separate runtime debug output from debug symbols

[Olaf Hering]

Two make variables exist (debug=y and debug_symbols=y) to control either
the creation of additional runtime debug or the creation of debug
symbols for tools like gdb.  Some places in the code still passed -g
unconditionally to the compiler. Wrap them into a "debug_symbols"
conditional. The xen build passed -g unconditional, reuse the existing
debug_symbols=y check in Config.mk. There are still parts of the code
that hardcode -g, namely the external qemu-traditional and stubdom
packages. They are not changed by this patch.

No change in behaviour is expected by that patch for tools and xen.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com> (for xen/Rules.mk)
Cc: David Scott <dave.scott@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Samuel Thibault <samuel.thibault@ens-lyon.org>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/Makefile          | 7 +++++++
 tools/ocaml/common.make | 5 ++++-
 xen/Rules.mk            | 2 +-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/Makefile b/tools/Makefile
index 543cd29..884dc91 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -197,6 +197,12 @@ else
 QEMU_XEN_ENABLE_DEBUG :=
 endif
 
+ifeq ($(debug_symbols),y)
+QEMU_XEN_ENABLE_DEBUG_SYMBOLS := --enable-debug-info --disable-strip
+else
+QEMU_XEN_ENABLE_DEBUG_SYMBOLS := --disable-debug-info
+endif
+
 subdir-all-qemu-xen-dir: qemu-xen-dir-find
 	if test -d $(QEMU_UPSTREAM_LOC) ; then \
 		source=$(QEMU_UPSTREAM_LOC); \
@@ -206,6 +212,7 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
 	cd qemu-xen-dir; \
 	$$source/configure --enable-xen --target-list=i386-softmmu \
 		$(QEMU_XEN_ENABLE_DEBUG) \
+		$(QEMU_XEN_ENABLE_DEBUG_SYMBOLS) \
 		--prefix=$(LIBEXEC) \
 		--libdir=$(LIBEXEC_LIB) \
 		--includedir=$(LIBEXEC_INC) \
diff --git a/tools/ocaml/common.make b/tools/ocaml/common.make
index d5478f6..33b3142 100644
--- a/tools/ocaml/common.make
+++ b/tools/ocaml/common.make
@@ -13,7 +13,10 @@ CFLAGS += -fPIC -Werror -I$(shell ocamlc -where)
 
 OCAMLOPTFLAG_G := $(shell $(OCAMLOPT) -h 2>&1 | sed -n 's/^  *\(-g\) .*/\1/p')
 OCAMLOPTFLAGS = $(OCAMLOPTFLAG_G) -ccopt "$(LDFLAGS)" -dtypes $(OCAMLINCLUDE) -cc $(CC) -w F -warn-error F
-OCAMLCFLAGS += -g $(OCAMLINCLUDE) -w F -warn-error F
+ifeq ($(debug_symbols),y)
+OCAMLCFLAGS += -g
+endif
+OCAMLCFLAGS += $(OCAMLINCLUDE) -w F -warn-error F
 
 VERSION := 4.1
 
diff --git a/xen/Rules.mk b/xen/Rules.mk
index a97405c..60cd173 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -43,7 +43,7 @@ ALL_OBJS-$(x86)          += $(BASEDIR)/crypto/built_in.o
 
 CFLAGS += -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
-CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
+CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
 CFLAGS += -nostdinc
 
 CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE

Re: [PATCH v3] Separate runtime debug output from debug symbols

[Ian Jackson]

Olaf Hering writes ("[PATCH v3] Separate runtime debug output from debug symbols"):
> Two make variables exist (debug=y and debug_symbols=y) to control either
> the creation of additional runtime debug or the creation of debug
> symbols for tools like gdb.  Some places in the code still passed -g
> unconditionally to the compiler. Wrap them into a "debug_symbols"
> conditional. The xen build passed -g unconditional, reuse the existing
> debug_symbols=y check in Config.mk. There are still parts of the code
> that hardcode -g, namely the external qemu-traditional and stubdom
> packages. They are not changed by this patch.

Why is it wrong to always pass -g ?

(There should indeed be separate control over whether installed code
is symbol-stripped.)

Ian.

Re: [PATCH v3] Separate runtime debug output from debug symbols

[Olaf Hering]

On Wed, Oct 08, Ian Jackson wrote:

> Why is it wrong to always pass -g ?

It consumes extra disk space and build time.

Olaf

Re: [PATCH v3] Separate runtime debug output from debug symbols

[Ian Jackson]

Olaf Hering writes ("Re: [PATCH v3] Separate runtime debug output from debug symbols"):
> On Wed, Oct 08, Ian Jackson wrote:
> > Why is it wrong to always pass -g ?
> 
> It consumes extra disk space and build time.

On the other hand it means that if you get a crash you can debug it.
Lots of projects turn on -g all the time.

Ian.

Re: [PATCH v3] Separate runtime debug output from debug symbols

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH v3] Separate runtime debug output from debug symbols"):
> Olaf Hering writes ("Re: [PATCH v3] Separate runtime debug output from debug symbols"):
> > On Wed, Oct 08, Ian Jackson wrote:
> > > Why is it wrong to always pass -g ?
> > 
> > It consumes extra disk space and build time.
> 
> On the other hand it means that if you get a crash you can debug it.
> Lots of projects turn on -g all the time.

For the avoidance of doubt: I don't object to a patch to make -g
controllable independently by the user building Xen.

I do object to a change to the default, which AFAICT is what happens
in your patch.

I also don't think this is a bugfix so it doesn't qualify for 4.5.

Ian.

Re: [PATCH v3] Separate runtime debug output from debug symbols

[Olaf Hering]

On Wed, Oct 08, Ian Jackson wrote:

> I do object to a change to the default, which AFAICT is what happens
> in your patch.

How does it change the default? Confik.mk has:

 19 # A debug build of Xen and tools?
 20 debug ?= y
 21 debug_symbols ?= $(debug)

Which means its always on.

Olaf

Re: [PATCH v3] Separate runtime debug output from debug symbols

[Ian Jackson]

Olaf Hering writes ("Re: [PATCH v3] Separate runtime debug output from debug symbols"):
> On Wed, Oct 08, Ian Jackson wrote:
> > I do object to a change to the default, which AFAICT is what happens
> > in your patch.
> 
> How does it change the default? Confik.mk has:

Sorry, I wasn't clear.  I mean that -g should remain the default even
in debug=n builds of Xen.

>  19 # A debug build of Xen and tools?
>  20 debug ?= y
>  21 debug_symbols ?= $(debug)
> 
> Which means its always on.

If debug=n, -g is turned off.  The user might do that.  Also we
routinely set debug=n during the last stages of the release process.

Ian.

Re: [PATCH v3] Separate runtime debug output from debug symbols

[Olaf Hering]

On Wed, Oct 08, Ian Jackson wrote:

> Olaf Hering writes ("Re: [PATCH v3] Separate runtime debug output from debug symbols"):
> > On Wed, Oct 08, Ian Jackson wrote:
> > > I do object to a change to the default, which AFAICT is what happens
> > > in your patch.
> > 
> > How does it change the default? Confik.mk has:
> 
> Sorry, I wasn't clear.  I mean that -g should remain the default even
> in debug=n builds of Xen.


> >  19 # A debug build of Xen and tools?
> >  20 debug ?= y
> >  21 debug_symbols ?= $(debug)
> > 
> > Which means its always on.
> 
> If debug=n, -g is turned off.  The user might do that.  Also we
> routinely set debug=n during the last stages of the release process.

I'm not sure I follow. Do you mean, in addition to my change to move more
places that append -g to CFLAGS, another change on top of that should set
"debug_symbols ?= y" in Config.mk?

If thats what you mean, I'm fine with that. Should I include it in v4 of
my change?


Olaf

Re: [PATCH v3] Separate runtime debug output from debug symbols

[Olaf Hering]

On Wed, Oct 08, Ian Jackson wrote:

> Olaf Hering writes ("Re: [PATCH v3] Separate runtime debug output from debug symbols"):
> > On Wed, Oct 08, Ian Jackson wrote:
> > > I do object to a change to the default, which AFAICT is what happens
> > > in your patch.
> > 
> > How does it change the default? Confik.mk has:
> 
> Sorry, I wasn't clear.  I mean that -g should remain the default even
> in debug=n builds of Xen.
> 
> >  19 # A debug build of Xen and tools?
> >  20 debug ?= y
> >  21 debug_symbols ?= $(debug)
> > 
> > Which means its always on.
> 
> If debug=n, -g is turned off.  The user might do that.  Also we
> routinely set debug=n during the last stages of the release process.

So how do we proceed here?

v3 of the patch is fine. It removes the hardcoded -g from more places.
The result is more free diskspace if one requests to compile the source
without -g. The default remains the same. 
If one does configure --disable-debug he may have a reason to do so.
Still "make debug_symbols=y" will build the whole thing with -g.

What the default for both knobs should be and how the default is
supposed to be set is a separate discussion not belonging to this.


Olaf