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