[PATCH v3] tools: detect appropriate debug optimization level

[PATCH v3] tools: detect appropriate debug optimization level

[Doug Goldstein]

When building debug use -Og as the optimization level if its available,
otherwise retain the use of -O0. -Og has been added by GCC to enable all
optimizations that to not affect debugging while retaining full
debugability.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
change since v2:
- switch back to cc-option-add to not call cc-option on every invocation
change since v1:
- switch to cc-option to only specify -O0 if -Og isn't supported
---
 tools/Rules.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 9ef0b47..1b79a6e 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -138,6 +138,7 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
 ifeq ($(debug),y)
 # Disable optimizations and enable debugging information for macros
 CFLAGS += -O0 -g3
+$(call cc-option-add,CFLAGS,CC,-Og)
 # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=<n>.
 PY_CFLAGS += $(PY_NOOPT_CFLAGS)
 endif
-- 
2.7.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3] tools: detect appropriate debug optimization level

[Wei Liu]

On Tue, Apr 26, 2016 at 09:38:45AM -0500, Doug Goldstein wrote:
> When building debug use -Og as the optimization level if its available,
> otherwise retain the use of -O0. -Og has been added by GCC to enable all
> optimizations that to not affect debugging while retaining full
> debugability.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

I think Ian's comment has been addressed in this version:

Acked-by: Wei Liu <wei.liu2@citrix.com>

I plan to accept this patch for 4.7, the risk is rather low anyway.

I will wait a bit before committing in case Ian has more to say.

> ---
> change since v2:
> - switch back to cc-option-add to not call cc-option on every invocation
> change since v1:
> - switch to cc-option to only specify -O0 if -Og isn't supported
> ---
>  tools/Rules.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 9ef0b47..1b79a6e 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -138,6 +138,7 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
>  ifeq ($(debug),y)
>  # Disable optimizations and enable debugging information for macros
>  CFLAGS += -O0 -g3
> +$(call cc-option-add,CFLAGS,CC,-Og)
>  # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=<n>.
>  PY_CFLAGS += $(PY_NOOPT_CFLAGS)
>  endif
> -- 
> 2.7.3
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3] tools: detect appropriate debug optimization level

[Ian Jackson]

Doug Goldstein writes ("[PATCH v3] tools: detect appropriate debug optimization level"):
> When building debug use -Og as the optimization level if its available,
> otherwise retain the use of -O0. -Og has been added by GCC to enable all
> optimizations that to not affect debugging while retaining full
> debugability.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3] tools: detect appropriate debug optimization level

[Wei Liu]

On Thu, Apr 28, 2016 at 06:19:19PM +0100, Ian Jackson wrote:
> Doug Goldstein writes ("[PATCH v3] tools: detect appropriate debug optimization level"):
> > When building debug use -Og as the optimization level if its available,
> > otherwise retain the use of -O0. -Og has been added by GCC to enable all
> > optimizations that to not affect debugging while retaining full
> > debugability.
> > 
> > Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Queued.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3] tools: detect appropriate debug optimization level

[Wei Liu]

On Tue, Apr 26, 2016 at 09:38:45AM -0500, Doug Goldstein wrote:
> When building debug use -Og as the optimization level if its available,
> otherwise retain the use of -O0. -Og has been added by GCC to enable all
> optimizations that to not affect debugging while retaining full
> debugability.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> ---
> change since v2:
> - switch back to cc-option-add to not call cc-option on every invocation
> change since v1:
> - switch to cc-option to only specify -O0 if -Og isn't supported
> ---
>  tools/Rules.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 9ef0b47..1b79a6e 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -138,6 +138,7 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
>  ifeq ($(debug),y)
>  # Disable optimizations and enable debugging information for macros
>  CFLAGS += -O0 -g3
> +$(call cc-option-add,CFLAGS,CC,-Og)
>  # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=<n>.
>  PY_CFLAGS += $(PY_NOOPT_CFLAGS)
>  endif
> -- 
> 2.7.3

Heh, this patch seems to has a (good) side effect. I will look into
fixing that tomorrow.

