GNOME Bugzilla – Bug 767882
Bit shift overflow (-Wshift-overflow) warning in gparam.h
Last modified: 2016-10-30 14:42:45 UTC
When building 32-bit Wine with gstreamer, I get this warning with gcc 6.1.1 20160602: gcc -m32 -c -o gstdemux.o gstdemux.c -I. -I../../include -I/usr/include/gstreamer-1.0 \ -I/usr/lib32/gstreamer-1.0/include -I/usr/include/glib-2.0 -I/usr/lib32/glib-2.0/include \ -D__WINESRC__ -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing -Wdeclaration-after-statement \ -Wempty-body -Wignored-qualifiers -Wshift-overflow=2 -Wstrict-prototypes -Wtype-limits \ -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith -Wlogical-op -gdwarf-2 \ -gstrict-dwarf -fno-omit-frame-pointer -g -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0 In file included from /usr/include/glib-2.0/gobject/gobject.h:26:0, from /usr/include/glib-2.0/gobject/gbinding.h:29, from /usr/include/glib-2.0/glib-object.h:23, from /usr/include/gstreamer-1.0/gst/gstenumtypes.h:7, from /usr/include/gstreamer-1.0/gst/gst.h:31, from gstdemux.c:24: /usr/include/glib-2.0/gobject/gparam.h:166:26: warning: result of ‘1 << 31’ requires 33 bits to represent, but ‘int’ only has 32 bits [-Wshift-overflow=] G_PARAM_DEPRECATED = 1 << 31 Enums are defined to be "int" so putting this bit there hits the sign flag, which gcc doesn't like. Maybe it should use some method other than bit-shift to set that bit. Even just disabling the warning would probably be fine.
See also Bug 767883 in gstreamer.
does changing the 1 << 31 to 1u << 31 make the warning go away?
(In reply to Ray Strode [halfline] from comment #2) > does changing the 1 << 31 to 1u << 31 make the warning go away? Yes, that fixes it.
Created attachment 331877 [details] [review] in glib2 remove gcc6 warning from G_PARAM_DEPRECATED [-Wpedantic] I propose a refined solution since 1u << 31 still causes a warning if compiled with -Wpedantic (other applications than gstreamer use also -Wpedantic): glib-2.0/gobject/gparam.h:166:33: warning: ISO C restricts enumerator values to range of 'int' [-Wpedantic] G_PARAM_DEPRECATED = 1u << 31 My proposed fix is as follows (refer to attached patch): G_PARAM_DEPRECATED = (gint)(1u << 31)
Could we expect the fix to be committed sometime soon? Would be great to get this into stable as well.
Review of attachment 331877 [details] [review]: The commit message is inadequate. You will need to add a link to this bug report, so that we keep track of the discussion. ::: gobject/gparam.h @@ +164,3 @@ /* User defined flags go here */ G_PARAM_EXPLICIT_NOTIFY = 1 << 30, + G_PARAM_DEPRECATED = (gint)(1u << 31) I'd probably add a comment above this line; something like: /* Avoid a warning with -Wpedantic */
Created attachment 337885 [details] [review] reworked patch according to review comments
Review of attachment 337885 [details] [review]: Looks good, now. Thanks.
*** Bug 761119 has been marked as a duplicate of this bug. ***
Probably the compiler requirements page should be updated: https://wiki.gnome.org/Projects/GLib/CompilerRequirements if this is no longer a requirement, right?
(In reply to Michael Catanzaro from comment #10) > Probably the compiler requirements page should be updated: > > https://wiki.gnome.org/Projects/GLib/CompilerRequirements > > if this is no longer a requirement, right? Since enumeration behaviour is still undefined in the ISO C specification, no: that requirement should still stand. The cast is there just to silence a pedantic warning: we're still expecting the compiler to ensure that an enumeration size gets updated depending on the biggest value defined inside it.
(In reply to Emmanuele Bassi (:ebassi) from comment #11) > > Since enumeration behaviour is still undefined in the ISO C specification, > no: that requirement should still stand. It's not undefined. The vintage C90 just says > Each enumerated type shall be compatible with an integer type. The choice of type is implementation-defined. Implementation-defined is far, far better than "undefined". And if you could move up to C99 - which I'd strongly recommend - then in fact, exactly what you want seems to be guaranteed. quotes via http://stackoverflow.com/a/1113869/2757035 C99, 6.7.2.2p4: > Each enumerated type shall be compatible with char, a signed integer type, or > an unsigned integer type. The choice of type is implementation-defined,108) > but shall be capable of representing the values of all the members of the > enumeration. [...] Footnote 108 adds: > An implementation may delay the choice of which integer type until all > enumeration constants have been seen. So, a C99 compliant compiler is required by the Standard to allocate sufficient storage for the distinct enumeration values you specify.