undefined behavior in simplecpp::TokenList::constFoldShift?

[Hexcoder]

Hello,

according to
https://www.viva64.com/en/w/v610/
the shift operators in C and C++ have some undefined (and unspecified) behavior:

1. if the right operand is negative or greater than or equal to the length in bits of the promoted left operand.
2. for '<<' if the left operand is of signed type but positive and the result of left * 2 ^ right does not fit into the type of the left operand.
3. for '>>' if the left operand is negative (unspecified behavior).

int A, B, C, D, E, F, G;
A = 0U << -1;
B = 0U >> -1;
C = 1U << (sizeof(int) * CHAR_BIT);
D = 1U >> (sizeof(int) * CHAR_BIT);
E = 1 << (sizeof(int) * CHAR_BIT - 1);
F = -1 << 0;
G = -1 >> 0;

Then I thought: does this apply for the preprocessor as well?
I could not find any clarification, so I assumed the preprocessor
acts like the compiler proper.
When these expressions are used in conditions, it is not clear,
what the result is.

Can anybody clarify?

Currently simplecpp does nothing special
and evaluates the operands unfiltered.

// see https://www.viva64.com/en/w/v610/
// undefined and unspecified behavior with shift operators
#include <limits.h>

// undefined behavior
#if (0U << -1) == 0
int a = 0U << -1;
#endif

#if (0U >> -1) == 0
int b = 0U >> -1;
#endif

#if (1U << (4 * CHAR_BIT)) > 0
int c = 1U << (sizeof(int) * CHAR_BIT);
#endif

#if (1U >> (4 * CHAR_BIT)) > 0
int d = 1U >> (sizeof(int) * CHAR_BIT);
#endif

#if (1 << (4 * CHAR_BIT)) > 0
int e = 1 << (sizeof(int) * CHAR_BIT);
#endif

#if (-1 << 0) < 0
int f = -1 << 0;
#endif

// unspecified behavior
#if (-1 >> 0) < 0
int g = -1 >> 0;
#endif

gcc-7 -W -Wall -Wextra -pedantic gives:

UB_shift.cpp:7:16: warning: left shift count is negative [-Wshift-count-negative]
 int a = 0U << -1;
                ^
UB_shift.cpp:11:16: warning: right shift count is negative [-Wshift-count-negative]
 int b = 0U >> -1;
                ^
UB_shift.cpp:15:38: warning: left shift count >= width of type [-Wshift-count-overflow]
 int c = 1U << (sizeof(int) * CHAR_BIT);
                                      ^
UB_shift.cpp:23:37: warning: left shift count >= width of type [-Wshift-count-overflow]
 int e = 1 << (sizeof(int) * CHAR_BIT);
                                     ^
UB_shift.cpp:27:15: warning: left shift of negative value [-Wshift-negative-value]
 int f = -1 << 0;
               ^

while clang (3.9.1) with the same options gives:

UB_shift.cpp:6:9: warning: integer overflow in preprocessor expression
#if (0U << -1) == 0
     ~~ ^  ~~
UB_shift.cpp:7:12: warning: shift count is negative [-Wshift-count-negative]
int a = 0U << -1;
           ^  ~~
UB_shift.cpp:10:9: warning: integer overflow in preprocessor expression
#if (0U >> -1) == 0
     ~~ ^  ~~
UB_shift.cpp:11:12: warning: shift count is negative [-Wshift-count-negative]
int b = 0U >> -1;
           ^  ~~
UB_shift.cpp:15:12: warning: shift count >= width of type [-Wshift-count-overflow]
int c = 1U << (sizeof(int) * CHAR_BIT);
           ^  ~~~~~~~~~~~~~~~~~~~~~~~~
UB_shift.cpp:23:11: warning: shift count >= width of type [-Wshift-count-overflow]
int e = 1 << (sizeof(int) * CHAR_BIT);
          ^  ~~~~~~~~~~~~~~~~~~~~~~~~
UB_shift.cpp:27:12: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
int f = -1 << 0;
        ~~ ^
7 warnings generated.

So none of these compiler talks about undefined behavior in the preprocessor (only overflows).
Clang reports two from seven cases, while gcc reports none.
In the C code gcc detects problems in 5 from 7 cases, while clang detects 5 from 7 cases.

Should we warn about this in cppccheck?

Greetings Hexcoder

[Daniel Marjamäki]

weird I thought cppcheck would warn about all these cases. in cppcheck, detecting ub is highest priority.

[Daniel Marjamäki]

the biggest problem was that this checker only checked the code in function bodies. Therefore your bugs was not detected.

If you see something like this in a checker:

const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
const std::size_t functions = symbolDatabase->functionScopes.size();
for (std::size_t i = 0; i < functions; ++i) {
    const Scope * scope = symbolDatabase->functionScopes[i];
    for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {

Then it will only check the code in function bodies.

We should probably review all these cases so they really make sense.

EDIT: I fixed the checker so all the bugs in your code are found:

$ ./cppcheck --enable=style --platform=unix32 -DCHAR_BIT=8 1.c
Checking 1.c ...
Checking 1.c: CHAR_BIT=8...
[1.c:2]: (error) Shifting by a negative value is undefined behaviour
[1.c:3]: (error) Shifting by a negative value is undefined behaviour
[1.c:7]: (portability) Shifting a negative value is technically undefined behaviour
[1.c:8]: (portability) Shifting a negative value is technically undefined behaviour
[1.c:4]: (error) Shifting 32-bit value by 32 bits is undefined behaviour
[1.c:5]: (error) Shifting 32-bit value by 32 bits is undefined behaviour
[1.c:6]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour

[Hexcoder]

Thanks for fixing!

From my understanding

[1.c:7]: (portability) Shifting a negative value is technically undefined behaviour

should be 'undefined behavior'.

BTW: My original concern was the preprocessor, which should emit a diagnostic, when its evaluation of expressions finds undefined behavior(http://en.cppreference.com/w/Talk:cpp/preprocessor/conditional)

for lines like this:
~~~
// undefined behavior
#if (0U << -1) == 0
...
~~~