make[10]: Entering directory '/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/tcgbios'
gcc   -O1 -fno-omit-frame-pointer -m32 -march=i686 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O0 -g3 -Og -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF .tcgbios.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -mno-tls-direct-seg-refs  -Werror -fno-stack-protector -fno-exceptions -fno-builtin -msoft-float -I/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/tcgbios/../../../../../tools/include -I.. -I../..  -c -o tcgbios.o tcgbios.c 
gcc   -O1 -fno-omit-frame-pointer -m32 -march=i686 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O0 -g3 -Og -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF .tpm_drivers.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -mno-tls-direct-seg-refs  -Werror -fno-stack-protector -fno-exceptions -fno-builtin -msoft-float -I/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/tcgbios/../../../../../tools/include -I.. -I../..  -c -o tpm_drivers.o tpm_drivers.c 
tcgbios.c: In function ‘tcpa_extend_acpi_log’:
tcgbios.c:362:3: error: ‘size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   memcpy((char *)lasa_last, (char *)entry_ptr, size);
   ^
tcgbios.c: In function ‘HashLogEvent32’:
tcgbios.c:1142:22: error: ‘entry’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    hleo->eventnumber = entry;
                      ^
tcgbios.c:1131:10: error: ‘logdataptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    entry = tcpa_extend_acpi_log(logdataptr);
          ^
cc1: all warnings being treated as errors
/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/tcgbios/../../../../../tools/Rules.mk:192: recipe for target 'tcgbios.o' failed
make[10]: *** [tcgbios.o] Error 1
make[10]: Leaving directory '/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/tcgbios'
/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/../../../../tools/Rules.mk:216: recipe for target 'subdir-all-tcgbios' failed
make[9]: *** [subdir-all-tcgbios] Error 2
make[9]: Leaving directory '/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit'
/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/../../../../tools/Rules.mk:211: recipe for target 'subdirs-all' failed
make[8]: *** [subdirs-all] Error 2
make[8]: Leaving directory '/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit'
/local/work/COMMITTER/xen.git/tools/firmware/rombios/../../../tools/Rules.mk:216: recipe for target 'subdir-all-32bit' failed
make[7]: *** [subdir-all-32bit] Error 2
make[7]: Leaving directory '/local/work/COMMITTER/xen.git/tools/firmware/rombios'
/local/work/COMMITTER/xen.git/tools/firmware/rombios/../../../tools/Rules.mk:211: recipe for target 'subdirs-all' failed
make[6]: *** [subdirs-all] Error 2
make[6]: Leaving directory '/local/work/COMMITTER/xen.git/tools/firmware/rombios'
/local/work/COMMITTER/xen.git/tools/firmware/../../tools/Rules.mk:216: recipe for target 'subdir-all-rombios' failed
make[5]: *** [subdir-all-rombios] Error 2
make[5]: Leaving directory '/local/work/COMMITTER/xen.git/tools/firmware'
/local/work/COMMITTER/xen.git/tools/firmware/../../tools/Rules.mk:211: recipe for target 'subdirs-all' failed
make[4]: *** [subdirs-all] Error 2
make[4]: Leaving directory '/local/work/COMMITTER/xen.git/tools/firmware'
Makefile:33: recipe for target 'all' failed
make[3]: *** [all] Error 2
make[3]: Leaving directory '/local/work/COMMITTER/xen.git/tools/firmware'
/local/work/COMMITTER/xen.git/tools/../tools/Rules.mk:216: recipe for target 'subdir-install-firmware' failed
make[2]: *** [subdir-install-firmware] Error 2
make[2]: Leaving directory '/local/work/COMMITTER/xen.git/tools'
/local/work/COMMITTER/xen.git/tools/../tools/Rules.mk:211: recipe for target 'subdirs-install' failed
make[1]: *** [subdirs-install] Error 2
make[1]: Leaving directory '/local/work/COMMITTER/xen.git/tools'
Makefile:101: recipe for target 'install-tools' failed
make: *** [install-tools] Error 2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3] tools: detect appropriate debug optimization level

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 3546 bytes --]

