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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
