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@xxxxxxxxxxxxxxxxxxxxxxx http://lists.fedoraproject.org/admin/lists/devel@xxxxxxxxxxxxxxxxxxxxxxx
