[Qemu-devel] [PATCH v2] build: try improve handling of clang warnings

Gerd Hoffmann posted 1 patch 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180309135945.20436-1-kraxel@redhat.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppcbe passed
Test ppcle passed
Test s390x passed
configure                 | 2 +-
util/coroutine-ucontext.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH v2] build: try improve handling of clang warnings
Posted by Gerd Hoffmann 6 years, 1 month ago
This patch disables the pragma diagnostic -Wunused-but-set-variable for
clang in util/coroutine-ucontext.c.

This in turn allows us to remove it from the configure check, so the
CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE will succeed for clang.

With that in place clang builds (linux) will use -Werror by default,
which breaks the build due to warning about unaligned struct members.

Just turning off this warning isn't a good idea as it indicates
portability problems.  So make it a warning again, using
-Wno-error=address-of-packed-member.  That way it doesn't break the
build but still shows up in the logs.

Now clang builds qemu without errors.  Well, almost.  There are some
left in the rdma code.  Leaving that to the rdma people.  All others can
use --disable-rdma to workarounds this.

Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 configure                 | 2 +-
 util/coroutine-ucontext.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 6f3921c02a..3a12173c83 100755
--- a/configure
+++ b/configure
@@ -1692,6 +1692,7 @@ gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
 gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags"
 gcc_flags="-Wno-initializer-overrides -Wexpansion-to-defined $gcc_flags"
 gcc_flags="-Wno-string-plus-int $gcc_flags"
+gcc_flags="-Wno-error=address-of-packed-member $gcc_flags"
 # Note that we do not add -Werror to gcc_flags here, because that would
 # enable it for all configure tests. If a configure test failed due
 # to -Werror this would just silently disable some features,
@@ -4860,7 +4861,6 @@ fi
 pragma_disable_unused_but_set=no
 cat > $TMPC << EOF
 #pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wunused-but-set-variable"
 #pragma GCC diagnostic ignored "-Wstrict-prototypes"
 #pragma GCC diagnostic pop
 
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 926d3402e3..090ba21a13 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -170,7 +170,7 @@ Coroutine *qemu_coroutine_new(void)
 }
 
 #ifdef CONFIG_VALGRIND_H
-#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && !defined(__clang__)
 /* Work around an unused variable in the valgrind.h macro... */
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
@@ -179,7 +179,7 @@ static inline void valgrind_stack_deregister(CoroutineUContext *co)
 {
     VALGRIND_STACK_DEREGISTER(co->valgrind_stack_id);
 }
-#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && !defined(__clang__)
 #pragma GCC diagnostic pop
 #endif
 #endif
-- 
2.9.3


Re: [Qemu-devel] [PATCH v2] build: try improve handling of clang warnings
Posted by Thomas Huth 6 years, 1 month ago
On 09.03.2018 14:59, Gerd Hoffmann wrote:
> This patch disables the pragma diagnostic -Wunused-but-set-variable for
> clang in util/coroutine-ucontext.c.
> 
> This in turn allows us to remove it from the configure check, so the
> CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE will succeed for clang.
> 
> With that in place clang builds (linux) will use -Werror by default,
> which breaks the build due to warning about unaligned struct members.
> 
> Just turning off this warning isn't a good idea as it indicates
> portability problems.  So make it a warning again, using
> -Wno-error=address-of-packed-member.  That way it doesn't break the
> build but still shows up in the logs.
> 
> Now clang builds qemu without errors.  Well, almost.  There are some
> left in the rdma code.  Leaving that to the rdma people.  All others can
> use --disable-rdma to workarounds this.
> 
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  configure                 | 2 +-
>  util/coroutine-ucontext.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v2] build: try improve handling of clang warnings
Posted by Philippe Mathieu-Daudé 6 years, 1 month ago
On 03/09/2018 02:59 PM, Gerd Hoffmann wrote:
> This patch disables the pragma diagnostic -Wunused-but-set-variable for
> clang in util/coroutine-ucontext.c.
> 
> This in turn allows us to remove it from the configure check, so the
> CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE will succeed for clang.
> 
> With that in place clang builds (linux) will use -Werror by default,
> which breaks the build due to warning about unaligned struct members.
> 
> Just turning off this warning isn't a good idea as it indicates
> portability problems.  So make it a warning again, using
> -Wno-error=address-of-packed-member.  That way it doesn't break the
> build but still shows up in the logs.
> 
> Now clang builds qemu without errors.  Well, almost.  There are some
> left in the rdma code.  Leaving that to the rdma people.  All others can
> use --disable-rdma to workarounds this.
> 
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  configure                 | 2 +-
>  util/coroutine-ucontext.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 6f3921c02a..3a12173c83 100755
> --- a/configure
> +++ b/configure
> @@ -1692,6 +1692,7 @@ gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
>  gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags"
>  gcc_flags="-Wno-initializer-overrides -Wexpansion-to-defined $gcc_flags"
>  gcc_flags="-Wno-string-plus-int $gcc_flags"
> +gcc_flags="-Wno-error=address-of-packed-member $gcc_flags"
>  # Note that we do not add -Werror to gcc_flags here, because that would
>  # enable it for all configure tests. If a configure test failed due
>  # to -Werror this would just silently disable some features,
> @@ -4860,7 +4861,6 @@ fi
>  pragma_disable_unused_but_set=no
>  cat > $TMPC << EOF
>  #pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wunused-but-set-variable"
>  #pragma GCC diagnostic ignored "-Wstrict-prototypes"
>  #pragma GCC diagnostic pop
>  
> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
> index 926d3402e3..090ba21a13 100644
> --- a/util/coroutine-ucontext.c
> +++ b/util/coroutine-ucontext.c
> @@ -170,7 +170,7 @@ Coroutine *qemu_coroutine_new(void)
>  }
>  
>  #ifdef CONFIG_VALGRIND_H
> -#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && !defined(__clang__)
>  /* Work around an unused variable in the valgrind.h macro... */
>  #pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
> @@ -179,7 +179,7 @@ static inline void valgrind_stack_deregister(CoroutineUContext *co)
>  {
>      VALGRIND_STACK_DEREGISTER(co->valgrind_stack_id);
>  }
> -#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && !defined(__clang__)
>  #pragma GCC diagnostic pop
>  #endif
>  #endif
>