This revision was automatically updated to reflect the committed changes.
Closed by commit rG68088563fbad: [analyzer] MallocOverflow should consider
comparisons only preceding malloc (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107804/new/
https://reviews.llvm.org/D107804
Files:
clang/docs/analyzer/checkers.rst
clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
clang/test/Analysis/malloc-overflow.c
Index: clang/test/Analysis/malloc-overflow.c
===================================================================
--- clang/test/Analysis/malloc-overflow.c
+++ 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
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<check::ASTCodeBody> {
@@ -46,8 +48,8 @@
BugReporter &BR) const;
void CheckMallocArgument(
- SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows,
- const Expr *TheArgument, ASTContext &Context) const;
+ SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows,
+ const CallExpr *TheCall, ASTContext &Context) const;
void OutputPossibleOverflows(
SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows,
@@ -62,16 +64,15 @@
}
void MallocOverflowSecurityChecker::CheckMallocArgument(
- SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows,
- const Expr *TheArgument,
- ASTContext &Context) const {
+ SmallVectorImpl<MallocOverflowCheck> &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<DeclRefExpr>(E))
- Erase<DeclRefExpr>(DR, PredTrue);
+ Erase<DeclRefExpr>(DR, PrecedesMalloc);
else if (const auto *ME = dyn_cast<MemberExpr>(E)) {
- Erase<MemberExpr>(ME, PredTrue);
+ Erase<MemberExpr>(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());
}
}
Index: clang/docs/analyzer/checkers.rst
===================================================================
--- clang/docs/analyzer/checkers.rst
+++ 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.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