On 4/28/16 12:40 PM, Wei Liu wrote:
> On Tue, Apr 26, 2016 at 09:38:45AM -0500, Doug Goldstein wrote:
>> When building debug use -Og as the optimization level if its available,
>> otherwise retain the use of -O0. -Og has been added by GCC to enable all
>> optimizations that to not affect debugging while retaining full
>> debugability.
>>
>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>> ---
>> change since v2:
>> - switch back to cc-option-add to not call cc-option on every invocation
>> change since v1:
>> - switch to cc-option to only specify -O0 if -Og isn't supported
>> ---
>>  tools/Rules.mk | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>> index 9ef0b47..1b79a6e 100644
>> --- a/tools/Rules.mk
>> +++ b/tools/Rules.mk
>> @@ -138,6 +138,7 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
>>  ifeq ($(debug),y)
>>  # Disable optimizations and enable debugging information for macros
>>  CFLAGS += -O0 -g3
>> +$(call cc-option-add,CFLAGS,CC,-Og)
>>  # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=<n>.
>>  PY_CFLAGS += $(PY_NOOPT_CFLAGS)
>>  endif
>> -- 
>> 2.7.3
> 
> Heh, this patch seems to has a (good) side effect. I will look into
> fixing that tomorrow.
> 
> make[10]: Entering directory '/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/tcgbios'
> gcc   -O1 -fno-omit-frame-pointer -m32 -march=i686 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O0 -g3 -Og -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF .tcgbios.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -mno-tls-direct-seg-refs  -Werror -fno-stack-protector -fno-exceptions -fno-builtin -msoft-float -I/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/tcgbios/../../../../../tools/include -I.. -I../..  -c -o tcgbios.o tcgbios.c 
> gcc   -O1 -fno-omit-frame-pointer -m32 -march=i686 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O0 -g3 -Og -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF .tpm_drivers.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -mno-tls-direct-seg-refs  -Werror -fno-stack-protector -fno-exceptions -fno-builtin -msoft-float -I/local/work/COMMITTER/xen.git/tools/firmware/rombios/32bit/tcgbios/../../../../../tools/include -I.. -I../..  -c -o tpm_drivers.o tpm_drivers.c 
> tcgbios.c: In function ‘tcpa_extend_acpi_log’:
> tcgbios.c:362:3: error: ‘size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    memcpy((char *)lasa_last, (char *)entry_ptr, size);
>    ^
> tcgbios.c: In function ‘HashLogEvent32’:
> tcgbios.c:1142:22: error: ‘entry’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>     hleo->eventnumber = entry;
>                       ^
> tcgbios.c:1131:10: error: ‘logdataptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>     entry = tcpa_extend_acpi_log(logdataptr);
>           ^

This appears to be fixed with 0b5e769ae3ac81f8a8dfaf476147ccdf41caeb68
but the patch referenced here never landed. The patch doesn't apply
cleanly anymore (due to context) but its a one-liner so maybe someone
can fix it up as its committed?

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 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 v3] tools: detect appropriate debug optimization level

[Wei Liu]

On Fri, Feb 23, 2018 at 11:26:17PM -0600, Doug Goldstein wrote:
> On 4/28/16 12:40 PM, Wei Liu wrote:
> > On Tue, Apr 26, 2016 at 09:38:45AM -0500, Doug Goldstein wrote:
> >> When building debug use -Og as the optimization level if its available,
> >> otherwise retain the use of -O0. -Og has been added by GCC to enable all
> >> optimizations that to not affect debugging while retaining full
> >> debugability.
> >>
> >> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> >> ---
> >> change since v2:
> >> - switch back to cc-option-add to not call cc-option on every invocation
> >> change since v1:
> >> - switch to cc-option to only specify -O0 if -Og isn't supported
> >> ---
> >>  tools/Rules.mk | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/tools/Rules.mk b/tools/Rules.mk
> >> index 9ef0b47..1b79a6e 100644
> >> --- a/tools/Rules.mk
> >> +++ b/tools/Rules.mk
> >> @@ -138,6 +138,7 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
> >>  ifeq ($(debug),y)
> >>  # Disable optimizations and enable debugging information for macros
> >>  CFLAGS += -O0 -g3
> >> +$(call cc-option-add,CFLAGS,CC,-Og)

Though -Og will supersede -O0 because it comes later, I would rather you
use cc-option to selectively add one of the two, like:

CFLAGS += $(cc-option,CC,-Og,-O0)

Wei.

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

Re: [PATCH v3] tools: detect appropriate debug optimization level

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 1622 bytes --]

On 3/2/18 6:46 AM, Wei Liu wrote:
> On Fri, Feb 23, 2018 at 11:26:17PM -0600, Doug Goldstein wrote:
>> On 4/28/16 12:40 PM, Wei Liu wrote:
>>> On Tue, Apr 26, 2016 at 09:38:45AM -0500, Doug Goldstein wrote:
>>>> When building debug use -Og as the optimization level if its available,
>>>> otherwise retain the use of -O0. -Og has been added by GCC to enable all
>>>> optimizations that to not affect debugging while retaining full
>>>> debugability.
>>>>
>>>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>>>> ---
>>>> change since v2:
>>>> - switch back to cc-option-add to not call cc-option on every invocation
>>>> change since v1:
>>>> - switch to cc-option to only specify -O0 if -Og isn't supported
>>>> ---
>>>>  tools/Rules.mk | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>>>> index 9ef0b47..1b79a6e 100644
>>>> --- a/tools/Rules.mk
>>>> +++ b/tools/Rules.mk
>>>> @@ -138,6 +138,7 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
>>>>  ifeq ($(debug),y)
>>>>  # Disable optimizations and enable debugging information for macros
>>>>  CFLAGS += -O0 -g3
>>>> +$(call cc-option-add,CFLAGS,CC,-Og)
> 
> Though -Og will supersede -O0 because it comes later, I would rather you
> use cc-option to selectively add one of the two, like:
> 
> CFLAGS += $(cc-option,CC,-Og,-O0)
> 
> Wei.
> 

Wei,

It was like that in v2 but I changed it per Ian's request to this
version. See:
https://lists.xenproject.org/archives/html/xen-devel/2016-04/msg02822.html
for reference.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 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 v3] tools: detect appropriate debug optimization level

[Wei Liu]

On Sat, Mar 10, 2018 at 11:08:21PM -0600, Doug Goldstein wrote:
> On 3/2/18 6:46 AM, Wei Liu wrote:
> > On Fri, Feb 23, 2018 at 11:26:17PM -0600, Doug Goldstein wrote:
> >> On 4/28/16 12:40 PM, Wei Liu wrote:
> >>> On Tue, Apr 26, 2016 at 09:38:45AM -0500, Doug Goldstein wrote:
> >>>> When building debug use -Og as the optimization level if its available,
> >>>> otherwise retain the use of -O0. -Og has been added by GCC to enable all
> >>>> optimizations that to not affect debugging while retaining full
> >>>> debugability.
> >>>>
> >>>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> >>>> ---
> >>>> change since v2:
> >>>> - switch back to cc-option-add to not call cc-option on every invocation
> >>>> change since v1:
> >>>> - switch to cc-option to only specify -O0 if -Og isn't supported
> >>>> ---
> >>>>  tools/Rules.mk | 1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/tools/Rules.mk b/tools/Rules.mk
> >>>> index 9ef0b47..1b79a6e 100644
> >>>> --- a/tools/Rules.mk
> >>>> +++ b/tools/Rules.mk
> >>>> @@ -138,6 +138,7 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
> >>>>  ifeq ($(debug),y)
> >>>>  # Disable optimizations and enable debugging information for macros
> >>>>  CFLAGS += -O0 -g3
> >>>> +$(call cc-option-add,CFLAGS,CC,-Og)
> > 
> > Though -Og will supersede -O0 because it comes later, I would rather you
> > use cc-option to selectively add one of the two, like:
> > 
> > CFLAGS += $(cc-option,CC,-Og,-O0)
> > 
> > Wei.
> > 
> 
> Wei,
> 
> It was like that in v2 but I changed it per Ian's request to this
> version. See:
> https://lists.xenproject.org/archives/html/xen-devel/2016-04/msg02822.html
> for reference.

That's a good point. We certainly want to avoid expanding the same thing
over and over again.

I think you can do with

ifneq ($(call cc-option,$(CC),-Og,n),n)
CFLAGS += -Og
else
CFLAGS += -O0
endif

Wei.

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