From acdc13cb00591e2ab2b168c7924d7eb57fa4808e Mon Sep 17 00:00:00 2001
From: Anna Zaks <ganna@apple.com>
Date: Thu, 7 Feb 2013 23:05:43 +0000
Subject: [PATCH] [analyzer] Add pointer escape type param to
 checkPointerEscape callback

The checkPointerEscape callback previously did not specify how a
pointer escaped. This change includes an enum which describes the
different ways a pointer may escape. This enum is passed to the
checkPointerEscape callback when a pointer escapes. If the escape
is due to a function call, the call is passed. This changes
previous behavior where the call is passed as NULL if the escape
was due to indirectly invalidating the region the pointer referenced.

A patch by Branden Archer!

llvm-svn: 174677
---
 clang/include/clang/StaticAnalyzer/Core/Checker.h  |  6 ++--
 .../clang/StaticAnalyzer/Core/CheckerManager.h     | 26 +++++++++++++++--
 .../Checkers/CheckerDocumentation.cpp              |  4 ++-
 .../lib/StaticAnalyzer/Checkers/MallocChecker.cpp  | 11 +++++--
 .../Checkers/SimpleStreamChecker.cpp               | 11 +++++--
 clang/lib/StaticAnalyzer/Core/CheckerManager.cpp   | 10 +++++--
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp       | 11 ++++---
 .../system-header-simulator-for-simple-stream.h    |  6 ++++
 .../test/Analysis/Inputs/system-header-simulator.h |  8 +++++
 clang/test/Analysis/malloc.c                       | 34 ++++++++++++++++++++++
 clang/test/Analysis/simple-stream-checks.c         | 13 +++++++++
 11 files changed, 123 insertions(+), 17 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index a190e1f..82bac7d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -323,10 +323,12 @@ class PointerEscape {
   _checkPointerEscape(void *checker,
                      ProgramStateRef State,
                      const InvalidatedSymbols &Escaped,
-                     const CallEvent *Call) {
+                     const CallEvent *Call,
+                     PointerEscapeKind Kind) {
     return ((const CHECKER *)checker)->checkPointerEscape(State, 
                                                           Escaped, 
-                                                          Call);
+                                                          Call,
+                                                          Kind);
   }
 
 public:
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 2fd71dd..4353ebf 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -112,6 +112,26 @@ public:
   RET operator()() const { return Fn(Checker); } 
 };
 
+/// \brief Describes the different reasons a pointer escapes
+/// during analysis.
+enum PointerEscapeKind {
+  /// A pointer escapes due to binding its value to a location
+  /// that the analyzer cannot track.
+  PSK_EscapeOnBind,
+
+  /// The pointer has been passed to a function call directly.
+  PSK_DirectEscapeOnCall,
+
+  /// The pointer has been passed to a function indirectly.
+  /// For example, the pointer is accessible through an
+  /// argument to a function.
+  PSK_IndirectEscapeOnCall,
+
+  /// The reason for pointer escape is unknown. For example, 
+  /// a region containing this pointer is invalidated.
+  PSK_EscapeOther
+};
+
 class CheckerManager {
   const LangOptions LangOpts;
 
@@ -330,7 +350,8 @@ public:
   ProgramStateRef 
   runCheckersForPointerEscape(ProgramStateRef State,
                               const InvalidatedSymbols &Escaped,
-                              const CallEvent *Call);
+                              const CallEvent *Call,
+                              PointerEscapeKind Kind);
 
   /// \brief Run checkers for handling assumptions on symbolic values.
   ProgramStateRef runCheckersForEvalAssume(ProgramStateRef state,
@@ -420,7 +441,8 @@ public:
 
   typedef CheckerFn<ProgramStateRef (ProgramStateRef,
                                      const InvalidatedSymbols &Escaped,
-                                     const CallEvent *Call)>
+                                     const CallEvent *Call,
+                                     PointerEscapeKind Kind)>
       CheckPointerEscapeFunc;
   
   typedef CheckerFn<ProgramStateRef (ProgramStateRef,
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
index 35baef6..ed89788 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
@@ -265,10 +265,12 @@ public:
   /// \param Escaped The list of escaped symbols.
   /// \param Call The corresponding CallEvent, if the symbols escape as 
   /// parameters to the given call.
+  /// \param Kind How the symbols have escaped.
   /// \returns Checkers can modify the state by returning a new state.
   ProgramStateRef checkPointerEscape(ProgramStateRef State,
                                      const InvalidatedSymbols &Escaped,
-                                     const CallEvent *Call) const {
+                                     const CallEvent *Call,
+                                     PointerEscapeKind Kind) const {
     return State;
   }
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 33553a1..9afd8cf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -158,7 +158,8 @@ public:
 
   ProgramStateRef checkPointerEscape(ProgramStateRef State,
                                     const InvalidatedSymbols &Escaped,
-                                    const CallEvent *Call) const;
+                                    const CallEvent *Call,
+                                    PointerEscapeKind Kind) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State,
                   const char *NL, const char *Sep) const;
