sval_type_max() sadness
Robert (Cc:ed) found a false positive from smatch, which boils down to:
```
#include <limits.h>
int
main(int argc, char *argv[])
{
long double sec = 0;
// also __int128 !
if (sec > (float)INT_MAX) {
0;
}
}
```$ ./src/smatch/smatch -m32 test.smatch.c test.smatch.c:9 main() warn: impossible condition '(sec > 2147483647) => (s32min-s32max > s32max)'
A cheesy fix here is:
$ git diff
smatch_type.c +5 -1
@@ -411,7 +411,11 @@ sval_t sval_type_max(struct symbol *base_type) base_type = &llong_ctype; ret.type = base_type; - ret.value = (~0ULL ) >> (64 -type_positive_bits (base_type)); +int pos_bits =type_positive_bits (base_type); +if (pos_bits >63 ) + pos_bits =63 ; + + ret.value = (~0ULL ) >> (64 - pos_bits);return ret; }
The issue being that for these types, type_positive_bits() is 64 or more. If only there were a tool to warn about negative shift operands :)
But type_positive_bits() is used all over, so I have no idea if there's a lot of other problems.
Or, if this patch is OK on its own, I can add a couple of tests and send a proper patch over.
Thoughts?
thanks
john
Thanks for the test case!
Of course, the history here is that the Linux kernel doesn't have floats so I never added support for it. I have this friend who is an expert on floats and people consult him when they are designing chips. He was explaining his work to me and I realized that I don't have the foggiest clue how floats work at all...
Anyway, I've tried to add floats to Smatch. It turned out not so hard as I imagined, but I know there are places that I missed. Probably it changes from a false positive into a crashing bug now. :/ The fix is almost always going to be to run valgrind to see which function crashes and then add a check:
if (type_is_fp(estate_type(state))) return false;
Do a pull and take a look. Tell me what you think. I will hack on this tomorrow as well.
regards,
can carpenter