Обсуждение: [HACKERS] "left shift of negative value" warnings
Hi, For a while I've been getting warnings like /home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c: In function ‘inet_cidr_ntop_ipv6’: /home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c:205:11: warning: left shift of negative value [-Wshift-negative-value] m = ~0 << (8 - b); ^~ /home/andres/src/postgresql/src/backend/utils/adt/network.c: In function ‘inetmi’: /home/andres/src/postgresql/src/backend/utils/adt/network.c:1482:24: warning: left shift of negative value [-Wshift-negative-value] res |= ((int64) -1) << (byte * 8); ^~ /home/andres/src/postgresql/src/backend/utils/adt/varbit.c: In function ‘bitfromint4’: /home/andres/src/postgresql/src/backend/utils/adt/varbit.c:1546:16: warning: left shift of negative value [-Wshift-negative-value] val |= (-1) << (srcbitsleft + 8 - destbitsleft); ^~ /home/andres/src/postgresql/src/backend/utils/adt/varbit.c: In function ‘bitfromint8’: /home/andres/src/postgresql/src/backend/utils/adt/varbit.c:1626:16: warning: left shift of negative value [-Wshift-negative-value] val |= (-1) << (srcbitsleft + 8 - destbitsleft); If I understand C99 correctly, the behaviour of a left-shift of a negative number is undefined (6.5.7 4.). In C89 the spec was very unclear about that. Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes: > For a while I've been getting warnings like > /home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c: In function ‘inet_cidr_ntop_ipv6’: > /home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c:205:11: warning: left shift of negative value [-Wshift-negative-value] > m = ~0 << (8 - b); > ^~ I imagine forcing the LHS to unsigned would silence that, though you'd have to be careful that the sign extension (widening) happened before you changed the value to unsigned, in the int64 cases. It's a bit odd though that it seems to think ~0 is signed. > If I understand C99 correctly, the behaviour of a left-shift of a > negative number is undefined (6.5.7 4.). As I read that, it's only "undefined" if overflow would occur (ie the sign bit would change). Your compiler is being a useless annoying nanny, but that seems to be the in thing for compiler authors these days. regards, tom lane
On 2017-04-09 19:20:27 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > For a while I've been getting warnings like > > /home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c: In function ‘inet_cidr_ntop_ipv6’: > > /home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c:205:11: warning: left shift of negative value [-Wshift-negative-value] > > m = ~0 << (8 - b); > > ^~ > > I imagine forcing the LHS to unsigned would silence that, though you'd > have to be careful that the sign extension (widening) happened before > you changed the value to unsigned, in the int64 cases. It's a bit odd > though that it seems to think ~0 is signed. Hm, why's that odd? By default integers are signed, and ~ doesn't change signedness? > > If I understand C99 correctly, the behaviour of a left-shift of a > > negative number is undefined (6.5.7 4.). > > As I read that, it's only "undefined" if overflow would occur (ie > the sign bit would change). Your compiler is being a useless annoying > nanny, but that seems to be the in thing for compiler authors these > days. "The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are filled with zeros. If E1 has an unsigned type, the value of the result is E1 × 2 E2 , reduced modulo one more than the maximum value representable in the result type. If E1 has a signed type and nonnegative value, and E1 × 2 E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined." As I read this it's defined iff E1 is signed, nonnegative *and* the the result of the shift is representable in the relevant type. That seems, uh, a bit restrictive, but that seems to be the only reading? Note that the otherwise is preceded by a semicolon... - Andres
Andres Freund <andres@anarazel.de> writes: > On 2017-04-09 19:20:27 -0400, Tom Lane wrote: >> As I read that, it's only "undefined" if overflow would occur (ie >> the sign bit would change). Your compiler is being a useless annoying >> nanny, but that seems to be the in thing for compiler authors these >> days. > "The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are filled with > zeros. If E1 has an unsigned type, the value of the result is E1 × 2 E2 , reduced modulo > one more than the maximum value representable in the result type. If E1 has a signed > type and nonnegative value, and E1 × 2 E2 is representable in the result type, then that is > the resulting value; otherwise, the behavior is undefined." > As I read this it's defined iff E1 is signed, nonnegative *and* the the > result of the shift is representable in the relevant type. That seems, > uh, a bit restrictive, but that seems to be the only reading? Oh --- I misread the "nonnegative" as applying to the shift count, but you're right, it's talking about the LHS. That's weird --- the E1 × 2^E2 definition works fine as long as there's no overflow, so why didn't they define it like that? It seems just arbitrarily broken this way. regards, tom lane
On 2017-04-10 15:25:57 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2017-04-09 19:20:27 -0400, Tom Lane wrote: > >> As I read that, it's only "undefined" if overflow would occur (ie > >> the sign bit would change). Your compiler is being a useless annoying > >> nanny, but that seems to be the in thing for compiler authors these > >> days. > > > "The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are filled with > > zeros. If E1 has an unsigned type, the value of the result is E1 × 2 E2 , reduced modulo > > one more than the maximum value representable in the result type. If E1 has a signed > > type and nonnegative value, and E1 × 2 E2 is representable in the result type, then that is > > the resulting value; otherwise, the behavior is undefined." > > > As I read this it's defined iff E1 is signed, nonnegative *and* the the > > result of the shift is representable in the relevant type. That seems, > > uh, a bit restrictive, but that seems to be the only reading? > > Oh --- I misread the "nonnegative" as applying to the shift count, but > you're right, it's talking about the LHS. That's weird --- the E1 × 2^E2 > definition works fine as long as there's no overflow, so why didn't they > define it like that? It seems just arbitrarily broken this way. I guess the motivation is that it's not entirely clear what happens with the sign bit, when shifting. Why they made that UB instead of implementation defined, is a complete mystery to me, however. We should do *something* about this? The warnings are a bit annoying :( - Andres
On 10/04/17 22:19, Andres Freund wrote: > I guess the motivation is that it's not entirely clear what happens with > the sign bit, when shifting. Indeed, certain one's complement CPUs even "outlived" C99 by a small margin, as it were: http://mainframe.typepad.com/blog/2012/10/sad-day-unisys-abandons-its-mainframe-processors.html And the respective ABI will be around for some time to come, apparently: https://www.theregister.co.uk/2016/03/31/free_x86_mainframes_for_all_virtual_mainframes_that_is :-) -- Markus Nullmeier http://www.g-vo.org German Astrophysical Virtual Observatory (GAVO)