sval_type_max() sadness

sval_type_max() sadness

[John Levon]

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
diff --git a/smatch_type.c b/smatch_type.c
index 305a0b5c..1e289363 100644
--- a/smatch_type.c
+++ b/smatch_type.c
@@ -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

Re: sval_type_max() sadness

[Dan Carpenter]

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