On Sat, 2016-04-23 at 06:42 -0700, John Reiser wrote:
> > 
> > GCC 6 does not like this:
> > 
> > #define VMW_BIT_MASK(shift) (((1 << (shift - 1)) << 1) - 1)
> > 
> > it produces errors:
> > 
> > /builddir/build/BUILD/open-vm-tools-10.0.0-
> > 3000743/lib/include/x86cpuid.h:912:51: error: result of '-
> > 2147483648 << 1' requires 33 bits to represent, but 'int' only has
> > 32 bits [-Werror=shift-overflow=]  #define
> > VMW_BIT_MASK(shift)  (((1 << (shift - 1)) << 1) - 1)

> #define VMW_BIT_MASK(shift) (int)~((~0u << (shift - 1)) << 1)
> 
> Both shifts are shifts of an 'unsigned int', not anything wider.
> Depending on the compiler, this may save time and/or a register
> when 'shift' is not a compile-time constant.  The final cast
> to (int) is only for the purpose of matching the type of the
> original expression.
> Of course 'shift' must be positive [especially not zero]
> and not more than the bit width of an 'int'.

So yes, this turns out to be a problem, unfortunately! This does solve
the initial error, but shift sometimes *is* zero, so indeed it breaks.
rwmjones tried a compile with the original VMW_BIT_MASK definition but
without -Werror: when you do that, you get errors like this:

In file included from 
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/hostinfo.h:35:0,
                 from dndGuest/rpcV3Util.cpp:45:
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:912:51:
 warning: result of '(-2147483648 << 1)' requires 33 bits to represent, but 
'int' only has 32 bits [-Wshift-overflow=]
 #define VMW_BIT_MASK(shift)  (((1 << (shift - 1)) << 1) - 1)
                                ~~~~~~~~~~~~~~~~~~~^~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:916:34:
 note: in expansion of macro 'VMW_BIT_MASK'
    CPUID_##name##_MASK         = VMW_BIT_MASK(size) << bitpos, \
                                  ^~~~~~~~~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:292:1:
 note: in expansion of macro 'FIELD'
 FIELD(  0,  0, EAX,  0, 32, NUMLEVELS,                             ANY, FALSE) 
\
 ^~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:870:4:
 note: in expansion of macro 'CPUID_FIELD_DATA_LEVEL_0'
    CPUID_FIELD_DATA_LEVEL_0                                           \
    ^~~~~~~~~~~~~~~~~~~~~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:927:4:
 note: in expansion of macro 'CPUID_FIELD_DATA'
    CPUID_FIELD_DATA
    ^~~~~~~~~~~~~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:912:51:
 error: left operand of shift expression '(-2147483648 << 1)' is negative 
[-fpermissive]
 #define VMW_BIT_MASK(shift)  (((1 << (shift - 1)) << 1) - 1)
                               ~~~~~~~~~~~~~~~~~~~~^~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:916:34:
 note: in expansion of macro 'VMW_BIT_MASK'
    CPUID_##name##_MASK         = VMW_BIT_MASK(size) << bitpos, \
                                  ^~~~~~~~~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:292:1:
 note: in expansion of macro 'FIELD'
 FIELD(  0,  0, EAX,  0, 32, NUMLEVELS,                             ANY, FALSE) 
\
 ^~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:870:4:
 note: in expansion of macro 'CPUID_FIELD_DATA_LEVEL_0'
    CPUID_FIELD_DATA_LEVEL_0                                           \
    ^~~~~~~~~~~~~~~~~~~~~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:927:4:
 note: in expansion of macro 'CPUID_FIELD_DATA'
    CPUID_FIELD_DATA
    ^~~~~~~~~~~~~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:292:22:
 error: enumerator value for 'CPUID_NUMLEVELS_MASK' is not an integer constant
 FIELD(  0,  0, EAX,  0, 32, NUMLEVELS,                             ANY, FALSE) 
\
                      ^

I tried a compile with John's definition; with that version you get
errors like this:

In file included from 
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/hostinfo.h:35:0,
                 from dndGuest/rpcV3Util.cpp:45:
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:916:53:
 error: left operand of shift expression '(-1 << 0)' is negative [-fpermissive]
    CPUID_##name##_MASK         = VMW_BIT_MASK(size) << bitpos, \
                                                     ^
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:292:1:
 note: in expansion of macro 'FIELD'
 FIELD(  0,  0, EAX,  0, 32, NUMLEVELS,                             ANY, FALSE) 
\
 ^~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:870:4:
 note: in expansion of macro 'CPUID_FIELD_DATA_LEVEL_0'
    CPUID_FIELD_DATA_LEVEL_0                                           \
    ^~~~~~~~~~~~~~~~~~~~~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:927:4:
 note: in expansion of macro 'CPUID_FIELD_DATA'
    CPUID_FIELD_DATA
    ^~~~~~~~~~~~~~~~
/builddir/build/BUILD/open-vm-tools-10.0.0-3000743/lib/include/x86cpuid.h:292:22:
 error: enumerator value for 'CPUID_NUMLEVELS_MASK' is not an integer constant
 FIELD(  0,  0, EAX,  0, 32, NUMLEVELS,                             ANY, FALSE) 
\
                      ^

I'm gonna try KK's version; if that doesn't work either I'm kinda
inclined to just leave it to upstream and we can drop open-vm-tools
from comps / spin-kickstarts, or something.
-- 
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | identi.ca: adamwfedora
http://www.happyassassin.net

--
devel mailing list
devel@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/devel@lists.fedoraproject.org

Reply via email to