[cfe-commits] r113524 - in /cfe/trunk: include/clang/Checker/BugReporter/BugReporter.h lib/Checker/BugReporter.cpp test/Analysis/plist-output-alternate.m
Ted Kremenek
kremenek at apple.com
Thu Sep 9 12:05:34 PDT 2010
Author: kremenek
Date: Thu Sep 9 14:05:34 2010
New Revision: 113524
URL: http://llvm.org/viewvc/llvm-project?rev=113524&view=rev
Log:
Use FindReportInEquivalenceClass to identify all the nodes used for the trimmed graph (in BugReporter). This fixes a problem where a leak that happened to occur on both an exit() path and a non-exit() path was getting reported with the exit() path (which users don't care about).
This fixes:
<rdar://problem/8331641> leak reports should not show paths that end with exit() (but ones that don't end with exit())
Modified:
cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h
cfe/trunk/lib/Checker/BugReporter.cpp
cfe/trunk/test/Analysis/plist-output-alternate.m
Modified: cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h?rev=113524&r1=113523&r2=113524&view=diff
==============================================================================
--- cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h Thu Sep 9 14:05:34 2010
@@ -306,7 +306,8 @@
SourceManager& getSourceManager() { return D.getSourceManager(); }
virtual void GeneratePathDiagnostic(PathDiagnostic& PD,
- BugReportEquivClass& EQ) {}
+ BugReportEquivClass& EQ,
+ llvm::SmallVectorImpl<const ExplodedNode*> &Nodes) {}
void Register(BugType *BT);
@@ -368,7 +369,8 @@
GRStateManager &getStateManager();
virtual void GeneratePathDiagnostic(PathDiagnostic& PD,
- BugReportEquivClass& R);
+ BugReportEquivClass& R,
+ llvm::SmallVectorImpl<const ExplodedNode*> &Nodes);
void addNotableSymbol(SymbolRef Sym) {
NotableSymbols.insert(Sym);
Modified: cfe/trunk/lib/Checker/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/BugReporter.cpp?rev=113524&r1=113523&r2=113524&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/BugReporter.cpp (original)
+++ cfe/trunk/lib/Checker/BugReporter.cpp Thu Sep 9 14:05:34 2010
@@ -1567,23 +1567,18 @@
}
void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
- BugReportEquivClass& EQ) {
+ BugReportEquivClass& EQ,
+ llvm::SmallVectorImpl<const ExplodedNode *> &Nodes) {
- std::vector<const ExplodedNode*> Nodes;
- for (BugReportEquivClass::iterator I=EQ.begin(), E=EQ.end(); I!=E; ++I) {
- const ExplodedNode* N = I->getEndNode();
- if (N) Nodes.push_back(N);
- }
-
- if (Nodes.empty())
- return;
+ assert(!Nodes.empty());
// Construct a new graph that contains only a single path from the error
// node to a root.
const std::pair<std::pair<ExplodedGraph*, NodeBackMap*>,
std::pair<ExplodedNode*, unsigned> >&
- GPair = MakeReportGraph(&getGraph(), &Nodes[0], &Nodes[0] + Nodes.size());
+ GPair = MakeReportGraph(&getGraph(), &Nodes[0],
+ Nodes.data() + Nodes.size());
// Find the BugReport with the original location.
BugReport *R = 0;
@@ -1657,21 +1652,36 @@
};
}
-static BugReport *FindReportInEquivalenceClass(BugReportEquivClass& EQ) {
+static BugReport *
+FindReportInEquivalenceClass(BugReportEquivClass& EQ,
+ llvm::SmallVectorImpl<const ExplodedNode*> &Nodes) {
+
BugReportEquivClass::iterator I = EQ.begin(), E = EQ.end();
assert(I != E);
BugReport *R = *I;
BugType& BT = R->getBugType();
-
- if (!BT.isSuppressOnSink())
+
+ // If we don't need to suppress any of the nodes because they are post-dominated
+ // by a sink, simply add all the nodes in the equivalence class to 'Nodes'.
+ if (!BT.isSuppressOnSink()) {
+ for (BugReportEquivClass::iterator I=EQ.begin(), E=EQ.end(); I!=E; ++I) {
+ const ExplodedNode* N = I->getEndNode();
+ if (N) {
+ R = *I;
+ Nodes.push_back(N);
+ }
+ }
return R;
-
+ }
+
// For bug reports that should be suppressed when all paths are post-dominated
// by a sink node, iterate through the reports in the equivalence class
// until we find one that isn't post-dominated (if one exists). We use a
// DFS traversal of the ExplodedGraph to find a non-sink node. We could write
// this as a recursive function, but we don't want to risk blowing out the
// stack for very long paths.
+ BugReport *ExampleReport = 0;
+
for (; I != E; ++I) {
R = *I;
const ExplodedNode *N = R->getEndNode();
@@ -1682,12 +1692,17 @@
if (N->isSink()) {
assert(false &&
"BugType::isSuppressSink() should not be 'true' for sink end nodes");
- return R;
+ return 0;
}
-
- if (N->succ_empty())
- return R;
-
+
+ // No successors? By definition this nodes isn't post-dominated by a sink.
+ if (N->succ_empty()) {
+ Nodes.push_back(N);
+ if (!ExampleReport)
+ ExampleReport = R;
+ continue;
+ }
+
// At this point we know that 'N' is not a sink and it has at least one
// successor. Use a DFS worklist to find a non-sink end-of-path node.
typedef FRIEC_WLItem WLItem;
@@ -1706,15 +1721,17 @@
const ExplodedNode *Succ = *WI.I;
// End-of-path node?
if (Succ->succ_empty()) {
- // If we found an end-of-path node that is not a sink, then return
- // this report.
- if (!Succ->isSink())
- return R;
-
+ // If we found an end-of-path node that is not a sink.
+ if (!Succ->isSink()) {
+ Nodes.push_back(N);
+ if (!ExampleReport)
+ ExampleReport = R;
+ WL.clear();
+ break;
+ }
// Found a sink? Continue on to the next successor.
continue;
}
-
// Mark the successor as visited. If it hasn't been explored,
// enqueue it to the DFS worklist.
unsigned &mark = Visited[Succ];
@@ -1724,17 +1741,18 @@
break;
}
}
-
- if (&WL.back() == &WI)
+
+ // The worklist may have been cleared at this point. First
+ // check if it is empty before checking the last item.
+ if (!WL.empty() && &WL.back() == &WI)
WL.pop_back();
}
}
-
- // If we reach here, the end nodes for all reports in the equivalence
- // class are post-dominated by a sink node.
- return NULL;
-}
+ // ExampleReport will be NULL if all the nodes in the equivalence class
+ // were post-dominated by sinks.
+ return ExampleReport;
+}
//===----------------------------------------------------------------------===//
// DiagnosticCache. This is a hack to cache analyzer diagnostics. It
@@ -1780,7 +1798,8 @@
}
void BugReporter::FlushReport(BugReportEquivClass& EQ) {
- BugReport *R = FindReportInEquivalenceClass(EQ);
+ llvm::SmallVector<const ExplodedNode*, 10> Nodes;
+ BugReport *R = FindReportInEquivalenceClass(EQ, Nodes);
if (!R)
return;
@@ -1797,7 +1816,8 @@
? R->getDescription() : R->getShortDescription(),
BT.getCategory()));
- GeneratePathDiagnostic(*D.get(), EQ);
+ if (!Nodes.empty())
+ GeneratePathDiagnostic(*D.get(), EQ, Nodes);
if (IsCachedDiagnostic(R, D.get()))
return;
Modified: cfe/trunk/test/Analysis/plist-output-alternate.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/plist-output-alternate.m?rev=113524&r1=113523&r2=113524&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/plist-output-alternate.m (original)
+++ cfe/trunk/test/Analysis/plist-output-alternate.m Thu Sep 9 14:05:34 2010
@@ -37,6 +37,25 @@
*(x.p) = 0xDEADBEEF;
}
+// <rdar://problem/8331641> leak reports should not show paths that end with exit() (but ones that don't end with exit())
+void panic() __attribute__((noreturn));
+enum { kCFNumberSInt8Type = 1, kCFNumberSInt16Type = 2, kCFNumberSInt32Type = 3, kCFNumberSInt64Type = 4, kCFNumberFloat32Type = 5, kCFNumberFloat64Type = 6, kCFNumberCharType = 7, kCFNumberShortType = 8, kCFNumberIntType = 9, kCFNumberLongType = 10, kCFNumberLongLongType = 11, kCFNumberFloatType = 12, kCFNumberDoubleType = 13, kCFNumberCFIndexType = 14, kCFNumberNSIntegerType = 15, kCFNumberCGFloatType = 16, kCFNumberMaxType = 16 };
+typedef const struct __CFAllocator * CFAllocatorRef;
+extern const CFAllocatorRef kCFAllocatorDefault;
+typedef signed long CFIndex;
+typedef CFIndex CFNumberType;
+typedef const struct __CFNumber * CFNumberRef;
+
+extern CFNumberRef CFNumberCreate(CFAllocatorRef allocator, CFNumberType theType, const void *valuePtr);
+
+void rdar8331641(int x) {
+ signed z = 1;
+ CFNumberRef value = CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &z); // expected-warning{{leak}}
+ if (x)
+ panic();
+ (void) value;
+}
+
// CHECK: <?xml version="1.0" encoding="UTF-8"?>
// CHECK: <!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
// CHECK: <plist version="1.0">
@@ -750,7 +769,246 @@
// CHECK: <key>file</key><integer>0</integer>
// CHECK: </dict>
// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>path</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>kind</key><string>control</string>
+// CHECK: <key>edges</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>start</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>52</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>52</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: <key>end</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>kind</key><string>control</string>
+// CHECK: <key>edges</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>start</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: <key>end</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>23</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>82</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>kind</key><string>event</string>
+// CHECK: <key>location</key>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>23</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <key>ranges</key>
+// CHECK: <array>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>23</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>82</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </array>
+// CHECK: <key>extended_message</key>
+// CHECK: <string>Call to function 'CFNumberCreate' returns a Core Foundation object with a +1 retain count (owning reference)</string>
+// CHECK: <key>message</key>
+// CHECK: <string>Call to function 'CFNumberCreate' returns a Core Foundation object with a +1 retain count (owning reference)</string>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>kind</key><string>control</string>
+// CHECK: <key>edges</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>start</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>23</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>53</integer>
+// CHECK: <key>col</key><integer>82</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: <key>end</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>54</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>54</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>kind</key><string>control</string>
+// CHECK: <key>edges</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>start</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>54</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>54</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: <key>end</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>56</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>56</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>kind</key><string>control</string>
+// CHECK: <key>edges</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>start</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>56</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>56</integer>
+// CHECK: <key>col</key><integer>3</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: <key>end</key>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>57</integer>
+// CHECK: <key>col</key><integer>1</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>57</integer>
+// CHECK: <key>col</key><integer>1</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>kind</key><string>event</string>
+// CHECK: <key>location</key>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>57</integer>
+// CHECK: <key>col</key><integer>1</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <key>ranges</key>
+// CHECK: <array>
+// CHECK: <array>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>57</integer>
+// CHECK: <key>col</key><integer>1</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>57</integer>
+// CHECK: <key>col</key><integer>1</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: </array>
+// CHECK: <key>extended_message</key>
+// CHECK: <string>Object allocated on line 53 and stored into 'value' is no longer referenced after this point and has a retain count of +1 (object leaked)</string>
+// CHECK: <key>message</key>
+// CHECK: <string>Object allocated on line 53 and stored into 'value' is no longer referenced after this point and has a retain count of +1 (object leaked)</string>
+// CHECK: </dict>
+// CHECK: </array>
+// CHECK: <key>description</key><string>Potential leak of an object allocated on line 53 and stored into 'value'</string>
+// CHECK: <key>category</key><string>Memory (Core Foundation/Objective-C)</string>
+// CHECK: <key>type</key><string>Leak of returned object</string>
+// CHECK: <key>location</key>
+// CHECK: <dict>
+// CHECK: <key>line</key><integer>57</integer>
+// CHECK: <key>col</key><integer>1</integer>
+// CHECK: <key>file</key><integer>0</integer>
+// CHECK: </dict>
+// CHECK: </dict>
// CHECK: </array>
// CHECK: </dict>
// CHECK: </plist>
-
More information about the cfe-commits
mailing list