[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!
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174677 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/StaticAnalyzer/Core/Checker.h b/include/clang/StaticAnalyzer/Core/Checker.h
index a190e1f..82bac7d 100644
--- a/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/include/clang/StaticAnalyzer/Core/Checker.h
@@ -323,10 +323,12 @@
_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/include/clang/StaticAnalyzer/Core/CheckerManager.h b/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 2fd71dd..4353ebf 100644
--- a/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -112,6 +112,26 @@
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 @@
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 @@
typedef CheckerFn<ProgramStateRef (ProgramStateRef,
const InvalidatedSymbols &Escaped,
- const CallEvent *Call)>
+ const CallEvent *Call,
+ PointerEscapeKind Kind)>
CheckPointerEscapeFunc;
typedef CheckerFn<ProgramStateRef (ProgramStateRef,
diff --git a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
index 35baef6..ed89788 100644
--- a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
@@ -265,10 +265,12 @@
/// \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/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 33553a1..9afd8cf 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -158,7 +158,8 @@
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 @@
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/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
index e2a19fc..1ccf339 100644
--- a/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
@@ -82,7 +82,8 @@
/// 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 @@
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/lib/StaticAnalyzer/Core/CheckerManager.cpp b/lib/StaticAnalyzer/Core/CheckerManager.cpp
index dc0cab7..a383799 100644
--- a/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -488,13 +488,19 @@
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/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 636aa3b..f092f1a 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1648,7 +1648,8 @@
const InvalidatedSymbols &EscapedSymbols = Scanner.getSymbols();
State = getCheckerManager().runCheckersForPointerEscape(State,
EscapedSymbols,
- /*CallEvent*/ 0);
+ /*CallEvent*/ 0,
+ PSK_EscapeOnBind);
return State;
}
@@ -1665,7 +1666,9 @@
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 @@
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/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h b/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
index f08f3f6..b65b7a6 100644
--- a/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
+++ b/test/Analysis/Inputs/system-header-simulator-for-simple-stream.h
@@ -13,3 +13,9 @@
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/test/Analysis/Inputs/system-header-simulator.h b/test/Analysis/Inputs/system-header-simulator.h
index 4c12131..04688c7 100644
--- a/test/Analysis/Inputs/system-header-simulator.h
+++ b/test/Analysis/Inputs/system-header-simulator.h
@@ -67,3 +67,11 @@
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/test/Analysis/malloc.c b/test/Analysis/malloc.c
index ed2d8e9..9dc17e6 100644
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -13,6 +13,7 @@
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 @@
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 @@
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/test/Analysis/simple-stream-checks.c b/test/Analysis/simple-stream-checks.c
index 74794cc..1fb6de3 100644
--- a/test/Analysis/simple-stream-checks.c
+++ b/test/Analysis/simple-stream-checks.c
@@ -76,3 +76,16 @@
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