Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
>>> On 10.09.18 at 19:51, wrote: > On 10/09/18 14:29, Jan Beulich wrote: > On 10.09.18 at 15:21, wrote: >> On 31.08.18 at 10:43, wrote: >>> On 31.08.18 at 10:29, wrote: > --- a/xen/Kconfig.debug > +++ b/xen/Kconfig.debug > @@ -11,6 +11,13 @@ config DEBUG > > You probably want to say 'N' here. > > +config DEBUG_INFO > + bool "Compile Xen with debug info" > + default y > + ---help--- > + If you say Y here the resulting Xen will include debugging info > + resulting in a larger binary image. > + > if DEBUG || EXPERT = "y" Perhaps better move your addition into this conditional section? >>> So this was a bad suggestion after all - with DEBUG=n DEBUG_INFO is >>> now implicitly n as well. The section needs to be moved back to where >>> you had it as per above, with the _prompt_ depending on >>> DEBUG || EXPERT="y". >> Furthermore - is COVERAGE without DEBUG_INFO of any use? > > Yes - very much so. > > From a "how much of my binary does do my tests cover" point of view, you > want the release binary rather than the debug binary. Did you confuse DEBUG and DEBUG_INFO? My question was because I suspect the analyzer to want/need to have debug information available. >> Are there >> perhaps any other dependencies (I think/hope live patching logic doesn't >> depend on debug info)? > > The livepatch build depends on xen-syms containing all the debug > information, but the runtime logic doesn't, I believe. But if the livepatch build requires debug information (rather than just the ELF symbol table), then it _is_ depending on DEBUG_INFO. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
On 10/09/18 14:29, Jan Beulich wrote: On 10.09.18 at 15:21, wrote: > On 31.08.18 at 10:43, wrote: >> On 31.08.18 at 10:29, wrote: --- a/xen/Kconfig.debug +++ b/xen/Kconfig.debug @@ -11,6 +11,13 @@ config DEBUG You probably want to say 'N' here. +config DEBUG_INFO + bool "Compile Xen with debug info" + default y + ---help--- +If you say Y here the resulting Xen will include debugging info +resulting in a larger binary image. + if DEBUG || EXPERT = "y" >>> Perhaps better move your addition into this conditional section? >> So this was a bad suggestion after all - with DEBUG=n DEBUG_INFO is >> now implicitly n as well. The section needs to be moved back to where >> you had it as per above, with the _prompt_ depending on >> DEBUG || EXPERT="y". > Furthermore - is COVERAGE without DEBUG_INFO of any use? Yes - very much so. From a "how much of my binary does do my tests cover" point of view, you want the release binary rather than the debug binary. In some copious free time, I'd like to automate the measurements of "how much of Xen does the XTF suite cover?" > Are there > perhaps any other dependencies (I think/hope live patching logic doesn't > depend on debug info)? The livepatch build depends on xen-syms containing all the debug information, but the runtime logic doesn't, I believe. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
>>> On 10.09.18 at 15:21, wrote: On 31.08.18 at 10:43, wrote: > On 31.08.18 at 10:29, wrote: >>> --- a/xen/Kconfig.debug >>> +++ b/xen/Kconfig.debug >>> @@ -11,6 +11,13 @@ config DEBUG >>> >>> You probably want to say 'N' here. >>> >>> +config DEBUG_INFO >>> + bool "Compile Xen with debug info" >>> + default y >>> + ---help--- >>> + If you say Y here the resulting Xen will include debugging info >>> + resulting in a larger binary image. >>> + >>> if DEBUG || EXPERT = "y" >> >> Perhaps better move your addition into this conditional section? > > So this was a bad suggestion after all - with DEBUG=n DEBUG_INFO is > now implicitly n as well. The section needs to be moved back to where > you had it as per above, with the _prompt_ depending on > DEBUG || EXPERT="y". Furthermore - is COVERAGE without DEBUG_INFO of any use? Are there perhaps any other dependencies (I think/hope live patching logic doesn't depend on debug info)? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
>>> On 31.08.18 at 10:43, wrote: On 31.08.18 at 10:29, wrote: >> --- a/xen/Kconfig.debug >> +++ b/xen/Kconfig.debug >> @@ -11,6 +11,13 @@ config DEBUG >> >>You probably want to say 'N' here. >> >> +config DEBUG_INFO >> +bool "Compile Xen with debug info" >> +default y >> +---help--- >> + If you say Y here the resulting Xen will include debugging info >> + resulting in a larger binary image. >> + >> if DEBUG || EXPERT = "y" > > Perhaps better move your addition into this conditional section? So this was a bad suggestion after all - with DEBUG=n DEBUG_INFO is now implicitly n as well. The section needs to be moved back to where you had it as per above, with the _prompt_ depending on DEBUG || EXPERT="y". Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
>>> On 31.08.18 at 11:50, wrote: > On 08/31/2018 10:25 AM, Jan Beulich wrote: > On 31.08.18 at 11:02, wrote: >>> On Fri, Aug 31, Jan Beulich wrote: >>> Perhaps better move your addition into this conditional section? >>> >>> Then -g would disappear when DEBUG is disabled. Is that intentional? >> >> It would still be available when EXPERT=y. > This feels a bit odd you suggest EXPERT=y here given you wrote earlier: > > "But note that without debug info the quality of certain linker errors > decreases, which is why I would recommend against doing so." > > If that can improve compiler or distro needs to create debug-symbols > package, then this should be selectable without EXPERT. Perhaps some misunderstanding? EXPERT=y would allow to turn _off_ that option; if would be _on_ by default. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
Hi Jan, On 08/31/2018 10:25 AM, Jan Beulich wrote: On 31.08.18 at 11:02, wrote: On Fri, Aug 31, Jan Beulich wrote: Perhaps better move your addition into this conditional section? Then -g would disappear when DEBUG is disabled. Is that intentional? It would still be available when EXPERT=y. This feels a bit odd you suggest EXPERT=y here given you wrote earlier: "But note that without debug info the quality of certain linker errors decreases, which is why I would recommend against doing so." If that can improve compiler or distro needs to create debug-symbols package, then this should be selectable without EXPERT. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
>>> On 31.08.18 at 11:02, wrote: > On Fri, Aug 31, Jan Beulich wrote: > >> Perhaps better move your addition into this conditional section? > > Then -g would disappear when DEBUG is disabled. Is that intentional? It would still be available when EXPERT=y. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
On Fri, Aug 31, Jan Beulich wrote: > Perhaps better move your addition into this conditional section? Then -g would disappear when DEBUG is disabled. Is that intentional? Olaf signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
On Fri, Aug 31, 2018 at 09:51:50AM +0100, Andrew Cooper wrote: > On 31/08/18 09:41, Jan Beulich wrote: > On 31.08.18 at 10:32, wrote: > >> On Fri, Aug 31, 2018 at 10:29:21AM +0200, Olaf Hering wrote: > >>> Creating debug info during build is not strictly required at runtime. > >>> Make it optional by introducing a new Kconfig knob "DEBUG_INFO". > >>> This slightly reduces build time and diskusage, if disabled. > >>> > >>> Signed-off-by: Olaf Hering > >>> --- > >>> xen/Kconfig.debug | 7 +++ > >>> xen/Rules.mk | 5 - > >>> 2 files changed, 11 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug > >>> index 380c4e8d75..1d8c2c94a9 100644 > >>> --- a/xen/Kconfig.debug > >>> +++ b/xen/Kconfig.debug > >>> @@ -11,6 +11,13 @@ config DEBUG > >>> > >>> You probably want to say 'N' here. > >>> > >>> +config DEBUG_INFO > >>> + bool "Compile Xen with debug info" > >>> + default y > >> At the very least this should be default DEBUG > > I disagree - -g is being passed to the compiler unconditionally at > > present. > > +1 > > Distro packagers need to build debuginfo packages, and having a release > build of Xen with xen-syms lacking all the debug symbols is quite useless. I see. I misunderstood the intent. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
On 31/08/18 09:41, Jan Beulich wrote: On 31.08.18 at 10:32, wrote: >> On Fri, Aug 31, 2018 at 10:29:21AM +0200, Olaf Hering wrote: >>> Creating debug info during build is not strictly required at runtime. >>> Make it optional by introducing a new Kconfig knob "DEBUG_INFO". >>> This slightly reduces build time and diskusage, if disabled. >>> >>> Signed-off-by: Olaf Hering >>> --- >>> xen/Kconfig.debug | 7 +++ >>> xen/Rules.mk | 5 - >>> 2 files changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug >>> index 380c4e8d75..1d8c2c94a9 100644 >>> --- a/xen/Kconfig.debug >>> +++ b/xen/Kconfig.debug >>> @@ -11,6 +11,13 @@ config DEBUG >>> >>> You probably want to say 'N' here. >>> >>> +config DEBUG_INFO >>> + bool "Compile Xen with debug info" >>> + default y >> At the very least this should be default DEBUG > I disagree - -g is being passed to the compiler unconditionally at > present. +1 Distro packagers need to build debuginfo packages, and having a release build of Xen with xen-syms lacking all the debug symbols is quite useless. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
>>> On 31.08.18 at 10:29, wrote: > --- a/xen/Kconfig.debug > +++ b/xen/Kconfig.debug > @@ -11,6 +11,13 @@ config DEBUG > > You probably want to say 'N' here. > > +config DEBUG_INFO > + bool "Compile Xen with debug info" > + default y > + ---help--- > + If you say Y here the resulting Xen will include debugging info > + resulting in a larger binary image. > + > if DEBUG || EXPERT = "y" Perhaps better move your addition into this conditional section? > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -55,7 +55,10 @@ endif > > CFLAGS += -nostdinc -fno-builtin -fno-common > CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith > -CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h > +ifeq ($(CONFIG_DEBUG_INFO),y) > +CFLAGS += -g Note how a few lines down from here we already use CFLAGS-y. Please make this CFLAGS-$(CONFIG_DEBUG_INFO) += -g Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
>>> On 31.08.18 at 10:32, wrote: > On Fri, Aug 31, 2018 at 10:29:21AM +0200, Olaf Hering wrote: >> Creating debug info during build is not strictly required at runtime. >> Make it optional by introducing a new Kconfig knob "DEBUG_INFO". >> This slightly reduces build time and diskusage, if disabled. >> >> Signed-off-by: Olaf Hering >> --- >> xen/Kconfig.debug | 7 +++ >> xen/Rules.mk | 5 - >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug >> index 380c4e8d75..1d8c2c94a9 100644 >> --- a/xen/Kconfig.debug >> +++ b/xen/Kconfig.debug >> @@ -11,6 +11,13 @@ config DEBUG >> >>You probably want to say 'N' here. >> >> +config DEBUG_INFO >> +bool "Compile Xen with debug info" >> +default y > > At the very least this should be default DEBUG I disagree - -g is being passed to the compiler unconditionally at present. > But what is the difference between this and DEBUG anyway? DEBUG_INFO controls use of the -g compiler option. DEBUG controls how e.g. ASSERT()s behave. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
On Fri, Aug 31, Wei Liu wrote: > But what is the difference between this and DEBUG anyway? DEBUG_INFO is for gcc (-g), DEBUG is for Kconfig. Adding a dependency to DEBUG may change behavior, not sure what the expected outcome of always using '-g' is. Olaf signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen: add DEBUG_INFO Kconfig symbol
On Fri, Aug 31, 2018 at 10:29:21AM +0200, Olaf Hering wrote: > Creating debug info during build is not strictly required at runtime. > Make it optional by introducing a new Kconfig knob "DEBUG_INFO". > This slightly reduces build time and diskusage, if disabled. > > Signed-off-by: Olaf Hering > --- > xen/Kconfig.debug | 7 +++ > xen/Rules.mk | 5 - > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug > index 380c4e8d75..1d8c2c94a9 100644 > --- a/xen/Kconfig.debug > +++ b/xen/Kconfig.debug > @@ -11,6 +11,13 @@ config DEBUG > > You probably want to say 'N' here. > > +config DEBUG_INFO > + bool "Compile Xen with debug info" > + default y At the very least this should be default DEBUG But what is the difference between this and DEBUG anyway? Wei. > + ---help--- > + If you say Y here the resulting Xen will include debugging info > + resulting in a larger binary image. > + > if DEBUG || EXPERT = "y" > > config CRASH_DEBUG > diff --git a/xen/Rules.mk b/xen/Rules.mk > index 47c954425d..47c5c694d6 100644 > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -55,7 +55,10 @@ endif > > CFLAGS += -nostdinc -fno-builtin -fno-common > CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith > -CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h > +ifeq ($(CONFIG_DEBUG_INFO),y) > +CFLAGS += -g > +endif > +CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h > CFLAGS += '-D__OBJECT_FILE__="$@"' > > ifneq ($(clang),y) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
