[PATCH] D9924: Ignore report when the argument to malloc is assigned known value

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

Thanks! See the comments inline.


================
Comment at: lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp:182
@@ +181,3 @@
+      if (rhs->isEvaluatable(Context))
+        eraseAssign = true;
+      // Erase if the multiplicand was assigned a value,
----------------
In this case, the size will be a multiplication os two constants, which we will assume cannot be exploitable, so seems legitimate. (Maybe spell this out in the comment?)

================
Comment at: lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp:186
@@ +185,3 @@
+      // is a division operator and the denominator is > other multiplicand.
+      const Expr *rhse = rhs->IgnoreParenImpCasts();
+      if (const BinaryOperator *BOp = dyn_cast<BinaryOperator>(rhse)) {
----------------
I am not sure about this one.

We are saying that if the size expression in malloc was a multiplication of an expression and a constant (maxVal), than we should not warn if the expression was a devision of something unknown by another constant (val) that is greater or equal to the first constant (maxVal).

We don't know what that expression is and what the lhs of the devision is...

Other comments in regards to this check: the names used to represent the constants are not very expressive and there is quite a bit of copy and paste from the function above. The test only tests the case where val is equal to maxVal.


================
Comment at: lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp:319
@@ -245,3 +318,3 @@
           if (!FD)
-            return;
+            continue;
 
----------------
Could you add a test case for this change and the one below?
This should probably be bart of a separate commit as this is unrelated to the other change.


http://reviews.llvm.org/D9924