hiraditya added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp:182
@@ +181,3 @@
+ if (rhs->isEvaluatable(Context))
+ eraseAssign = true;
+ // Erase if the multiplicand was assigned a value,
----------------
zaks.anna wrote:
> 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?)
Done.
================
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)) {
----------------
zaks.anna wrote:
> 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.
>
I have added more comments for clarity. Essentially if:
x = a/b; // where n < b
malloc (x*n); //Then x*n will not overflow.
With regards to copy paste, I'm not sure about how to do this in a different
way.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp:319
@@ -245,3 +318,3 @@
if (!FD)
- return;
+ continue;
----------------
zaks.anna wrote:
> 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.
I tried to find a test case but couldn't. If you want i can submit this as a
separate patch without a test case.
http://reviews.llvm.org/D9924
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
