qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] build: Silence clang warning on older glib autoptr usage
@ 2020-03-17 17:55 Eric Blake
  2020-03-17 18:01 ` John Snow
  2020-03-17 18:11 ` Peter Maydell
  0 siblings, 2 replies; 7+ messages in thread
From: Eric Blake @ 2020-03-17 17:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, jsnow, peter.maydell

glib's G_DEFINE_AUTOPTR_CLEANUP_FUNC() macro defines several static
inline functions, often with some of them unused, but prior to 2.57.2
did not mark the functions as such.  As a result, clang (but not gcc)
fails to build with older glib unless -Wno-unused-function is enabled.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Eric Blake <eblake@redhat.com>
---

Half-tested: I proved to myself that this does NOT enable
-Wno-unused-function on my setup of glib 2.62.5 and gcc 9.2.1 (Fedora
31), but would do so if I introduced an intentional compile error into
the sample program; but Iwas unable to test that it would prevent the
build failure encountered by Peter on John's pull request (older glib
but exact version unknown, clang, on NetBSD).

 configure | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/configure b/configure
index eb49bb6680c1..57a72f120aa9 100755
--- a/configure
+++ b/configure
@@ -3832,6 +3832,26 @@ if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; then
     fi
 fi

+# Silence clang warnings triggered by glib < 2.57.2
+cat > $TMPC << EOF
+#include <glib.h>
+typedef struct Foo {
+    int i;
+} Foo;
+static void foo_free(Foo *f)
+{
+    g_free(f);
+}
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free);
+int main(void) { return 0; }
+EOF
+if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; then
+    if cc_has_warning_flag "-Wno-unused-function"; then
+        glib_cflags="$glib_cflags -Wno-unused-function"
+        CFLAGS="$CFLAGS -Wno-unused-function"
+    fi
+fi
+
 #########################################
 # zlib check

-- 
2.25.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: Silence clang warning on older glib autoptr usage
  2020-03-17 17:55 [PATCH] build: Silence clang warning on older glib autoptr usage Eric Blake
@ 2020-03-17 18:01 ` John Snow
  2020-03-17 18:11 ` Peter Maydell
  1 sibling, 0 replies; 7+ messages in thread
From: John Snow @ 2020-03-17 18:01 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: peter.maydell, berrange



On 3/17/20 1:55 PM, Eric Blake wrote:
> glib's G_DEFINE_AUTOPTR_CLEANUP_FUNC() macro defines several static
> inline functions, often with some of them unused, but prior to 2.57.2
> did not mark the functions as such.  As a result, clang (but not gcc)
> fails to build with older glib unless -Wno-unused-function is enabled.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> Half-tested: I proved to myself that this does NOT enable
> -Wno-unused-function on my setup of glib 2.62.5 and gcc 9.2.1 (Fedora
> 31), but would do so if I introduced an intentional compile error into
> the sample program; but Iwas unable to test that it would prevent the
> build failure encountered by Peter on John's pull request (older glib
> but exact version unknown, clang, on NetBSD).
> 
>  configure | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/configure b/configure
> index eb49bb6680c1..57a72f120aa9 100755
> --- a/configure
> +++ b/configure
> @@ -3832,6 +3832,26 @@ if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; then
>      fi
>  fi
> 
> +# Silence clang warnings triggered by glib < 2.57.2
> +cat > $TMPC << EOF
> +#include <glib.h>
> +typedef struct Foo {
> +    int i;
> +} Foo;
> +static void foo_free(Foo *f)
> +{
> +    g_free(f);
> +}
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free);
> +int main(void) { return 0; }
> +EOF
> +if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; then
> +    if cc_has_warning_flag "-Wno-unused-function"; then
> +        glib_cflags="$glib_cflags -Wno-unused-function"
> +        CFLAGS="$CFLAGS -Wno-unused-function"
> +    fi
> +fi
> +
>  #########################################
>  # zlib check
> 

This looks good to me. By using the glib function under question as the
test program we disable the warning only on some very limited cases.

This might silence SOME legitimate code hygiene warnings so long as
older glib is being used, but there's simply nothing we can do to
prevent this unless glibc is updated on those machines.

Unfortunately, I do not have a NetBSD machine at present, nor do I have
enough familiarity with that platform to make any informed decisions
about if we can say:

- NetBSD is only supported when using glibc XXX and newer

because I do not know if we support any NetBSD platforms that do not
have the updated library in their package repositories.

So, given all of that, this is the most targeted fix that I am able to
reason about at present, so:

Reviewed-by: John Snow <jsnow@redhat.com>



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: Silence clang warning on older glib autoptr usage
  2020-03-17 17:55 [PATCH] build: Silence clang warning on older glib autoptr usage Eric Blake
  2020-03-17 18:01 ` John Snow
@ 2020-03-17 18:11 ` Peter Maydell
  2020-03-18 11:19   ` Eric Blake
  2020-03-18 12:55   ` Eric Blake
  1 sibling, 2 replies; 7+ messages in thread
From: Peter Maydell @ 2020-03-17 18:11 UTC (permalink / raw)
  To: Eric Blake; +Cc: Daniel P. Berrange, John Snow, QEMU Developers

