[Analyzer][StreamChecker] Updated initialization of BugType's.

[balazske]

Summary

BugType objects are initialized in-class instead of by lazy initialization.
FuchsiaHandleChecker does this already.

[balazske]

Why is BuiltinBug used instead of BugType? The differentiation between these classes looks not obvious. There is even a non-virtual getDescription function in both classes and a confusion with meaning of "name" and "description".

[Szelethus]

Yay.

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

ProgramStateRef bindInt(uint64_t Value, ProgramStateRef State,
                         CheckerContext &C, const CallExpr *CE) {
   State = State->BindExpr(CE, C.getLocationContext(),
                           C.getSValBuilder().makeIntVal(Value, false));
   return State;
 }
 
 class StreamChecker
     : public Checker<check::PreCall, eval::Call, check::DeadSymbols> {
-  mutable std::unique_ptr<BuiltinBug> BT_nullfp, BT_illegalwhence,
-      BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof,
-      BT_IndeterminatePosition;
+  BuiltinBug BT_FileNull{this, "NULL stream pointer",
+                         "Stream pointer might be NULL."};
+  BuiltinBug BT_UseAfterClose{
+      this, "Closed stream",
+      "Stream might be already closed. Causes undefined behaviour."};
+  BuiltinBug BT_UseAfterOpenFailed{this, "Invalid stream",
+                                   "Stream might be invalid after "
+                                   "(re-)opening it has failed. "
+                                   "Can cause undefined behaviour."};
+  BuiltinBug BT_IndeterminatePosition{
+      this, "Invalid stream state",
+      "File position of the stream might be 'indeterminate' "
+      "after a failed operation. "
+      "Can cause undefined behavior."};
+  BuiltinBug BT_IllegalWhence{this, "Illegal whence argument",
+                              "The whence argument to fseek() should be "
+                              "SEEK_SET, SEEK_END, or SEEK_CUR."};
+  BuiltinBug BT_StreamEof{this, "Stream already in EOF",
+                          "Read function called when stream is in EOF state. "
+                          "Function has no effect."};
+  BuiltinBug BT_ResourceLeak{
+      this, "Resource Leak",
+      "Opened File never closed. Potential Resource leak."};
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 
   /// If true, evaluate special testing stream functions.
   bool TestMode = false;
StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
 
   ConstraintManager &CM = C.getConstraintManager();
 
   ProgramStateRef StateNotNull, StateNull;
   std::tie(StateNotNull, StateNull) = CM.assumeDual(C.getState(), *Stream);
 
   if (!StateNotNull && StateNull) {
     if (ExplodedNode *N = C.generateErrorNode(StateNull)) {
-      if (!BT_nullfp)
-        BT_nullfp.reset(new BuiltinBug(this, "NULL stream pointer",
-                                       "Stream pointer might be NULL."));
       C.emitReport(std::make_unique<PathSensitiveBugReport>(
-          *BT_nullfp, BT_nullfp->getDescription(), N));
+          BT_FileNull, BT_FileNull.getDescription(), N));
     }
     return nullptr;
   }
 
   return StateNotNull;
 }
 
 ProgramStateRef StreamChecker::ensureStreamOpened(SVal StreamVal,
                                                   CheckerContext &C,
                                                   ProgramStateRef State) const {
   SymbolRef Sym = StreamVal.getAsSymbol();
   if (!Sym)
     return State;
 
   const StreamState *SS = State->get<StreamMap>(Sym);
   if (!SS)
     return State;
 
   if (SS->isClosed()) {
     // Using a stream pointer after 'fclose' causes undefined behavior
     // according to cppreference.com .
     ExplodedNode *N = C.generateErrorNode();
     if (N) {
-      if (!BT_UseAfterClose)
-        BT_UseAfterClose.reset(new BuiltinBug(this, "Closed stream",
-                                              "Stream might be already closed. "
-                                              "Causes undefined behaviour."));
       C.emitReport(std::make_unique<PathSensitiveBugReport>(
-          *BT_UseAfterClose, BT_UseAfterClose->getDescription(), N));
+          BT_UseAfterClose, BT_UseAfterClose.getDescription(), N));
       return nullptr;
     }
 
     return State;
   }
 
   if (SS->isOpenFailed()) {
     // Using a stream that has failed to open is likely to cause problems.
     // This should usually not occur because stream pointer is NULL.
     // But freopen can cause a state when stream pointer remains non-null but
     // failed to open.
     ExplodedNode *N = C.generateErrorNode();
     if (N) {
-      if (!BT_UseAfterOpenFailed)
-        BT_UseAfterOpenFailed.reset(
-            new BuiltinBug(this, "Invalid stream",
-                           "Stream might be invalid after "
-                           "(re-)opening it has failed. "
-                           "Can cause undefined behaviour."));
       C.emitReport(std::make_unique<PathSensitiveBugReport>(
-          *BT_UseAfterOpenFailed, BT_UseAfterOpenFailed->getDescription(), N));
+          BT_UseAfterOpenFailed, BT_UseAfterOpenFailed.getDescription(), N));
       return nullptr;
     }
     return State;
   }
 
   return State;
 }
 
 ProgramStateRef StreamChecker::ensureNoFilePositionIndeterminate(
     SVal StreamVal, CheckerContext &C, ProgramStateRef State) const {
   SymbolRef Sym = StreamVal.getAsSymbol();
   if (!Sym)
     return State;
 
   const StreamState *SS = State->get<StreamMap>(Sym);
   if (!SS)
     return State;
 
   assert(SS->isOpened() && "First ensure that stream is opened.");
 
   if (SS->FilePositionIndeterminate) {
-    if (!BT_IndeterminatePosition)
-      BT_IndeterminatePosition.reset(
-          new BuiltinBug(this, "Invalid stream state",
-                         "File position of the stream might be 'indeterminate' "
-                         "after a failed operation. "
-                         "Can cause undefined behavior."));
-
     if (SS->ErrorState & ErrorFEof) {
       // The error is unknown but may be FEOF.
       // Continue analysis with the FEOF error state.
       // Report warning because the other possible error states.
       ExplodedNode *N = C.generateNonFatalErrorNode(State);
       if (!N)
         return nullptr;
 
       C.emitReport(std::make_unique<PathSensitiveBugReport>(
-          *BT_IndeterminatePosition, BT_IndeterminatePosition->getDescription(),
+          BT_IndeterminatePosition, BT_IndeterminatePosition.getDescription(),
           N));
       return State->set<StreamMap>(
           Sym, StreamState::getOpened(SS->LastOperation, ErrorFEof, false));
     }
 
     // Known or unknown error state without FEOF possible.
     // Stop analysis, report error.
     ExplodedNode *N = C.generateErrorNode(State);
     if (N)
       C.emitReport(std::make_unique<PathSensitiveBugReport>(
-          *BT_IndeterminatePosition, BT_IndeterminatePosition->getDescription(),
+          BT_IndeterminatePosition, BT_IndeterminatePosition.getDescription(),
           N));
 
     return nullptr;
   }
 
   return State;
 }
 
 ProgramStateRef
 StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
                                         ProgramStateRef State) const {
   Optional<nonloc::ConcreteInt> CI = WhenceVal.getAs<nonloc::ConcreteInt>();
   if (!CI)
     return State;
 
   int64_t X = CI->getValue().getSExtValue();
   if (X >= 0 && X <= 2)
     return State;
 
   if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
-    if (!BT_illegalwhence)
-      BT_illegalwhence.reset(
-          new BuiltinBug(this, "Illegal whence argument",
-                         "The whence argument to fseek() should be "
-                         "SEEK_SET, SEEK_END, or SEEK_CUR."));
     C.emitReport(std::make_unique<PathSensitiveBugReport>(
-        *BT_illegalwhence, BT_illegalwhence->getDescription(), N));
+        BT_IllegalWhence, BT_IllegalWhence.getDescription(), N));
     return nullptr;
   }
 
   return State;
 }
 
 void StreamChecker::reportFEofWarning(CheckerContext &C,
                                       ProgramStateRef State) const {
   if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
-    if (!BT_StreamEof)
-      BT_StreamEof.reset(
-          new BuiltinBug(this, "Stream already in EOF",
-                         "Read function called when stream is in EOF state. "
-                         "Function has no effect."));
     C.emitReport(std::make_unique<PathSensitiveBugReport>(
-        *BT_StreamEof, BT_StreamEof->getDescription(), N));
+        BT_StreamEof, BT_StreamEof.getDescription(), N));
     return;
   }
   C.addTransition(State);
 }
 
 void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                      CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
   // TODO: Clean up the state.
   const StreamMapTy &Map = State->get<StreamMap>();
   for (const auto &I : Map) {
     SymbolRef Sym = I.first;
     const StreamState &SS = I.second;
     if (!SymReaper.isDead(Sym) || !SS.isOpened())
       continue;
 
     ExplodedNode *N = C.generateErrorNode();
     if (!N)
       continue;
 
-    if (!BT_ResourceLeak)
-      BT_ResourceLeak.reset(
-          new BuiltinBug(this, "Resource Leak",
-                         "Opened File never closed. Potential Resource leak."));
     C.emitReport(std::make_unique<PathSensitiveBugReport>(
-        *BT_ResourceLeak, BT_ResourceLeak->getDescription(), N));
+        BT_ResourceLeak, BT_ResourceLeak.getDescription(), N));
   }
 }
 
 void ento::registerStreamChecker(CheckerManager &Mgr) {
   Mgr.registerChecker<StreamChecker>();
 }
 
 bool ento::shouldRegisterStreamChecker(const CheckerManager &Mgr) {