diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -2190,10 +2190,6 @@ not tighten the domain to prevent the overflow in the subsequent multiplication operation. - - If the variable ``n`` participates in a comparison anywhere in the enclosing - function's scope, even after the ``malloc()``, the report will be still - suppressed. - - It is an AST-based checker, thus it does not make use of the path-sensitive taint-analysis. diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp @@ -32,12 +32,14 @@ namespace { struct MallocOverflowCheck { + const CallExpr *call; const BinaryOperator *mulop; const Expr *variable; APSInt maxVal; - MallocOverflowCheck(const BinaryOperator *m, const Expr *v, APSInt val) - : mulop(m), variable(v), maxVal(std::move(val)) {} + MallocOverflowCheck(const CallExpr *call, const BinaryOperator *m, + const Expr *v, APSInt val) + : call(call), mulop(m), variable(v), maxVal(std::move(val)) {} }; class MallocOverflowSecurityChecker : public Checker { @@ -46,8 +48,8 @@ BugReporter &BR) const; void CheckMallocArgument( - SmallVectorImpl &PossibleMallocOverflows, - const Expr *TheArgument, ASTContext &Context) const; + SmallVectorImpl &PossibleMallocOverflows, + const CallExpr *TheCall, ASTContext &Context) const; void OutputPossibleOverflows( SmallVectorImpl &PossibleMallocOverflows, @@ -62,16 +64,15 @@ } void MallocOverflowSecurityChecker::CheckMallocArgument( - SmallVectorImpl &PossibleMallocOverflows, - const Expr *TheArgument, - ASTContext &Context) const { + SmallVectorImpl &PossibleMallocOverflows, + const CallExpr *TheCall, ASTContext &Context) const { /* Look for a linear combination with a single variable, and at least one multiplication. Reject anything that applies to the variable: an explicit cast, conditional expression, an operation that could reduce the range of the result, or anything too complicated :-). */ - const Expr *e = TheArgument; + const Expr *e = TheCall->getArg(0); const BinaryOperator * mulop = nullptr; APSInt maxVal; @@ -115,9 +116,8 @@ // the data so when the body of the function is completely available // we can check for comparisons. - // TODO: Could push this into the innermost scope where 'e' is - // defined, rather than the whole function. - PossibleMallocOverflows.push_back(MallocOverflowCheck(mulop, e, maxVal)); + PossibleMallocOverflows.push_back( + MallocOverflowCheck(TheCall, mulop, e, maxVal)); } namespace { @@ -158,12 +158,15 @@ } void CheckExpr(const Expr *E_p) { - auto PredTrue = [](const MallocOverflowCheck &) { return true; }; const Expr *E = E_p->IgnoreParenImpCasts(); + const auto PrecedesMalloc = [E, this](const MallocOverflowCheck &c) { + return Context.getSourceManager().isBeforeInTranslationUnit( + E->getExprLoc(), c.call->getExprLoc()); + }; if (const DeclRefExpr *DR = dyn_cast(E)) - Erase(DR, PredTrue); + Erase(DR, PrecedesMalloc); else if (const auto *ME = dyn_cast(E)) { - Erase(ME, PredTrue); + Erase(ME, PrecedesMalloc); } } @@ -322,7 +325,7 @@ if (FnInfo->isStr ("malloc") || FnInfo->isStr ("_MALLOC")) { if (TheCall->getNumArgs() == 1) - CheckMallocArgument(PossibleMallocOverflows, TheCall->getArg(0), + CheckMallocArgument(PossibleMallocOverflows, TheCall, mgr.getASTContext()); } } diff --git a/clang/test/Analysis/malloc-overflow.c b/clang/test/Analysis/malloc-overflow.c --- a/clang/test/Analysis/malloc-overflow.c +++ b/clang/test/Analysis/malloc-overflow.c @@ -111,3 +111,40 @@ return NULL; return malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} } + +void *check_before_malloc(int n, int x) { + int *p = NULL; + if (n > 10) + return NULL; + if (x == 42) + p = malloc(n * sizeof(int)); // no-warning, the check precedes the allocation + + // Do some other stuff, e.g. initialize the memory. + return p; +} + +void *check_after_malloc(int n, int x) { + int *p = NULL; + if (x == 42) + p = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} + + // The check is after the allocation! + if (n > 10) { + // Do something conditionally. + } + return p; +} + +#define GREATER_THAN(lhs, rhs) (lhs > rhs) +void *check_after_malloc_using_macros(int n, int x) { + int *p = NULL; + if (x == 42) + p = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}} + + if (GREATER_THAN(n, 10)) + return NULL; + + // Do some other stuff, e.g. initialize the memory. + return p; +} +#undef GREATER_THAN