Is there a GCC warning that detects bit shift operations on signed types?
Asked Answered
A

2

13

If I read the C++ ISO specification (sections 5.8.2 and 5.8.3) right, the right-shift of negative signed types is implementation specific and the left-shift undefined behaviour.

Therefore I would like to find shift operations on signed types in our legacy source code which we compile with g++ 4.8.2.

Unfortunately, I couldn't find such an option in the manual. I can for example compile this code with "g++ -Wall -Wextra -pedantic" without a warning:

int si    = -1;
int left  = si << 1; // -2 (multiplication by 2, sign is preserved)
int right = si >> 1; // -1 (no change, only 1s)

Can anyone tell me if there is such a warning and, if not, why gcc doesn't care about it?

Ally answered 31/7, 2014 at 9:0 Comment(0)
J
6

AFAIK gcc doesn't provide such an option. The standard, as you cited, says:

N3690 - §5.8.3

The value of E1 >> E2 is E1 right-shifted E2 bit positions. If E1 has an unsigned type or if E1 has a signed type and a non-negative value, the value of the result is the integral part of the quotient of E1/2E2. If E1 has a signed type and a negative value, the resulting value is implementation-defined.

and that means doing

int si    = -1;
int right = si >> 1;

might or might not yield -1 as a result. It's implementation defined. And that means the compiler is not forced to yield a warning like "other compilers might do this another way".

Some of the reasons for this choice follow.

The original K&R passage says:

"Right shifting an unsigned quantity, fills vacated bits with 0. Right shifting a signed quantity will fill with sign bits (arithmetic shift) on some machines such as the PDP-11, and with 0 bits (logical shift) on others."

this means that the operation is architecture-dependent. The reason behind this is that some architectures are fast in doing one of the two but not both.

This reason plus the fact that the usefulness of sign-extended shifts is marginal made the standard take the choice of leaving it "implementation defined". By "usefulness of sign-extended shifts" I mean that right shifting a negative signed integer arithmetically does not work as the positive counterpart (because losing a 1 on the right makes the negative number smaller, i.e. bigger in modulus)

+63 >> 1 = +31 (integral part of quotient E1/2E2)
00111111 >> 1 = 00011111
-63 >> 1 = -32 
11000001 >> 1 = 11100000

References for further reading:

https://mcmap.net/q/378950/-right-shifting-negative-numbers-in-c

http://www.ccsinfo.com/forum/viewtopic.php?t=45711

https://isocpp.org/std/the-standard


Edit: if the above doesn't address the issue (i.e. the code is valid, why should a compiler warn about that?) I'm providing a second solution: AST matchers

As described here: http://eli.thegreenplace.net/2014/07/29/ast-matchers-and-clang-refactoring-tools/ you can write some code to identify quickly all the right-shifts-with-signed-integers spots in your program.

Think of it as "writing my small one-task static analysis checker".


Edit 2: you might also try other static-analysis tools, clang has a -fsanitize=shift option which could work for you. AFAIK also for gcc they were implementing an undefined behavior sanitizer which could help diagnosing those errors. I've not been following the story but I guess you could give it a try as well.

Jigger answered 31/7, 2014 at 9:50 Comment(11)
Awesome, you know a lot about the standards but are you also attempting to answer that question up there?Defense
There's a lot of useful information on right shifts in this answer, but nothing I see that's actually relevant to the OP's question. It'd make a good answer to a different question, but that's all.Thereat
@hvd gcc doesn't care because it's not an error. Implementation defined means that every compiler can implement it as it wants. The reasons on why this is implementation defined are above. I don't understand what you expect exactly.Jigger
@MarcoA. Compilers warn about perfectly valid code all the time, by design, so the fact that it's perfectly valid doesn't address the question of why the compiler doesn't warn. It does address the question of why the compiler doesn't give an error message, but that's not what the OP asked.Thereat
the fact that it's perfectly valid doesn't address the question of why the compiler doesn't warn .. I.. simply don't understand this, I'm sorry : |Jigger
@MarcoA. Consider int main() { }. This is perfectly valid C. Yet compilers can and do warn about this: GCC optionally warns about an "old-style function definition". Warnings are issued for plenty of perfectly valid yet according to the compiler authors questionable practises. A right shift on a signed integer type could well be an example of that.Thereat
@MarcoA. But the link in your edit does look like a good and useful approach. It still doesn't exactly match what the OP asked, but it does help a great deal in what the OP actually wants to accomplish, do I've removed my downvote.Thereat
@MarcoA. consider -Wconversion: calling <cstdlib>'s std::abs(my_double) is perfectly legal but contains an implicit conversion that may not have been intended by the programmer (they probably wanted <cmath>'s std::abs(double)... so GCC's -Wconversion is available to warn about this. Similarly, -63 >> 1 is legal but - with known negative values and (to a lesser extent) signed values may be accidental and/or dangerous in terms of breaking program logic, and worth the programmer revisiting on a case by case basis.Gaskell
I agree that they might issue warnings, especially for legacy code, but I believe that unless explicitly stated in the standard that is up to the compiler implementation. I don't know of any option to do that (I edited the question to specify it) and also provided the reasons why this isn't enforced in the standard. I agree that gcc could, but it's not forced so.. "why should it?" appears to be a valid reply tooJigger
@MarcoA. Thank you for your detailed answer. I will try to clang option and report back if it worked. Otherwise I can still try some static code analyzers but if possible I prefer compiler warnings.Ally
-Wshift-overflow=2Psi
S
2

The GCC documentation is here.

The answer appears to be "no", there's not any warning option for what you want.

I don't know why, but I'd guess that there's just so much code out there that uses this that it would be too noisy. C may not define arithmetic shifts, but pretty much all CPUs do it the same way, these days, so most people assume it is defined that way.

Scholasticism answered 31/7, 2014 at 10:0 Comment(2)
The warning would not have to be enabled by default. But it would be useful to detect portability issues. Ditto for complement on a signed type.Implicit
-Wshift-overflow=2Psi

© 2022 - 2024 — McMap. All rights reserved.