[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338780: [analyzer] Detect pointers escaped after ReturnStmt
execution in MallocChecker. (authored by rkovacs, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49361?vs=158681=158857#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49361
Files:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/test/Analysis/inner-pointer.cpp
cfe/trunk/test/Analysis/malloc-free-after-return.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -161,6 +161,7 @@
check::PointerEscape,
check::ConstPointerEscape,
check::PreStmt,
+ check::EndFunction,
check::PreCall,
check::PostStmt,
check::PostStmt,
@@ -217,6 +218,7 @@
void checkPostStmt(const BlockExpr *BE, CheckerContext ) const;
void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
void checkPreStmt(const ReturnStmt *S, CheckerContext ) const;
+ void checkEndFunction(const ReturnStmt *S, CheckerContext ) const;
ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
bool Assumption) const;
void checkLocation(SVal l, bool isLoad, const Stmt *S,
@@ -353,7 +355,7 @@
static ProgramStateRef CallocMem(CheckerContext , const CallExpr *CE,
ProgramStateRef State);
- ///Check if the memory associated with this symbol was released.
+ /// Check if the memory associated with this symbol was released.
bool isReleased(SymbolRef Sym, CheckerContext ) const;
bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
@@ -377,13 +379,16 @@
ProgramStateRef State,
SymbolRef ) const;
- // Implementation of the checkPointerEscape callabcks.
+ // Implementation of the checkPointerEscape callbacks.
ProgramStateRef checkPointerEscapeAux(ProgramStateRef State,
const InvalidatedSymbols ,
const CallEvent *Call,
PointerEscapeKind Kind,
bool(*CheckRefState)(const RefState*)) const;
+ // Implementation of the checkPreStmt and checkEndFunction callbacks.
+ void checkEscapeOnReturn(const ReturnStmt *S, CheckerContext ) const;
+
///@{
/// Tells if a given family/call/symbol is tracked by the current checker.
/// Sets CheckKind to the kind of the checker responsible for this
@@ -2451,7 +2456,24 @@
}
}
-void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext ) const {
+void MallocChecker::checkPreStmt(const ReturnStmt *S,
+ CheckerContext ) const {
+ checkEscapeOnReturn(S, C);
+}
+
+// In the CFG, automatic destructors come after the return statement.
+// This callback checks for returning memory that is freed by automatic
+// destructors, as those cannot be reached in checkPreStmt().
+void MallocChecker::checkEndFunction(const ReturnStmt *S,
+ CheckerContext ) const {
+ checkEscapeOnReturn(S, C);
+}
+
+void MallocChecker::checkEscapeOnReturn(const ReturnStmt *S,
+CheckerContext ) const {
+ if (!S)
+return;
+
const Expr *E = S->getRetValue();
if (!E)
return;
Index: cfe/trunk/test/Analysis/inner-pointer.cpp
===
--- cfe/trunk/test/Analysis/inner-pointer.cpp
+++ cfe/trunk/test/Analysis/inner-pointer.cpp
@@ -361,3 +361,24 @@
consume(c);// expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
+
+struct S {
+ std::string to_string() { return s; }
+private:
+ std::string s;
+};
+
+const char *escape_via_return_temp() {
+ S x;
+ return x.to_string().c_str(); // expected-note {{Dangling inner pointer obtained here}}
+ // expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+ // expected-warning@-2 {{Use of memory after it is freed}}
+ // expected-note@-3 {{Use of memory after it is freed}}
+}
+
+const char *escape_via_return_local() {
+ std::string s;
+ return s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+// expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+} // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is
[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker
NoQ accepted this revision. NoQ added a comment. All this stuff looks great! Please commit. https://reviews.llvm.org/D49361 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker
rnkovacs updated this revision to Diff 158681.
rnkovacs marked an inline comment as done.
rnkovacs added a comment.
Add helper function to be used in both callbacks.
https://reviews.llvm.org/D49361
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/inner-pointer.cpp
test/Analysis/malloc-free-after-return.cpp
Index: test/Analysis/malloc-free-after-return.cpp
===
--- /dev/null
+++ test/Analysis/malloc-free-after-return.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct S {
+ S() : Data(new int) {}
+ ~S() { delete Data; }
+ int *getData() { return Data; }
+
+private:
+ int *Data;
+};
+
+int *freeAfterReturnTemp() {
+ return S().getData(); // expected-warning {{Use of memory after it is freed}}
+}
+
+int *freeAfterReturnLocal() {
+ S X;
+ return X.getData();
+} // expected-warning {{Use of memory after it is freed}}
Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -361,3 +361,24 @@
consume(c);// expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
+
+struct S {
+ std::string to_string() { return s; }
+private:
+ std::string s;
+};
+
+const char *escape_via_return_temp() {
+ S x;
+ return x.to_string().c_str(); // expected-note {{Dangling inner pointer obtained here}}
+ // expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+ // expected-warning@-2 {{Use of memory after it is freed}}
+ // expected-note@-3 {{Use of memory after it is freed}}
+}
+
+const char *escape_via_return_local() {
+ std::string s;
+ return s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+// expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+} // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is freed}}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -161,6 +161,7 @@
check::PointerEscape,
check::ConstPointerEscape,
check::PreStmt,
+ check::EndFunction,
check::PreCall,
check::PostStmt,
check::PostStmt,
@@ -217,6 +218,7 @@
void checkPostStmt(const BlockExpr *BE, CheckerContext ) const;
void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
void checkPreStmt(const ReturnStmt *S, CheckerContext ) const;
+ void checkEndFunction(const ReturnStmt *S, CheckerContext ) const;
ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
bool Assumption) const;
void checkLocation(SVal l, bool isLoad, const Stmt *S,
@@ -353,7 +355,7 @@
static ProgramStateRef CallocMem(CheckerContext , const CallExpr *CE,
ProgramStateRef State);
- ///Check if the memory associated with this symbol was released.
+ /// Check if the memory associated with this symbol was released.
bool isReleased(SymbolRef Sym, CheckerContext ) const;
bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
@@ -377,13 +379,16 @@
ProgramStateRef State,
SymbolRef ) const;
- // Implementation of the checkPointerEscape callabcks.
+ // Implementation of the checkPointerEscape callbacks.
ProgramStateRef checkPointerEscapeAux(ProgramStateRef State,
const InvalidatedSymbols ,
const CallEvent *Call,
PointerEscapeKind Kind,
bool(*CheckRefState)(const RefState*)) const;
+ // Implementation of the checkPreStmt and checkEndFunction callbacks.
+ void checkEscapeOnReturn(const ReturnStmt *S, CheckerContext ) const;
+
///@{
/// Tells if a given family/call/symbol is tracked by the current checker.
/// Sets CheckKind to the kind of the checker responsible for this
@@ -2451,7 +2456,24 @@
}
}
-void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext ) const {
+void MallocChecker::checkPreStmt(const ReturnStmt *S,
+ CheckerContext ) const {
+ checkEscapeOnReturn(S, C);
+}
+
+// In the CFG, automatic destructors come after the return statement.
+// This callback checks for returning memory that is freed by automatic
+// destructors, as those
[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2488 + + checkPreStmt(S, C); +} Let's do a common sub-function, similarly to how `MallocChecker::checkPointerEscape` and `MallocChecker::checkConstPointerEscape` both call `MallocChecker::checkPointerEscapeAux`. Should prevent us from unleashing `addTransition` hell if the code becomes more complicated. https://reviews.llvm.org/D49361 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker
rnkovacs updated this revision to Diff 157966.
rnkovacs marked an inline comment as done.
rnkovacs added a comment.
De-duplicate & add comment.
https://reviews.llvm.org/D49361
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/inner-pointer.cpp
test/Analysis/malloc-free-after-return.cpp
Index: test/Analysis/malloc-free-after-return.cpp
===
--- /dev/null
+++ test/Analysis/malloc-free-after-return.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct S {
+ S() : Data(new int) {}
+ ~S() { delete Data; }
+ int *getData() { return Data; }
+
+private:
+ int *Data;
+};
+
+int *freeAfterReturnTemp() {
+ return S().getData(); // expected-warning {{Use of memory after it is freed}}
+}
+
+int *freeAfterReturnLocal() {
+ S X;
+ return X.getData();
+} // expected-warning {{Use of memory after it is freed}}
Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -361,3 +361,24 @@
consume(c);// expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
+
+struct S {
+ std::string to_string() { return s; }
+private:
+ std::string s;
+};
+
+const char *escape_via_return_temp() {
+ S x;
+ return x.to_string().c_str(); // expected-note {{Dangling inner pointer
obtained here}}
+ // expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+ // expected-warning@-2 {{Use of memory after it is freed}}
+ // expected-note@-3 {{Use of memory after it is freed}}
+}
+
+const char *escape_via_return_local() {
+ std::string s;
+ return s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+// expected-note@-1 {{Inner pointer invalidated by call to
destructor}}
+} // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is freed}}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -161,6 +161,7 @@
check::PointerEscape,
check::ConstPointerEscape,
check::PreStmt,
+ check::EndFunction,
check::PreCall,
check::PostStmt,
check::PostStmt,
@@ -217,6 +218,7 @@
void checkPostStmt(const BlockExpr *BE, CheckerContext ) const;
void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
void checkPreStmt(const ReturnStmt *S, CheckerContext ) const;
+ void checkEndFunction(const ReturnStmt *S, CheckerContext ) const;
ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
bool Assumption) const;
void checkLocation(SVal l, bool isLoad, const Stmt *S,
@@ -2475,6 +2477,17 @@
checkUseAfterFree(Sym, C, E);
}
+// In the CFG, automatic destructors come after the return statement.
+// This callback checks for returning memory that is freed by automatic
+// destructors, as those cannot be reached from `checkPreStmt()`.
+void MallocChecker::checkEndFunction(const ReturnStmt *S,
+ CheckerContext ) const {
+ if (!S)
+return;
+
+ checkPreStmt(S, C);
+}
+
// TODO: Blocks should be either inlined or should call invalidate regions
// upon invocation. After that's in place, special casing here will not be
// needed.
Index: test/Analysis/malloc-free-after-return.cpp
===
--- /dev/null
+++ test/Analysis/malloc-free-after-return.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct S {
+ S() : Data(new int) {}
+ ~S() { delete Data; }
+ int *getData() { return Data; }
+
+private:
+ int *Data;
+};
+
+int *freeAfterReturnTemp() {
+ return S().getData(); // expected-warning {{Use of memory after it is freed}}
+}
+
+int *freeAfterReturnLocal() {
+ S X;
+ return X.getData();
+} // expected-warning {{Use of memory after it is freed}}
Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -361,3 +361,24 @@
consume(c);// expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
+
+struct S {
+ std::string to_string() { return s; }
+private:
+ std::string s;
+};
+
+const char
[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Yup, tests for temporaries are great to have as well. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2475-2477 // Check if we are returning freed memory. if (Sym) checkUseAfterFree(Sym, C, E); The old code makes the warning appear slightly earlier, so we still need it, even if duplicated. Let's comment about that and maybe de-duplicate? https://reviews.llvm.org/D49361 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker
rnkovacs updated this revision to Diff 157375.
rnkovacs retitled this revision from "[analyzer][WIP] Detect pointers escaped
after return statement execution in MallocChecker" to "[analyzer] Detect
pointers escaped after return statement execution in MallocChecker".
rnkovacs edited the summary of this revision.
rnkovacs added a comment.
Updated to use the extended `checkEndFunction()` callback (committed in
https://reviews.llvm.org/rL337215 - I forgot to add it as a dependency).
The first ones of the 2-2 tests added for `InnerPointerChecker` and
`NewDeleteChecker` already worked before this patch, the second ones are the
newly working ones. I just felt they are somewhat both relevant and maybe worth
comparing.
https://reviews.llvm.org/D49361
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/inner-pointer.cpp
test/Analysis/malloc-free-after-return.cpp
Index: test/Analysis/malloc-free-after-return.cpp
===
--- /dev/null
+++ test/Analysis/malloc-free-after-return.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete
-analyzer-output=text -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct S {
+ S() : Data(new int) {}
+ ~S() { delete Data; }
+ int *getData() { return Data; }
+
+private:
+ int *Data;
+};
+
+int *freeAfterReturnTemp() {
+ return S().getData(); // expected-warning {{Use of memory after it is freed}}
+}
+
+int *freeAfterReturnLocal() {
+ S X;
+ return X.getData();
+} // expected-warning {{Use of memory after it is freed}}
Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -277,6 +277,27 @@
// expected-note@-1 {{Use of memory after it is freed}}
}
+struct S {
+ std::string to_string() { return s; }
+private:
+ std::string s;
+};
+
+const char *escape_via_return_temp() {
+ S x;
+ return x.to_string().c_str(); // expected-note {{Dangling inner pointer
obtained here}}
+ // expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+ // expected-warning@-2 {{Use of memory after it is freed}}
+ // expected-note@-3 {{Use of memory after it is freed}}
+}
+
+const char *escape_via_return_local() {
+ std::string s;
+ return s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
+// expected-note@-1 {{Inner pointer invalidated by call to
destructor}}
+} // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is freed}}
+
void deref_after_scope_ok(bool cond) {
const char *c, *d;
std::string s;
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -161,6 +161,7 @@
check::PointerEscape,
check::ConstPointerEscape,
check::PreStmt,
+ check::EndFunction,
check::PreCall,
check::PostStmt,
check::PostStmt,
@@ -217,6 +218,7 @@
void checkPostStmt(const BlockExpr *BE, CheckerContext ) const;
void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
void checkPreStmt(const ReturnStmt *S, CheckerContext ) const;
+ void checkEndFunction(const ReturnStmt *S, CheckerContext ) const;
ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
bool Assumption) const;
void checkLocation(SVal l, bool isLoad, const Stmt *S,
@@ -2475,6 +2477,20 @@
checkUseAfterFree(Sym, C, E);
}
+void MallocChecker::checkEndFunction(const ReturnStmt *S,
+ CheckerContext ) const {
+ if (!S)
+return;
+
+ const Expr *E = S->getRetValue();
+ if (!E)
+return;
+
+ SymbolRef Sym = C.getSVal(E).getAsSymbol();
+ if (Sym)
+checkUseAfterFree(Sym, C, E);
+}
+
// TODO: Blocks should be either inlined or should call invalidate regions
// upon invocation. After that's in place, special casing here will not be
// needed.
Index: test/Analysis/malloc-free-after-return.cpp
===
--- /dev/null
+++ test/Analysis/malloc-free-after-return.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete -analyzer-output=text -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct S {
+ S() : Data(new int) {}
+ ~S() { delete Data; }
+ int *getData() { return Data; }
+
+private:
+ int *Data;
+};
+
+int *freeAfterReturnTemp() {
+ return S().getData(); // expected-warning {{Use of memory after it is freed}}
+}
+
+int
