[PATCH] D10634: Loss of Sign Checker

[Anna Zaks]

zaks.anna added a subscriber: zaks.anna.
zaks.anna added a comment.

I have a couple of high level comments.

Why do you have checkASTDecl (which is a syntactic check) in addition to the checkBind (which is a path-sensitive check)? Does checkASTDecl find more issues? can those be found using a path sensitive callback?

I am leaning toward allowing explicit assignments to "-1", like in this case: "unsigned int j = -1". The tool is much more usable if there are few false positives.

Some of the results look like false positives. For example, this one from https://drive.google.com/file/d/0BykPmWrCOxt2aGtRNTY4eXQ1OU0/view: 
fitscore.c:1077:21: warning: assigning negative value to unsigned variable loses sign and may cause undesired runtime behavior [clang-analyzer-alpha.core.LossOfSignAssign]

  namelen -= 9;  /* deleted the string 'HIERARCH ' */

It's hard to know what they are if we are only looking at the line where the issue is reported without seeing the pull path. Do we know that the value can be negative on that path?


================
Comment at: test/Analysis/LossOfSignAssign.c:19
@@ +18,3 @@
+  return i+j; // implicit conversion here too!
+}
+
----------------
What happens if the return type is unsigned?


http://reviews.llvm.org/D10634