From: Reka Kovacs <rekanikolett@gmail.com>
Date: Thu, 2 Aug 2018 23:02:08 +0000 (+0000)
Subject: [analyzer] Detect pointers escaped after ReturnStmt execution in MallocChecker.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bdd41df411fb9cd693bd21520c1e37ee4f3b71bc;p=clang

[analyzer] Detect pointers escaped after ReturnStmt execution in MallocChecker.

Objects local to a function are destroyed right after the statement returning
(part of) them is executed in the analyzer. This patch enables MallocChecker to
warn in these cases.

Differential Revision: https://reviews.llvm.org/D49361

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@338780 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index ebaf79a780..3ac7105f44 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -161,6 +161,7 @@ class MallocChecker : public Checker<check::DeadSymbols,
                                      check::PointerEscape,
                                      check::ConstPointerEscape,
                                      check::PreStmt<ReturnStmt>,
+                                     check::EndFunction,
                                      check::PreCall,
                                      check::PostStmt<CallExpr>,
                                      check::PostStmt<CXXNewExpr>,
@@ -217,6 +218,7 @@ public:
   void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
+  void checkEndFunction(const ReturnStmt *S, CheckerContext &C) const;
   ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
                             bool Assumption) const;
   void checkLocation(SVal l, bool isLoad, const Stmt *S,
@@ -353,7 +355,7 @@ private:
   static ProgramStateRef CallocMem(CheckerContext &C, 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 &C) const;
 
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const;
@@ -377,13 +379,16 @@ private:
                                    ProgramStateRef State,
                                    SymbolRef &EscapingSymbol) const;
 
-  // Implementation of the checkPointerEscape callabcks.
+  // Implementation of the checkPointerEscape callbacks.
   ProgramStateRef checkPointerEscapeAux(ProgramStateRef State,
                                   const InvalidatedSymbols &Escaped,
                                   const CallEvent *Call,
                                   PointerEscapeKind Kind,
                                   bool(*CheckRefState)(const RefState*)) const;
 
+  // Implementation of the checkPreStmt and checkEndFunction callbacks.
+  void checkEscapeOnReturn(const ReturnStmt *S, CheckerContext &C) 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::checkPreCall(const CallEvent &Call,
   }
 }
 
-void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const {
+void MallocChecker::checkPreStmt(const ReturnStmt *S,
+                                 CheckerContext &C) 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 &C) const {
+  checkEscapeOnReturn(S, C);
+}
+
+void MallocChecker::checkEscapeOnReturn(const ReturnStmt *S,
+                                        CheckerContext &C) const {
+  if (!S)
+    return;
+
   const Expr *E = S->getRetValue();
   if (!E)
     return;
diff --git a/test/Analysis/inner-pointer.cpp b/test/Analysis/inner-pointer.cpp
index 230e3396c5..6f9c8a8e8a 100644
--- a/test/Analysis/inner-pointer.cpp
+++ b/test/Analysis/inner-pointer.cpp
@@ -361,3 +361,24 @@ void func_default_arg() {
   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}}
diff --git a/test/Analysis/malloc-free-after-return.cpp b/test/Analysis/malloc-free-after-return.cpp
new file mode 100644
index 0000000000..80b443d87d
--- /dev/null
+++ b/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}}