Index: clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp +++ 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,8 +158,11 @@ } void CheckExpr(const Expr *E_p) { - auto PredTrue = [](const MallocOverflowCheck &) { return true; }; const Expr *E = E_p->IgnoreParenImpCasts(); + auto PredTrue = [E, this](const MallocOverflowCheck &c) { + return Context.getSourceManager().isBeforeInTranslationUnit( + E->getExprLoc(), c.call->getExprLoc()); + }; if (const DeclRefExpr *DR = dyn_cast(E)) Erase(DR, PredTrue); else if (const auto *ME = dyn_cast(E)) { @@ -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()); } } Index: clang/test/Analysis/malloc-overflow.c =================================================================== --- clang/test/Analysis/malloc-overflow.c +++ clang/test/Analysis/malloc-overflow.c @@ -111,3 +111,26 @@ 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; +}