[PATCH] D37437: [analyzer] Fix SimpleStreamChecker's output plist not containing the checker name

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Cool. Thanks!

> In the future probably it would be better to alter the signature of the 
> checkers' constructor to set the name in the constructor so it is possible to 
> create the BugType eagerly.

Still, should we add an assertion so that we could be sure that every bug type 
contains a checker name?


Repository:
  rL LLVM

https://reviews.llvm.org/D37437



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37437: [analyzer] Fix SimpleStreamChecker's output plist not containing the checker name

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.
Herald added subscribers: baloghadamsoftware, whisperity.

Unfortunately, currently, the analyzer core sets the checker name after the 
constructor was already run. So if we set the BugType in the constructor, the 
output plist will not contain a checker name.  Right now the idiomatic solution 
is to create the BugType lazily. This patch updates the SimpleStreamChecker to 
follow this idiom.

In the future probably it would be better to alter the signature of the 
checkers and set the checker name in the constructor so it is possible to 
create the BugType eagerly.


Repository:
  rL LLVM

https://reviews.llvm.org/D37437

Files:
  lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp


Index: lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
+++ lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
@@ -54,8 +54,8 @@
check::PointerEscape> {
   CallDescription OpenFn, CloseFn;
 
-  std::unique_ptr DoubleCloseBugType;
-  std::unique_ptr LeakBugType;
+  mutable std::unique_ptr DoubleCloseBugType;
+  mutable std::unique_ptr LeakBugType;
 
   void reportDoubleClose(SymbolRef FileDescSym,
  const CallEvent &Call,
@@ -104,16 +104,7 @@
 } // end anonymous namespace
 
 SimpleStreamChecker::SimpleStreamChecker()
-: OpenFn("fopen"), CloseFn("fclose", 1) {
-  // Initialize the bug types.
-  DoubleCloseBugType.reset(
-  new BugType(this, "Double fclose", "Unix Stream API Error"));
-
-  LeakBugType.reset(
-  new BugType(this, "Resource Leak", "Unix Stream API Error"));
-  // Sinks are higher importance bugs as well as calls to assert() or exit(0).
-  LeakBugType->setSuppressOnSink(true);
-}
+: OpenFn("fopen"), CloseFn("fclose", 1) {}
 
 void SimpleStreamChecker::checkPostCall(const CallEvent &Call,
 CheckerContext &C) const {
@@ -206,6 +197,10 @@
   if (!ErrNode)
 return;
 
+  if (!DoubleCloseBugType)
+DoubleCloseBugType.reset(
+new BugType(this, "Double fclose", "Unix Stream API Error"));
+
   // Generate the report.
   auto R = llvm::make_unique(*DoubleCloseBugType,
   "Closing a previously closed file stream", ErrNode);
@@ -217,6 +212,13 @@
 void SimpleStreamChecker::reportLeaks(ArrayRef LeakedStreams,
   CheckerContext &C,
   ExplodedNode *ErrNode) const {
+
+  if (!LeakBugType) {
+LeakBugType.reset(
+new BugType(this, "Resource Leak", "Unix Stream API Error"));
+// Sinks are higher importance bugs as well as calls to assert() or 
exit(0).
+LeakBugType->setSuppressOnSink(true);
+  }
   // Attach bug reports to the leak node.
   // TODO: Identify the leaked file descriptor.
   for (SymbolRef LeakedStream : LeakedStreams) {


Index: lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
+++ lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
@@ -54,8 +54,8 @@
check::PointerEscape> {
   CallDescription OpenFn, CloseFn;
 
-  std::unique_ptr DoubleCloseBugType;
-  std::unique_ptr LeakBugType;
+  mutable std::unique_ptr DoubleCloseBugType;
+  mutable std::unique_ptr LeakBugType;
 
   void reportDoubleClose(SymbolRef FileDescSym,
  const CallEvent &Call,
@@ -104,16 +104,7 @@
 } // end anonymous namespace
 
 SimpleStreamChecker::SimpleStreamChecker()
-: OpenFn("fopen"), CloseFn("fclose", 1) {
-  // Initialize the bug types.
-  DoubleCloseBugType.reset(
-  new BugType(this, "Double fclose", "Unix Stream API Error"));
-
-  LeakBugType.reset(
-  new BugType(this, "Resource Leak", "Unix Stream API Error"));
-  // Sinks are higher importance bugs as well as calls to assert() or exit(0).
-  LeakBugType->setSuppressOnSink(true);
-}
+: OpenFn("fopen"), CloseFn("fclose", 1) {}
 
 void SimpleStreamChecker::checkPostCall(const CallEvent &Call,
 CheckerContext &C) const {
@@ -206,6 +197,10 @@
   if (!ErrNode)
 return;
 
+  if (!DoubleCloseBugType)
+DoubleCloseBugType.reset(
+new BugType(this, "Double fclose", "Unix Stream API Error"));
+
   // Generate the report.
   auto R = llvm::make_unique(*DoubleCloseBugType,
   "Closing a previously closed file stream", ErrNode);
@@ -217,6 +212,13 @@
 void SimpleStreamChecker::reportLeaks(ArrayRef LeakedStreams,
   CheckerContext &C,
   ExplodedNode *ErrNode) const {
+
+  if (!LeakBugType) {
+LeakBugType.reset(
+new BugType(this, "Resource Leak", "Unix Stream API Error"));
+// Sinks are higher importance bugs as well as calls to assert() or exit(0).
+LeakBugType->