[Qemu-devel] [PATCH 1/3] build-sys: remove useless extra*flags variables

[Qemu-devel] [PATCH 1/3] build-sys: remove useless extra*flags variables

[Marc-André Lureau]

Only EXTRA_LDFLAGS seems to be used during configure Xen checks.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 configure | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/configure b/configure
index 831ebf248f..41d8a2666c 100755
--- a/configure
+++ b/configure
@@ -471,10 +471,8 @@ for opt do
   --cpu=*) cpu="$optarg"
   ;;
   --extra-cflags=*) QEMU_CFLAGS="$QEMU_CFLAGS $optarg"
-                    EXTRA_CFLAGS="$optarg"
   ;;
   --extra-cxxflags=*) QEMU_CXXFLAGS="$QEMU_CXXFLAGS $optarg"
-                      EXTRA_CXXFLAGS="$optarg"
   ;;
   --extra-ldflags=*) LDFLAGS="$LDFLAGS $optarg"
                      EXTRA_LDFLAGS="$optarg"
@@ -1424,7 +1422,6 @@ case "$cpu" in
 esac
 
 QEMU_CFLAGS="$CPU_CFLAGS $QEMU_CFLAGS"
-EXTRA_CFLAGS="$CPU_CFLAGS $EXTRA_CFLAGS"
 
 # For user-mode emulation the host arch has to be one we explicitly
 # support, even if we're using TCI.
@@ -5817,9 +5814,6 @@ if test "$mingw32" = "no" ; then
   echo "qemu_localstatedir=$local_statedir" >> $config_host_mak
 fi
 echo "qemu_helperdir=$libexecdir" >> $config_host_mak
-echo "extra_cflags=$EXTRA_CFLAGS" >> $config_host_mak
-echo "extra_cxxflags=$EXTRA_CXXFLAGS" >> $config_host_mak
-echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak
 echo "qemu_localedir=$qemu_localedir" >> $config_host_mak
 echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
 echo "GIT=$git" >> $config_host_mak
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 2/3] build-sys: check static linking of UBSAN

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 configure | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 41d8a2666c..beb0de6a77 100755
--- a/configure
+++ b/configure
@@ -5247,7 +5247,15 @@ fi
 ##########################################
 # checks for sanitizers
 
-write_c_skeleton
+# we could use a simple skeleton for flags chekcs, but this also
+# detect the static linking issue of ubsan, see also:
+# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84285
+cat > $TMPC << EOF
+#include <stdint.h>
+int main(void) {
+  return INT32_MIN / -1;
+}
+EOF
 
 have_asan=no
 have_ubsan=no
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 3/3] build-sys: enable sanitizers by default with --enable-debug

[Marc-André Lureau]

The original commit 247724cb302af5d70c8853154b640dfabf2bbb56 was meant
to enable sanitizers by default when --enable-debug, but failed
because of a gcc static linking bug. Try to enable it back now that
there is a stronger check.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 configure | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index beb0de6a77..700ff35383 100755
--- a/configure
+++ b/configure
@@ -355,7 +355,7 @@ rdma=""
 gprof="no"
 debug_tcg="no"
 debug="no"
-sanitizers="no"
+sanitizers=""
 fortify_source=""
 strip_opt="yes"
 tcg_interpreter="no"
@@ -5262,6 +5262,11 @@ have_ubsan=no
 have_asan_iface_h=no
 have_asan_iface_fiber=no
 
+# enable sanitizers by default if --enable-debug
+if test "$sanitizers" = "" -a "$debug" = "yes"; then
+  sanitizers=yes
+fi
+
 if test "$sanitizers" = "yes" ; then
   if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address" ""; then
       have_asan=yes
-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH 3/3] build-sys: enable sanitizers by default with --enable-debug

[Paolo Bonzini]

On 08/02/2018 17:23, Marc-André Lureau wrote:
> The original commit 247724cb302af5d70c8853154b640dfabf2bbb56 was meant
> to enable sanitizers by default when --enable-debug, but failed
> because of a gcc static linking bug. Try to enable it back now that
> there is a stronger check.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

I'm more afraid that there are quite a few reports from sanitizers.  I
wonder if that makes --enable-debug unusable; as a non-user of
--enable-debug I'm a bit wary of pushing this patch.

I've queued 1 and 2 though.

Paolo