@@ -1450,11 +1451,15 @@ bool MallocChecker::doesNotFreeMemory(const CallEvent *Call,
 
 ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State,
                                              const InvalidatedSymbols &Escaped,
-                                             const CallEvent *Call) const {
+                                             const CallEvent *Call,
+                                             PointerEscapeKind Kind) const {
   // If we know that the call does not free memory, keep tracking the top
   // level arguments.       
-  if (Call && doesNotFreeMemory(Call, State))
+  if ((Kind == PSK_DirectEscapeOnCall ||
+       Kind == PSK_IndirectEscapeOnCall) &&
+      doesNotFreeMemory(Call, State)) {
     return State;
+  }
 
   for (InvalidatedSymbols::const_iterator I = Escaped.begin(),
                                           E = Escaped.end();
diff --git a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
index e2a19fc..1ccf339 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
@@ -82,7 +82,8 @@ public:
   /// Stop tracking addresses which escape.
   ProgramStateRef checkPointerEscape(ProgramStateRef State,
                                     const InvalidatedSymbols &Escaped,
-                                    const CallEvent *Call) const;
+                                    const CallEvent *Call,
+                                    PointerEscapeKind Kind) const;
 };
 
 } // end anonymous namespace
@@ -255,10 +256,14 @@ bool SimpleStreamChecker::guaranteedNotToCloseFile(const CallEvent &Call) const{
 ProgramStateRef
 SimpleStreamChecker::checkPointerEscape(ProgramStateRef State,
                                         const InvalidatedSymbols &Escaped,
-                                        const CallEvent *Call) const {
+                                        const CallEvent *Call,
+                                        PointerEscapeKind Kind) const {
   // If we know that the call cannot close a file, there is nothing to do.
-  if (Call && guaranteedNotToCloseFile(*Call))
+  if ((Kind == PSK_DirectEscapeOnCall ||
+       Kind == PSK_IndirectEscapeOnCall) &&
+      guaranteedNotToCloseFile(*Call)) {
     return State;
+  }
 
   for (InvalidatedSymbols::const_iterator I = Escaped.begin(),
                                           E = Escaped.end();
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index dc0cab7..a383799 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -488,13 +488,19 @@ CheckerManager::runCheckersForRegionChanges(ProgramStateRef state,
 ProgramStateRef
 CheckerManager::runCheckersForPointerEscape(ProgramStateRef State,
                                            const InvalidatedSymbols &Escaped,
-                                           const CallEvent *Call) {
+                                           const CallEvent *Call,
+                                           PointerEscapeKind Kind) {
+  assert((Call != NULL ||
+          (Kind != PSK_DirectEscapeOnCall &&
+           Kind != PSK_IndirectEscapeOnCall)) &&
+         "Call must not be NULL when escaping on call");
+
   for (unsigned i = 0, e = PointerEscapeCheckers.size(); i != e; ++i) {
     // If any checker declares the state infeasible (or if it starts that way),
     // bail out.
     if (!State)
       return NULL;
-    State = PointerEscapeCheckers[i](State, Escaped, Call);
+    State = PointerEscapeCheckers[i](State, Escaped, Call, Kind);
   }
   return State;
 }
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 636aa3b..f092f1a 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1648,7 +1648,8 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(ProgramStateRef State,
   const InvalidatedSymbols &EscapedSymbols = Scanner.getSymbols();
   State = getCheckerManager().runCheckersForPointerEscape(State,
                                                           EscapedSymbols,
-                                                          /*CallEvent*/ 0);
+                                                          /*CallEvent*/ 0,
+                                                          PSK_EscapeOnBind);
 
   return State;
 }
@@ -1665,7 +1666,9 @@ ExprEngine::processPointerEscapedOnInvalidateRegions(ProgramStateRef State,
 
   if (!Call)
     return getCheckerManager().runCheckersForPointerEscape(State,
-                                                           *Invalidated, 0);
+                                                           *Invalidated,
+                                                           0,
+                                                           PSK_EscapeOther);
     
   // If the symbols were invalidated by a call, we want to find out which ones 
   // were invalidated directly due to being arguments to the call.
@@ -1687,12 +1690,12 @@ ExprEngine::processPointerEscapedOnInvalidateRegions(ProgramStateRef State,
 
   if (!SymbolsDirectlyInvalidated.empty())
     State = getCheckerManager().runCheckersForPointerEscape(State,
-        SymbolsDirectlyInvalidated, Call);
+        SymbolsDirectlyInvalidated, Call, PSK_DirectEscapeOnCall);
 
   // Notify about the symbols that get indirectly invalidated by the call.
   if (!SymbolsIndirectlyInvalidated.empty())
     State = getCheckerManager().runCheckersForPointerEscape(State,
-        SymbolsIndirectlyInvalidated, /*CallEvent*/ 0);
+        SymbolsIndirectlyInvalidated, Call, PSK_IndirectEscapeOnCall);
 
   return State;
 }
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index f08f3f6..b65b7a6 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -13,3 +13,9 @@ int fputc(int, FILE *);
 int fputs(const char * restrict, FILE * restrict) __asm("_" "fputs" );
 int fclose(FILE *);
 void exit(int);
+
+// The following is a fake system header function
+typedef struct __FileStruct {
+  FILE * p;
+} FileStruct;
+void fakeSystemHeaderCall(FileStruct *);
diff --git a/clang/test/Analysis/Inputs/system-header-simulator.h b/clang/test/Analysis/Inputs/system-header-simulator.h
index 4c12131..04688c78 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator.h
@@ -67,3 +67,11 @@ typedef void (*xpc_finalizer_t)(void *value);
 void xpc_connection_set_context(xpc_connection_t connection, void *context);
 void xpc_connection_set_finalizer_f(xpc_connection_t connection, xpc_finalizer_t finalizer);
 void xpc_connection_resume(xpc_connection_t connection);
+
+//The following is a fake system header function
+void fakeSystemHeaderCallInt(int *);
+
+typedef struct __SomeStruct {
+  char * p;
+} SomeStruct;
+void fakeSystemHeaderCall(SomeStruct *);
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index ed2d8e9..9dc17e6 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -13,6 +13,7 @@ void *reallocf(void *ptr, size_t size);
 void *calloc(size_t nmemb, size_t size);
 char *strdup(const char *s);
 char *strndup(const char *s, size_t n);
+int memcmp(const void *s1, const void *s2, size_t n);
 
 void myfoo(int *p);
 void myfooint(int p);
@@ -1023,6 +1024,27 @@ char *testLeakWithinReturn(char *str) {
   return strdup(strdup(str)); // expected-warning{{leak}}
 }
 
+void passConstPtr(const char * ptr);
+
+void testPassConstPointer() {
+  char * string = malloc(sizeof(char)*10);
+  passConstPtr(string);
+  return; // expected-warning {{leak}}
+}
+
+void testPassConstPointerIndirectly() {
+  char *p = malloc(1);
+  p++;
+  memcmp(p, p, sizeof(&p));
+  return; // expected-warning {{leak}}
+}
+
+void testPassToSystemHeaderFunctionIndirectly() {
+  int *p = malloc(4);
+  p++;
+  fakeSystemHeaderCallInt(p);
+} // expected-warning {{leak}}
+
 // ----------------------------------------------------------------------------
 // False negatives.
 
@@ -1055,5 +1077,17 @@ void localStructTest() {
   pSt->memP = malloc(12);
 } // missing warning
 
+void testPassConstPointerIndirectlyStruct() {
+  struct HasPtr hp;
+  hp.p = malloc(10);
+  memcmp(&hp, &hp, sizeof(hp));
+  return; // missing leak
+}
+
+void testPassToSystemHeaderFunctionIndirectlyStruct() {
+  SomeStruct ss;
+  ss.p = malloc(1);
+  fakeSystemHeaderCall(&ss);
+} // missing leak
 
 
diff --git a/clang/test/Analysis/simple-stream-checks.c b/clang/test/Analysis/simple-stream-checks.c
index 74794cc..1fb6de3 100644
--- a/clang/test/Analysis/simple-stream-checks.c
+++ b/clang/test/Analysis/simple-stream-checks.c
@@ -76,3 +76,16 @@ void SymbolDoesNotEscapeThoughStringAPIs(char *Data) {
   fputc(*Data, F);
   return; // expected-warning {{Opened file is never closed; potential resource leak}}
 }
+
+void passConstPointer(const FILE * F);
+void testPassConstPointer() {
+  FILE *F = fopen("myfile.txt", "w");
+  passConstPointer(F);
+  return; // expected-warning {{Opened file is never closed; potential resource leak}}
+}
+
+void testPassToSystemHeaderFunctionIndirectly() {
+  FileStruct fs;
+  fs.p = fopen("myfile.txt", "w");
+  fakeSystemHeaderCall(&fs);
+} // expected leak warning
-- 
2.7.4