On Tue, 17 Mar 2020 at 17:55, Eric Blake <eblake@redhat.com> wrote:
>
> glib's G_DEFINE_AUTOPTR_CLEANUP_FUNC() macro defines several static
> inline functions, often with some of them unused, but prior to 2.57.2
> did not mark the functions as such.  As a result, clang (but not gcc)
> fails to build with older glib unless -Wno-unused-function is enabled.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>
> Half-tested: I proved to myself that this does NOT enable
> -Wno-unused-function on my setup of glib 2.62.5 and gcc 9.2.1 (Fedora
> 31), but would do so if I introduced an intentional compile error into
> the sample program; but Iwas unable to test that it would prevent the
> build failure encountered by Peter on John's pull request (older glib
> but exact version unknown, clang, on NetBSD).

This wasn't a NetBSD failure. I hit it on my clang-on-x86-64-Ubuntu
setup, and also on FreeBSD. (The latter is just the tests/vm
FreeBSD config, so you can repro that if you need to.)

The ubuntu setup is libglib 2.56.4-0ubuntu0.18.04.4 and
clang 6.0.0-1ubuntu2.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: Silence clang warning on older glib autoptr usage
  2020-03-17 18:11 ` Peter Maydell
@ 2020-03-18 11:19   ` Eric Blake
  2020-03-18 12:45     ` Eric Blake
  2020-03-18 12:55   ` Eric Blake
  1 sibling, 1 reply; 7+ messages in thread
From: Eric Blake @ 2020-03-18 11:19 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Daniel P. Berrange, John Snow, QEMU Developers

On 3/17/20 1:11 PM, Peter Maydell wrote:
> On Tue, 17 Mar 2020 at 17:55, Eric Blake <eblake@redhat.com> wrote:
>>
>> glib's G_DEFINE_AUTOPTR_CLEANUP_FUNC() macro defines several static
>> inline functions, often with some of them unused, but prior to 2.57.2
>> did not mark the functions as such.  As a result, clang (but not gcc)
>> fails to build with older glib unless -Wno-unused-function is enabled.
>>
>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> Half-tested: I proved to myself that this does NOT enable
>> -Wno-unused-function on my setup of glib 2.62.5 and gcc 9.2.1 (Fedora
>> 31), but would do so if I introduced an intentional compile error into
>> the sample program; but Iwas unable to test that it would prevent the
>> build failure encountered by Peter on John's pull request (older glib
>> but exact version unknown, clang, on NetBSD).
> 
> This wasn't a NetBSD failure. I hit it on my clang-on-x86-64-Ubuntu
> setup, and also on FreeBSD. (The latter is just the tests/vm
> FreeBSD config, so you can repro that if you need to.)
> 
> The ubuntu setup is libglib 2.56.4-0ubuntu0.18.04.4 and
> clang 6.0.0-1ubuntu2.

Thanks; I ran:

$ make docker-test-clang@ubuntu1804 DEBUG=1
...
bash-4.4$ cat > foo.c <<\EOF
 > #include <glib.h>
 > typedef struct Foo {
 >     int i;
 > } Foo;
 > static void foo_free(Foo *f)
 > {
 >     g_free(f);
 > }
 > G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free);
 > int main(void) { return 0; }
 > EOF
bash-4.4$ clang -Wall -Werror $(pkg-config --cflags gmodule-2.0) \
   -o foo -c foo.c
...
glib_slistautoptr_cleanup_Foo
^
3 errors generated.
bash-4.4$ clang -Wall -Werror $(pkg-config --cflags gmodule-2.0) \
   -o foo -c foo.c -Wno-unused-function
bash-4.4$

to confirm that my test snippet does flush out the error in question. 
However, removing DEBUG=1 takes a long time to run (hmm, maybe I should 
set TARGET_LIST to speed it up), and did not reproduce the failure for 
me without the patch (making it hard to tell if the patch made a 
difference).  I'm still playing with testing, but at least I feel better 
that this patch is on the right track, now that I have an environment 
that can reproduce the situation.

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: Silence clang warning on older glib autoptr usage
  2020-03-18 11:19   ` Eric Blake
@ 2020-03-18 12:45     ` Eric Blake
  0 siblings, 0 replies; 7+ messages in thread
From: Eric Blake @ 2020-03-18 12:45 UTC (permalink / raw)
  To: Peter Maydell; +Cc: John Snow, Daniel P. Berrange, QEMU Developers

On 3/18/20 6:19 AM, Eric Blake wrote:

>> This wasn't a NetBSD failure. I hit it on my clang-on-x86-64-Ubuntu
>> setup, and also on FreeBSD. (The latter is just the tests/vm
>> FreeBSD config, so you can repro that if you need to.)
>>
>> The ubuntu setup is libglib 2.56.4-0ubuntu0.18.04.4 and
>> clang 6.0.0-1ubuntu2.
> 
> Thanks; I ran:
> 
> $ make docker-test-clang@ubuntu1804 DEBUG=1

