[libvirt PATCH] Set -Wno-incompatible-pointer-types-discards-qualifiers

Daniel P. Berrangé posted 1 patch 3 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201112094139.415962-1-berrange@redhat.com
meson.build | 3 +++
1 file changed, 3 insertions(+)
[libvirt PATCH] Set -Wno-incompatible-pointer-types-discards-qualifiers
Posted by Daniel P. Berrangé 3 years, 4 months ago
With CLang we're getting failures from cats in GLib macros

../dist-unpack/libvirt-6.10.0/src/util/vireventthread.c:35:1:
  error: passing 'typeof (*(&g_define_type_id__volatile)) *'
  (aka 'volatile unsigned long *') to parameter of type
  'gsize *' (aka 'unsigned long *') discards qualifiers
  [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
G_DEFINE_TYPE(virEventThread, vir_event_thread, G_TYPE_OBJECT)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The cast is valid and there's no way for libvirt to workaround
the issue, so we must disable this CLang warning flag.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 meson.build | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meson.build b/meson.build
index f2e4f2cc23..e4b7fa0728 100644
--- a/meson.build
+++ b/meson.build
@@ -405,6 +405,9 @@ cc_flags += [
   # so use this CLang-specific arg to keep it quiet
   '-Wno-typedef-redefinition',
 
+  # CLang complains about casts in G_DEFINE_TYPE(...)
+  '-Wno-incompatible-pointer-types-discards-qualifiers',
+
   # We don't use -Wc++-compat so we have to enable it explicitly
   '-Wjump-misses-init',
 
-- 
2.25.4

Re: [libvirt PATCH] Set -Wno-incompatible-pointer-types-discards-qualifiers
Posted by Ján Tomko 3 years, 4 months ago
On a Thursday in 2020, Daniel P. Berrangé wrote:
>With CLang we're getting failures from cats in GLib macros

s/cats/casts/ :)

>
>../dist-unpack/libvirt-6.10.0/src/util/vireventthread.c:35:1:
>  error: passing 'typeof (*(&g_define_type_id__volatile)) *'
>  (aka 'volatile unsigned long *') to parameter of type
>  'gsize *' (aka 'unsigned long *') discards qualifiers
>  [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
>G_DEFINE_TYPE(virEventThread, vir_event_thread, G_TYPE_OBJECT)
>^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>The cast is valid and there's no way for libvirt to workaround
>the issue, so we must disable this CLang warning flag.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> meson.build | 3 +++
> 1 file changed, 3 insertions(+)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [libvirt PATCH] Set -Wno-incompatible-pointer-types-discards-qualifiers
Posted by Martin Kletzander 3 years, 4 months ago
On Thu, Nov 12, 2020 at 09:41:39AM +0000, Daniel P. Berrangé wrote:
>With CLang we're getting failures from cats in GLib macros
>
>../dist-unpack/libvirt-6.10.0/src/util/vireventthread.c:35:1:
>  error: passing 'typeof (*(&g_define_type_id__volatile)) *'
>  (aka 'volatile unsigned long *') to parameter of type
>  'gsize *' (aka 'unsigned long *') discards qualifiers
>  [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
>G_DEFINE_TYPE(virEventThread, vir_event_thread, G_TYPE_OBJECT)
>^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>The cast is valid and there's no way for libvirt to workaround
>the issue, so we must disable this CLang warning flag.
>

Can't we have the warning disabled (maybe by force casting it before the macro
or in a macro wrapper) just on those lines?  Because otherwise we are silencing
whole lot of other warnings that are actually very good to have, like discarding
const qualifiers etc. in the rest of the code AFAIUI.

>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> meson.build | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/meson.build b/meson.build
>index f2e4f2cc23..e4b7fa0728 100644
>--- a/meson.build
>+++ b/meson.build
>@@ -405,6 +405,9 @@ cc_flags += [
>   # so use this CLang-specific arg to keep it quiet
>   '-Wno-typedef-redefinition',
>
>+  # CLang complains about casts in G_DEFINE_TYPE(...)
>+  '-Wno-incompatible-pointer-types-discards-qualifiers',
>+
>   # We don't use -Wc++-compat so we have to enable it explicitly
>   '-Wjump-misses-init',
>
>-- 
>2.25.4
>
Re: [libvirt PATCH] Set -Wno-incompatible-pointer-types-discards-qualifiers
Posted by Daniel P. Berrangé 3 years, 4 months ago
On Thu, Nov 12, 2020 at 01:42:32PM +0100, Martin Kletzander wrote:
> On Thu, Nov 12, 2020 at 09:41:39AM +0000, Daniel P. Berrangé wrote:
> > With CLang we're getting failures from cats in GLib macros
> > 
> > ../dist-unpack/libvirt-6.10.0/src/util/vireventthread.c:35:1:
> >  error: passing 'typeof (*(&g_define_type_id__volatile)) *'
> >  (aka 'volatile unsigned long *') to parameter of type
> >  'gsize *' (aka 'unsigned long *') discards qualifiers
> >  [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> > G_DEFINE_TYPE(virEventThread, vir_event_thread, G_TYPE_OBJECT)
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > The cast is valid and there's no way for libvirt to workaround
> > the issue, so we must disable this CLang warning flag.
> > 
> 
> Can't we have the warning disabled (maybe by force casting it before the macro
> or in a macro wrapper) just on those lines?  Because otherwise we are silencing
> whole lot of other warnings that are actually very good to have, like discarding
> const qualifiers etc. in the rest of the code AFAIUI.

AFAIK, this warning flag is a new thing in latest clang in rawhide,
so all the existing stuff we've relied on long term is still active.

Doing this selectively wold be quite tedious because it isn't just
one file long term. Once we've converted over all the current virObject
classes, we'll have many many cases.

> 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > meson.build | 3 +++
> > 1 file changed, 3 insertions(+)
> > 
> > diff --git a/meson.build b/meson.build
> > index f2e4f2cc23..e4b7fa0728 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -405,6 +405,9 @@ cc_flags += [
> >   # so use this CLang-specific arg to keep it quiet
> >   '-Wno-typedef-redefinition',
> > 
> > +  # CLang complains about casts in G_DEFINE_TYPE(...)
> > +  '-Wno-incompatible-pointer-types-discards-qualifiers',
> > +
> >   # We don't use -Wc++-compat so we have to enable it explicitly
> >   '-Wjump-misses-init',
> > 
> > -- 
> > 2.25.4
> > 



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH] Set -Wno-incompatible-pointer-types-discards-qualifiers
Posted by Martin Kletzander 3 years, 4 months ago
On Thu, Nov 12, 2020 at 12:48:21PM +0000, Daniel P. Berrangé wrote:
>On Thu, Nov 12, 2020 at 01:42:32PM +0100, Martin Kletzander wrote:
>> On Thu, Nov 12, 2020 at 09:41:39AM +0000, Daniel P. Berrangé wrote:
>> > With CLang we're getting failures from cats in GLib macros
>> >
>> > ../dist-unpack/libvirt-6.10.0/src/util/vireventthread.c:35:1:
>> >  error: passing 'typeof (*(&g_define_type_id__volatile)) *'
>> >  (aka 'volatile unsigned long *') to parameter of type
>> >  'gsize *' (aka 'unsigned long *') discards qualifiers
>> >  [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
>> > G_DEFINE_TYPE(virEventThread, vir_event_thread, G_TYPE_OBJECT)
>> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >
>> > The cast is valid and there's no way for libvirt to workaround
>> > the issue, so we must disable this CLang warning flag.
>> >
>>
>> Can't we have the warning disabled (maybe by force casting it before the macro
>> or in a macro wrapper) just on those lines?  Because otherwise we are silencing
>> whole lot of other warnings that are actually very good to have, like discarding
>> const qualifiers etc. in the rest of the code AFAIUI.
>
>AFAIK, this warning flag is a new thing in latest clang in rawhide,
>so all the existing stuff we've relied on long term is still active.
>

I'm quite sure this flag exists for a long time already.  Maybe gobject changed
something lately?

Also what clang version does rawhide have?  With clang 11.0 and current master
I'm not getting any such warning and 12.0 is still in progress.  Maybe rawhide
is shipping some buggy pre-release again?

>Doing this selectively wold be quite tedious because it isn't just
>one file long term. Once we've converted over all the current virObject
>classes, we'll have many many cases.
>

I'm quite sure this is very severe issue that needs to be fixed either in
gobject or clang as otherwise this is a valid code that compiles without a notice:

const char *
meh(char **input)
{
     char *ret = ((const char *const *)input)[0];

     return ret;
}

int
main(int argc,
      char **argv)
{
     char *ptr = meh((const char *const *) argv);

     if (ptr[0] != '.')
         return 1;

     return 0;
}

And since we are at least trying to keep some C const correctness this warning
might actually help us catch some of common problems.  Like, for example,
imagine this stupid code:


void
meh(const char *tmp,
     void *opaque)
{
     char *data = opaque;
     data[0] = tmp[0];
}

int
main(int argc,
      char **argv)
{
     const char *cnst = "asdf";
     meh(cnst, argv[0]);

     return 0;
}

It works as it should and if you change the order of the arguments to the "meh"
function by mistake, then the compiler will shout at you.  Unless you use
-Wno-incompatible-pointer-types-discards-qualifiers in which case it will
compile, run, and segfault (if your system is sane).

G_DEFINE_TYPE is currently used 4 times in our code.  One extra cast or a
wrapper macro (for the time being) is not something that tedious, especially
since this needs to be dealt with in either glib or clang.

Maybe the workaround is not as easy as I imagine, but I can't really try it
since I'm not getting this error.  Since most of these are solvable by
force-casting it I don't think it's going to be that bad.

I just do not like disabling warnings for no good reason.  Please give the
temporary workaround some thought, thank you.

>>
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> > ---
>> > meson.build | 3 +++
>> > 1 file changed, 3 insertions(+)
>> >
>> > diff --git a/meson.build b/meson.build
>> > index f2e4f2cc23..e4b7fa0728 100644
>> > --- a/meson.build
>> > +++ b/meson.build
>> > @@ -405,6 +405,9 @@ cc_flags += [
>> >   # so use this CLang-specific arg to keep it quiet
>> >   '-Wno-typedef-redefinition',
>> >
>> > +  # CLang complains about casts in G_DEFINE_TYPE(...)
>> > +  '-Wno-incompatible-pointer-types-discards-qualifiers',
>> > +
>> >   # We don't use -Wc++-compat so we have to enable it explicitly
>> >   '-Wjump-misses-init',
>> >
>> > --
>> > 2.25.4
>> >
>
>
>
>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [libvirt PATCH] Set -Wno-incompatible-pointer-types-discards-qualifiers
Posted by Andrea Bolognani 3 years, 4 months ago
On Thu, 2020-11-12 at 09:41 +0000, Daniel P. Berrangé wrote:
> With CLang we're getting failures from cats in GLib macros

s/CLang/Clang/

> ../dist-unpack/libvirt-6.10.0/src/util/vireventthread.c:35:1:
>   error: passing 'typeof (*(&g_define_type_id__volatile)) *'
>   (aka 'volatile unsigned long *') to parameter of type
>   'gsize *' (aka 'unsigned long *') discards qualifiers
>   [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> G_DEFINE_TYPE(virEventThread, vir_event_thread, G_TYPE_OBJECT)
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> The cast is valid and there's no way for libvirt to workaround
> the issue, so we must disable this CLang warning flag.

Same here.

> +++ b/meson.build
> @@ -405,6 +405,9 @@ cc_flags += [
>    # so use this CLang-specific arg to keep it quiet
>    '-Wno-typedef-redefinition',
>  
> +  # CLang complains about casts in G_DEFINE_TYPE(...)
> +  '-Wno-incompatible-pointer-types-discards-qualifiers',

And here.

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

Was the issue reported against GLib?

-- 
Andrea Bolognani / Red Hat / Virtualization