[PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Olaf Hering]

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 <olaf@aepfle.de>
---
 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
+	---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

Re: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Wei Liu]

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 <olaf@aepfle.de>
> ---
>  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

Re: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 259 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Jan Beulich]

>>> On 31.08.18 at 10:32, <wei.liu2@citrix.com> 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 <olaf@aepfle.de>
>> ---
>>  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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Andrew Cooper]

On 31/08/18 09:41, Jan Beulich wrote:
>>>> On 31.08.18 at 10:32, <wei.liu2@citrix.com> 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 <olaf@aepfle.de>
>>> ---
>>>  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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Wei Liu]

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, <wei.liu2@citrix.com> 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 <olaf@aepfle.de>
> >>> ---
> >>>  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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Jan Beulich]

>>> On 31.08.18 at 10:29, <olaf@aepfle.de> 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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 179 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Jan Beulich]

>>> On 31.08.18 at 11:02, <olaf@aepfle.de> 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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Julien Grall]

Hi Jan,

On 08/31/2018 10:25 AM, Jan Beulich wrote:
>>>> On 31.08.18 at 11:02, <olaf@aepfle.de> 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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Jan Beulich]

>>> On 31.08.18 at 11:50, <julien.grall@arm.com> wrote:
> On 08/31/2018 10:25 AM, Jan Beulich wrote:
>>>>> On 31.08.18 at 11:02, <olaf@aepfle.de> 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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Jan Beulich]

>>> On 31.08.18 at 10:43, <JBeulich@suse.com> wrote:
>>>> On 31.08.18 at 10:29, <olaf@aepfle.de> 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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Jan Beulich]

>>> On 10.09.18 at 15:21, <JBeulich@suse.com> wrote:
>>>> On 31.08.18 at 10:43, <JBeulich@suse.com> wrote:
>>>>> On 31.08.18 at 10:29, <olaf@aepfle.de> 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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Andrew Cooper]

On 10/09/18 14:29, Jan Beulich wrote:
>>>> On 10.09.18 at 15:21, <JBeulich@suse.com> wrote:
>>>>> On 31.08.18 at 10:43, <JBeulich@suse.com> wrote:
>>>>>> On 31.08.18 at 10:29, <olaf@aepfle.de> 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: [PATCH v2] xen: add DEBUG_INFO Kconfig symbol

[Jan Beulich]

>>> On 10.09.18 at 19:51, <andrew.cooper3@citrix.com> wrote:
> On 10/09/18 14:29, Jan Beulich wrote:
>>>>> On 10.09.18 at 15:21, <JBeulich@suse.com> wrote:
>>>>>> On 31.08.18 at 10:43, <JBeulich@suse.com> wrote:
>>>>>>> On 31.08.18 at 10:29, <olaf@aepfle.de> 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