> 
> to confirm that my test snippet does flush out the error in question. 
> However, removing DEBUG=1 takes a long time to run (hmm, maybe I should 
> set TARGET_LIST to speed it up), and did not reproduce the failure for 
> me without the patch (making it hard to tell if the patch made a 
> difference).  I'm still playing with testing, but at least I feel better 
> that this patch is on the right track, now that I have an environment 
> that can reproduce the situation.

Aha - I figured out my problem: I have to apply John's pull request, but 
not my configure patch, to reproduce the build failure that Peter hit. 
And with that, I can now state that this patch HAS been fully-tested by 
me (but giving Tested-by: to my own patch feels weird):

make docker-test-clang@ubuntu1804 TARGETS=x86_64

on the following matrix:

   John's PR =>         excluded included
my patch excluded         pass    fail
my patch included         pass    pass

Looks like the next step is for John to send v2 of the pull request with 
my configure patch inserted.

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: Silence clang warning on older glib autoptr usage
  2020-03-17 18:11 ` Peter Maydell
  2020-03-18 11:19   ` Eric Blake
@ 2020-03-18 12:55   ` Eric Blake
  2020-03-18 17:18     ` John Snow
  1 sibling, 1 reply; 7+ messages in thread
From: Eric Blake @ 2020-03-18 12:55 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Daniel P. Berrange, John Snow, QEMU Developers

On 3/17/20 1:11 PM, Peter Maydell wrote:
> On Tue, 17 Mar 2020 at 17:55, Eric Blake <eblake@redhat.com> wrote:
>>
>> glib's G_DEFINE_AUTOPTR_CLEANUP_FUNC() macro defines several static
>> inline functions, often with some of them unused, but prior to 2.57.2
>> did not mark the functions as such.  As a result, clang (but not gcc)
>> fails to build with older glib unless -Wno-unused-function is enabled.
>>
>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> Half-tested: I proved to myself that this does NOT enable
>> -Wno-unused-function on my setup of glib 2.62.5 and gcc 9.2.1 (Fedora
>> 31), but would do so if I introduced an intentional compile error into
>> the sample program; but Iwas unable to test that it would prevent the
>> build failure encountered by Peter on John's pull request (older glib
>> but exact version unknown, clang, on NetBSD).
> 
> This wasn't a NetBSD failure. I hit it on my clang-on-x86-64-Ubuntu
> setup, and also on FreeBSD. (The latter is just the tests/vm
> FreeBSD config, so you can repro that if you need to.)

Not sure where I got NetBSD from (maybe because the build failure 
happened in a file with 'nbd' in the name and I gravitated to the 'n'?). 
  But now that I've re-read your replies to the pull request, I'm glad 
to state that my mistake on reproduction platform is confined to the 
part after the ---; the commit message itself is accurate as-is.

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] build: Silence clang warning on older glib autoptr usage
  2020-03-18 12:55   ` Eric Blake
@ 2020-03-18 17:18     ` John Snow
  0 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2020-03-18 17:18 UTC (permalink / raw)
  To: Eric Blake, Peter Maydell; +Cc: Daniel P. Berrange, QEMU Developers



On 3/18/20 8:55 AM, Eric Blake wrote:
> On 3/17/20 1:11 PM, Peter Maydell wrote:
>> On Tue, 17 Mar 2020 at 17:55, Eric Blake <eblake@redhat.com> wrote:
>>>
>>> glib's G_DEFINE_AUTOPTR_CLEANUP_FUNC() macro defines several static
>>> inline functions, often with some of them unused, but prior to 2.57.2
>>> did not mark the functions as such.  As a result, clang (but not gcc)
>>> fails to build with older glib unless -Wno-unused-function is enabled.
>>>
>>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>
>>> Half-tested: I proved to myself that this does NOT enable
>>> -Wno-unused-function on my setup of glib 2.62.5 and gcc 9.2.1 (Fedora
>>> 31), but would do so if I introduced an intentional compile error into
>>> the sample program; but Iwas unable to test that it would prevent the
>>> build failure encountered by Peter on John's pull request (older glib
>>> but exact version unknown, clang, on NetBSD).
>>
>> This wasn't a NetBSD failure. I hit it on my clang-on-x86-64-Ubuntu
>> setup, and also on FreeBSD. (The latter is just the tests/vm
>> FreeBSD config, so you can repro that if you need to.)
> 
> Not sure where I got NetBSD from (maybe because the build failure
> happened in a file with 'nbd' in the name and I gravitated to the 'n'?).
>  But now that I've re-read your replies to the pull request, I'm glad to
> state that my mistake on reproduction platform is confined to the part
> after the ---; the commit message itself is accurate as-is.
> 

OK, thanks -- I'm going to take this patch and re-send the PR.



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-03-18 17:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-17 17:55 [PATCH] build: Silence clang warning on older glib autoptr usage Eric Blake
2020-03-17 18:01 ` John Snow
2020-03-17 18:11 ` Peter Maydell
2020-03-18 11:19   ` Eric Blake
2020-03-18 12:45     ` Eric Blake
2020-03-18 12:55   ` Eric Blake
2020-03-18 17:18     ` John Snow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).