> ---
>  configure | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index beb0de6a77..700ff35383 100755
> --- a/configure
> +++ b/configure
> @@ -355,7 +355,7 @@ rdma=""
>  gprof="no"
>  debug_tcg="no"
>  debug="no"
> -sanitizers="no"
> +sanitizers=""
>  fortify_source=""
>  strip_opt="yes"
>  tcg_interpreter="no"
> @@ -5262,6 +5262,11 @@ have_ubsan=no
>  have_asan_iface_h=no
>  have_asan_iface_fiber=no
>  
> +# enable sanitizers by default if --enable-debug
> +if test "$sanitizers" = "" -a "$debug" = "yes"; then
> +  sanitizers=yes
> +fi
> +
>  if test "$sanitizers" = "yes" ; then
>    if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address" ""; then
>        have_asan=yes
> 

Re: [Qemu-devel] [PATCH 2/3] build-sys: check static linking of UBSAN

[Eric Blake]

On 02/08/2018 10:23 AM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   configure | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 41d8a2666c..beb0de6a77 100755
> --- a/configure
> +++ b/configure
> @@ -5247,7 +5247,15 @@ fi
>   ##########################################
>   # checks for sanitizers
>   
> -write_c_skeleton
> +# we could use a simple skeleton for flags chekcs, but this also

s/chekcs/checks/

> +# detect the static linking issue of ubsan, see also:
> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84285
> +cat > $TMPC << EOF
> +#include <stdint.h>
> +int main(void) {
> +  return INT32_MIN / -1;
> +}
> +EOF
>   
>   have_asan=no
>   have_ubsan=no
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 3/3] build-sys: enable sanitizers by default with --enable-debug

[Marc-André Lureau]

Hi

On Thu, Feb 8, 2018 at 6:46 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 08/02/2018 17:23, Marc-André Lureau wrote:
>> The original commit 247724cb302af5d70c8853154b640dfabf2bbb56 was meant
>> to enable sanitizers by default when --enable-debug, but failed
>> because of a gcc static linking bug. Try to enable it back now that
>> there is a stronger check.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> I'm more afraid that there are quite a few reports from sanitizers.  I
> wonder if that makes --enable-debug unusable; as a non-user of
> --enable-debug I'm a bit wary of pushing this patch.

I understand the concern, but at the same time, people should care
about fixing those. If they want to keep ignoring them (for bad
reasons), they can --disable-sanitizers. At least, I would want to
reach a point where no ASAN regression get introduced in x86 target.
It would be nice if patchew warn on new ASAN reports for example.

>
> I've queued 1 and 2 though.
>
> Paolo
>
>> ---
>>  configure | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index beb0de6a77..700ff35383 100755
>> --- a/configure
>> +++ b/configure
>> @@ -355,7 +355,7 @@ rdma=""
>>  gprof="no"
>>  debug_tcg="no"
>>  debug="no"
>> -sanitizers="no"
>> +sanitizers=""
>>  fortify_source=""
>>  strip_opt="yes"
>>  tcg_interpreter="no"
>> @@ -5262,6 +5262,11 @@ have_ubsan=no
>>  have_asan_iface_h=no
>>  have_asan_iface_fiber=no
>>
>> +# enable sanitizers by default if --enable-debug
>> +if test "$sanitizers" = "" -a "$debug" = "yes"; then
>> +  sanitizers=yes
>> +fi
>> +
>>  if test "$sanitizers" = "yes" ; then
>>    if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address" ""; then
>>        have_asan=yes
>>
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 3/3] build-sys: enable sanitizers by default with --enable-debug

[Fam Zheng]

On Thu, 02/08 20:03, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Feb 8, 2018 at 6:46 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > On 08/02/2018 17:23, Marc-André Lureau wrote:
> >> The original commit 247724cb302af5d70c8853154b640dfabf2bbb56 was meant
> >> to enable sanitizers by default when --enable-debug, but failed
> >> because of a gcc static linking bug. Try to enable it back now that
> >> there is a stronger check.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > I'm more afraid that there are quite a few reports from sanitizers.  I
> > wonder if that makes --enable-debug unusable; as a non-user of
> > --enable-debug I'm a bit wary of pushing this patch.
> 
> I understand the concern, but at the same time, people should care
> about fixing those. If they want to keep ignoring them (for bad
> reasons), they can --disable-sanitizers. At least, I would want to
> reach a point where no ASAN regression get introduced in x86 target.
> It would be nice if patchew warn on new ASAN reports for example.

Could you write a docker test for that?

Fam