| Message ID | 20171215150659.1811-5-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Various build-sys and ASAN related fixes | expand |
Hi On Fri, Dec 15, 2017 at 4:06 PM, Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > Enable ASAN by default if the compiler supports it. > > If necessary, we could consider a seperate configure option, although > I like the idea to have it enabled by default with --enable-debug. Peter, Paolo, Fam, any thoughts about having ASAN enabled by default with --enable-debug? (when available) Slow down is not really noticeable to me when running make check, but I can do some measurements if that helps. thanks > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > configure | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/configure b/configure > index 2b8c71f522..52d9fd71e5 100755 > --- a/configure > +++ b/configure > @@ -5129,6 +5129,11 @@ elif test "$fortify_source" = "yes" ; then > CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS" > elif test "$debug" = "no"; then > CFLAGS="-O2 $CFLAGS" > +elif test "$debug" = "yes"; then > + write_c_skeleton; > + if compile_prog "-fsanitize=address" ""; then > + CFLAGS="-fsanitize=address $CFLAGS" > + fi > fi > > ########################################## > -- > 2.15.1.355.g36791d7216 > >
On Tue, Dec 19, 2017 at 4:48 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > Hi > > On Fri, Dec 15, 2017 at 4:06 PM, Marc-André Lureau > <marcandre.lureau@redhat.com> wrote: >> Enable ASAN by default if the compiler supports it. >> >> If necessary, we could consider a seperate configure option, although >> I like the idea to have it enabled by default with --enable-debug. > > Peter, Paolo, Fam, any thoughts about having ASAN enabled by default > with --enable-debug? (when available) > > Slow down is not really noticeable to me when running make check, but > I can do some measurements if that helps. > > thanks ping, thanks > > >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> --- >> configure | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/configure b/configure >> index 2b8c71f522..52d9fd71e5 100755 >> --- a/configure >> +++ b/configure >> @@ -5129,6 +5129,11 @@ elif test "$fortify_source" = "yes" ; then >> CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS" >> elif test "$debug" = "no"; then >> CFLAGS="-O2 $CFLAGS" >> +elif test "$debug" = "yes"; then >> + write_c_skeleton; >> + if compile_prog "-fsanitize=address" ""; then >> + CFLAGS="-fsanitize=address $CFLAGS" >> + fi >> fi >> >> ########################################## >> -- >> 2.15.1.355.g36791d7216 >> >> > > > > -- > Marc-André Lureau
On 02/01/2018 16:49, Marc-André Lureau wrote: >>> If necessary, we could consider a seperate configure option, although >>> I like the idea to have it enabled by default with --enable-debug. >> Peter, Paolo, Fam, any thoughts about having ASAN enabled by default >> with --enable-debug? (when available) >> >> Slow down is not really noticeable to me when running make check, but >> I can do some measurements if that helps. >> >> thanks > ping, thanks > Sounds good, but: 1) please remove --enable-debug from existing docker tests and add a new one based on tests/docker/test-full; 2) I think removing -O2 from --enable-debug should be removed at the same time. That pretty much guarantees that nobody will use --enable-debug, and optimized builds are decently debuggable nowadays. The best would be to detect -Og, and add either -Og or -O1 depending on presence. Paolo
On 2 January 2018 at 17:31, Paolo Bonzini <pbonzini@redhat.com> wrote: > 2) I think removing -O2 from --enable-debug should be removed at the > same time. That pretty much guarantees that nobody will use > --enable-debug, and optimized builds are decently debuggable nowadays. > The best would be to detect -Og, and add either -Og or -O1 depending on > presence. Hmm. I use --enable-debug all the time and one of the reasons I use it is that the optimized build is usually more pain to debug with... thanks -- PMM
Hi On Wed, Jan 3, 2018 at 6:52 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 2 January 2018 at 17:31, Paolo Bonzini <pbonzini@redhat.com> wrote: >> 2) I think removing -O2 from --enable-debug should be removed at the >> same time. That pretty much guarantees that nobody will use >> --enable-debug, and optimized builds are decently debuggable nowadays. >> The best would be to detect -Og, and add either -Og or -O1 depending on >> presence. > > Hmm. I use --enable-debug all the time and one of the reasons > I use it is that the optimized build is usually more pain > to debug with... -Og Optimize debugging experience. -Og enables optimizations that do not interfere with debugging. It should be the optimization level of choice for the standard edit-compile-debug cycle, offering a reasonable level of optimization while maintaining fast compilation and a good debugging experience. That should cover debugging nicely. Tbh, I am quite happy with compiler default to O0 when --enable-debug. Og doesn't give me much different experience. However, it produces false-positive warnings with gcc. Quoting the patch I was about to send: Unfortunately, gcc has many false-positive maybe-uninitialized errors with Og and O1 (f27 gcc 7.2.1 20170915): /home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c: In function ‘ipmi_kcs_ioport_read’: /home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c:279:12: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized] return ret; ^~~ cc1: all warnings being treated as errors make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ipmi/isa_ipmi_kcs.o] Error 1 make: *** Waiting for unfinished jobs.... /home/elmarco/src/qemu/hw/ide/ahci.c: In function ‘ahci_populate_sglist’: /home/elmarco/src/qemu/hw/ide/ahci.c:903:58: error: ‘tbl_entry_size’ may be used uninitialized in this function [-Werror=maybe-uninitialized] if ((off_idx == -1) || (off_pos < 0) || (off_pos > tbl_entry_size)) { ~~~~~~~~~^~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ide/ahci.o] Error 1 /home/elmarco/src/qemu/hw/display/qxl.c: In function ‘qxl_add_memslot’: /home/elmarco/src/qemu/hw/display/qxl.c:1397:52: error: ‘pci_start’ may be used uninitialized in this function [-Werror=maybe-uninitialized] memslot.virt_end = virt_start + (guest_end - pci_start); ~~~~~~~~~~~~~^~~~~~~~~~~~ /home/elmarco/src/qemu/hw/display/qxl.c:1389:9: error: ‘pci_region’ may be used uninitialized in this function [-Werror=maybe-uninitialized] qxl_set_guest_bug(d, "%s: pci_region = %d", __func__, pci_region); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors There seems to be a long list of related bugs in upstream GCC, some of them are being fixed very recently: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639 For now, let's workaround it by using Wno-maybe-uninitialized (gcc-only).
On 03/01/2018 19:02, Marc-André Lureau wrote: > Hi > > On Wed, Jan 3, 2018 at 6:52 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 2 January 2018 at 17:31, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> 2) I think removing -O2 from --enable-debug should be removed at the >>> same time. That pretty much guarantees that nobody will use >>> --enable-debug, and optimized builds are decently debuggable nowadays. >>> The best would be to detect -Og, and add either -Og or -O1 depending on >>> presence. >> >> Hmm. I use --enable-debug all the time and one of the reasons >> I use it is that the optimized build is usually more pain >> to debug with... > > -Og Optimize debugging experience. -Og enables optimizations > that do not interfere with debugging. It should be the optimization > level of choice for the standard edit-compile-debug cycle, offering > a reasonable level of optimization while maintaining fast > compilation and a good debugging experience. > > That should cover debugging nicely. Tbh, I am quite happy with > compiler default to O0 when --enable-debug. Og doesn't give me much > different experience. > > However, it produces false-positive warnings with gcc. -O0 doesn't enable those warnings at all, because it would have even more false positives. :) Paolo
diff --git a/configure b/configure index 2b8c71f522..52d9fd71e5 100755 --- a/configure +++ b/configure @@ -5129,6 +5129,11 @@ elif test "$fortify_source" = "yes" ; then CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS" elif test "$debug" = "no"; then CFLAGS="-O2 $CFLAGS" +elif test "$debug" = "yes"; then + write_c_skeleton; + if compile_prog "-fsanitize=address" ""; then + CFLAGS="-fsanitize=address $CFLAGS" + fi fi ##########################################
Enable ASAN by default if the compiler supports it. If necessary, we could consider a seperate configure option, although I like the idea to have it enabled by default with --enable-debug. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- configure | 5 +++++ 1 file changed, 5 insertions(+)