[PATCH] D49361: [analyzer] Detect pointers escaped after return statement execution in MallocChecker

[Reka Kovacs via Phabricator via cfe-commits]

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 at -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 at -1 {{Inner pointer invalidated by call to destructor}}
+  // expected-warning at -2 {{Use of memory after it is freed}}
+  // expected-note at -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 at -1 {{Inner pointer invalidated by call to destructor}}
+} // expected-warning {{Use of memory after it is freed}}
+// expected-note at -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<ReturnStmt>,
+                                     check::EndFunction,
                                      check::PreCall,
                                      check::PostStmt<CallExpr>,
                                      check::PostStmt<CXXNewExpr>,
@@ -217,6 +218,7 @@
   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,
@@ -2475,6 +2477,20 @@
     checkUseAfterFree(Sym, C, E);
 }
 
+void MallocChecker::checkEndFunction(const ReturnStmt *S,
+                                     CheckerContext &C) 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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49361.157375.patch
Type: text/x-patch
Size: 3530 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180725/569cb885/attachment.bin>