llvmbot
commented
10 years ago I just looked at the FAQ and it looks like I misunderstood what the "Note: Taking true branch" message means. I guessed that that message was warning me that scan-build had proved that the false branch was never taken (i.e. dead code). After seeing the example warnings in the FAQ it seems that message is not an independent warning but rather an explanation of under what circumstances it thinks the error about the "<<" being undefined may occur. Therefore never mind what I said about the taking true branch note being wrong. Instead make this bug about the error about the "<<" being undefined, which is a false positive. I believe this false positive is caused by scan-build (apparently) not modeling double to unsigned long long conversion (i.e. 5.0 to 5ull), so it can't rule out Log2Floor64 receiving n=0 and returning -1, which in turn would make the left-shift undefined. My evidence for this is if you replace the problematic line with the following three the error message stays the same but uncommenting the assert makes it go away: unsigned long long five = 5.0; // assert(five>0); //uncommenting this line causes error report to go away unsigned long long count = 1ull << Log2Floor64(five);
Now that I (mostly) understand the bug I have four suggestions. These suggestions are mostly independent so you may want to make additional bugs for one or more of them to keep them separate.
-
Model double to integral type conversion so this false positive goes away.
-
Please add a page to the "user manual" menu on the webpage describing how to interpret the error reports, including the fact that the notes are statements of the branches taken in the error report, not a statement that the value of the tests can be determined statically. If a user receives multiple correct error reports it's possible to deduce what the error reports mean but a single error report, especially a false positive one, is ambiguous so documentation would help.
-
The error message says that the result "is undefined", which can be naturally interpreted as a statement that the analyzer has proven that undefined behavior will definitely occur, which is wrong in this case. If the analyzer is uncertain about whether or not undefined behavior is occurring it should say the result "may be undefined". The same suggestion applies to all error messages: make it clear whether the error message is for a certain error or a possible error.
-
The error message would be a lot more useful if it were more specific about the problem than just the left-shift being undefined. For example state that scan-build deduced that the right-hand operand was bounded between -1 and 63 inclusive (or whatever scan-build actually deduced), which contains value(s) below the allowed range 0 to 63 inclusive.
belkadan
Extended Description
Running scan-build on this program yeilds a warning about the result of the left-shift expression being undefined, which appears to be spurious, and a note that the 'true' branch of an 'if' is taken, which is definitely invalid. I've attached the scan-build error report. Running the program confirms the obvious: the 'false' branch of the 'if' is taken. The expected output is no bug reports and (optionally) a note that the 'false' branch of the 'if' is taken.
------ Details ------
I created a fresh Ubuntu 14.04.1 install. I then installed the packages clang-3.5 and g++ with apt-get.
wschudy@ubuntu:~$ cat test.cc
include
include
inline int Log2Floor64(unsigned long long n) { return n == 0 ? -1 : 63 ^ __builtin_clzll(n); }
int main(int argc, char** argv) { std::vector xs = {0};
if (xs[0] != 0) {
unsigned long long count = 1ull << Log2Floor64(5.0);
printf("\"Then\" branch, i.e. xs[0] != 0, with count == %llu\n", count);
} else {
printf("\"Else\" branch, i.e. xs[0] == 0\n");
}
}
wschudy@ubuntu:~$ clang --version Ubuntu clang version 3.5-1ubuntu1 (trunk) (based on LLVM 3.5) Target: x86_64-pc-linux-gnu Thread model: posix wschudy@ubuntu:~$ clang test.cc -std=c++11 -lstdc++ wschudy@ubuntu:~$ ./a.out "Else" branch, i.e. xs[0] == 0 wschudy@ubuntu:~$ scan-build clang test.cc -std=c++11 -lstdc++ scan-build: Using '/usr/bin/clang' for static analysis test.cc:11:37: warning: The result of the '<<' expression is undefined unsigned long long count = 1ull << Log2Floor64(5.0);