[analyzer] [NFC] Simplify some visitors by giving a convenient getter from state to analysis manager

[george.karpenkov]

@NoQ should be fine to commit?

[NoQ]

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

289–290 ↗

(On Diff #157178)

You can easily obtain SM from Ctx. Moreover, in fact Ctx can be obtained from R->getMemRegionManager() (i've no idea why do we keep a manager pointer in every region).

clang/lib/StaticAnalyzer/Core/ProgramState.cpp

500 ↗

(On Diff #157178)

Unnecessary whitespace.

[NoQ]

cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

739

Btw why isn't this field private? I guess nobody cares because it's anyway exposed as a mutable reference, but we might have as well had consistency.

[george.karpenkov]

cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

739

yeah that's how we always got it. No idea why.

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

 class APSInt;
 }
 
 namespace clang {
 class ASTContext;
 
 namespace ento {
 
+class AnalysisManager;
 class CallEvent;
 class CallEventManager;
 
 typedef std::unique_ptr<ConstraintManager>(*ConstraintManagerCreator)(
     ProgramStateManager &, SubEngine *);
 typedef std::unique_ptr<StoreManager>(*StoreManagerCreator)(
     ProgramStateManager &);
 typedef llvm::ImmutableMap<const SubRegion*, TaintTagType> TaintedSubRegions;
public:
 
   ~ProgramState();
 
   /// Return the ProgramStateManager associated with this state.
   ProgramStateManager &getStateManager() const {
     return *stateMgr;
   }
 
+  AnalysisManager &getAnalysisManager() const;
+
   /// Return the ConstraintManager.
   ConstraintManager &getConstraintManager() const;
 
   /// getEnvironment - Return the environment associated with this state.
   ///  The environment is the mapping from expressions to values.
   const Environment& getEnvironment() const { return Env; }
 
   /// Return the store associated with this state.  The store

cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

 namespace {
 
 /// Put a diagnostic on return statement of all inlined functions
 /// for which  the region of interest \p RegionOfInterest was passed into,
 /// but not written inside, and it has caused an undefined read or a null
 /// pointer dereference outside.
 class NoStoreFuncVisitor final : public BugReporterVisitor {
   const SubRegion *RegionOfInterest;
+  const SourceManager &SM;
+  const PrintingPolicy &PP;
   static constexpr const char *DiagnosticsMsg =
       "Returning without writing to '";
 
   /// Frames writing into \c RegionOfInterest.
   /// This visitor generates a note only if a function does not write into
   /// a region of interest. This information is not immediately available
   /// by looking at the node associated with the exit from the function
   /// (usually the return statement). To avoid recomputing the same information
   /// many times (going up the path for each node and checking whether the
   /// region was written into) we instead lazily compute the
   /// stack frames along the path which write into the region of interest.
   llvm::SmallPtrSet<const StackFrameContext *, 32> FramesModifyingRegion;
   llvm::SmallPtrSet<const StackFrameContext *, 32> FramesModifyingCalculated;
 
 public:
-  NoStoreFuncVisitor(const SubRegion *R) : RegionOfInterest(R) {}
+  NoStoreFuncVisitor(const SubRegion *R)
+      : RegionOfInterest(R),
+        SM(R->getMemRegionManager()->getContext().getSourceManager()),
+        PP(R->getMemRegionManager()->getContext().getPrintingPolicy()) {}
 
   void Profile(llvm::FoldingSetNodeID &ID) const override {
     static int Tag = 0;
     ID.AddPointer(&Tag);
   }
 
   std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
                                                  const ExplodedNode *PrevN,
                                                  BugReporterContext &BRC,
                                                  BugReport &BR) override {
 
     const LocationContext *Ctx = N->getLocationContext();
     const StackFrameContext *SCtx = Ctx->getStackFrame();
     ProgramStateRef State = N->getState();
     auto CallExitLoc = N->getLocationAs<CallExitBegin>();
 
     // No diagnostic if region was modified inside the frame.
     if (!CallExitLoc)
       return nullptr;
 
     CallEventRef<> Call =
         BRC.getStateManager().getCallEventManager().getCaller(SCtx, State);
-    const PrintingPolicy &PP = BRC.getASTContext().getPrintingPolicy();
-    const SourceManager &SM = BRC.getSourceManager();
 
     // Region of interest corresponds to an IVar, exiting a method
     // which could have written into that IVar, but did not.
     if (const auto *MC = dyn_cast<ObjCMethodCall>(Call))
       if (const auto *IvarR = dyn_cast<ObjCIvarRegion>(RegionOfInterest))
         if (potentiallyWritesIntoIvar(Call->getRuntimeDefinition().getDecl(),
                                       IvarR->getDecl()) &&
             !isRegionOfInterestModifiedInFrame(N))
           return notModifiedMemberDiagnostics(
-              Ctx, SM, PP, *CallExitLoc, Call,
-              MC->getReceiverSVal().getAsRegion());
+              Ctx, *CallExitLoc, Call, MC->getReceiverSVal().getAsRegion());
 
     if (const auto *CCall = dyn_cast<CXXConstructorCall>(Call)) {
-      const MemRegion *ThisRegion = CCall->getCXXThisVal().getAsRegion();
-      if (RegionOfInterest->isSubRegionOf(ThisRegion)
+      const MemRegion *ThisR = CCall->getCXXThisVal().getAsRegion();
+      if (RegionOfInterest->isSubRegionOf(ThisR)
           && !CCall->getDecl()->isImplicit()
           && !isRegionOfInterestModifiedInFrame(N))
-        return notModifiedMemberDiagnostics(Ctx, SM, PP, *CallExitLoc,
-                                                   CCall, ThisRegion);
+        return notModifiedMemberDiagnostics(Ctx, *CallExitLoc, Call, ThisR);
     }
 
     ArrayRef<ParmVarDecl *> parameters = getCallParameters(Call);
     for (unsigned I = 0; I < Call->getNumArgs() && I < parameters.size(); ++I) {
       const ParmVarDecl *PVD = parameters[I];
       SVal S = Call->getArgSVal(I);
       unsigned IndirectionLevel = 1;
       QualType T = PVD->getType();
       while (const MemRegion *R = S.getAsRegion()) {
         if (RegionOfInterest->isSubRegionOf(R)
             && !isPointerToConst(PVD->getType())) {
 
           if (isRegionOfInterestModifiedInFrame(N))
             return nullptr;
 
           return notModifiedParameterDiagnostics(
-              Ctx, SM, PP, *CallExitLoc, Call, PVD, R, IndirectionLevel);
+              Ctx, *CallExitLoc, Call, PVD, R, IndirectionLevel);
         }
         QualType PT = T->getPointeeType();
         if (PT.isNull() || PT->isVoidType()) break;
         S = State->getSVal(R, PT);
         T = PT;
         IndirectionLevel++;
       }
     }
bool isPointerToConst(QualType Ty) {
     return !Ty->getPointeeType().isNull() &&
            Ty->getPointeeType().getCanonicalType().isConstQualified();
   }
 
   /// \return Diagnostics piece for the member field not modified
   /// in a given function.
   std::shared_ptr<PathDiagnosticPiece> notModifiedMemberDiagnostics(
       const LocationContext *Ctx,
-      const SourceManager &SM,
-      const PrintingPolicy &PP,
       CallExitBegin &CallExitLoc,
       CallEventRef<> Call,
       const MemRegion *ArgRegion) {
     const char *TopRegionName = isa<ObjCMethodCall>(Call) ? "self" : "this";
     SmallString<256> sbuf;
     llvm::raw_svector_ostream os(sbuf);
     os << DiagnosticsMsg;
     bool out = prettyPrintRegionName(TopRegionName, "->", /*IsReference=*/true,
private:
   }
 
   /// \return Diagnostics piece for the parameter \p PVD not modified
   /// in a given function.
   /// \p IndirectionLevel How many times \c ArgRegion has to be dereferenced
   /// before we get to the super region of \c RegionOfInterest
   std::shared_ptr<PathDiagnosticPiece>
   notModifiedParameterDiagnostics(const LocationContext *Ctx,
-                         const SourceManager &SM,
-                         const PrintingPolicy &PP,
                          CallExitBegin &CallExitLoc,
                          CallEventRef<> Call,
                          const ParmVarDecl *PVD,
                          const MemRegion *ArgRegion,
                          unsigned IndirectionLevel) {
     PathDiagnosticLocation L = getPathDiagnosticLocation(
         CallExitLoc.getReturnStmt(), SM, Ctx, Call);
     SmallString<256> sbuf;
std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
 
     return nullptr;
   }
 
   static void addMacroVisitorIfNecessary(
         const ExplodedNode *N, const MemRegion *R,
         bool EnableNullFPSuppression, BugReport &BR,
         const SVal V) {
-    AnalyzerOptions &Options = N->getState()->getStateManager()
-        .getOwningEngine()->getAnalysisManager().options;
+    AnalyzerOptions &Options = N->getState()->getAnalysisManager().options;
     if (EnableNullFPSuppression && Options.shouldSuppressNullReturnPaths()
           && V.getAs<Loc>())
       BR.addVisitor(llvm::make_unique<MacroNullReturnSuppressionVisitor>(
               R->getAs<SubRegion>(), V));
   }
 
   void* getTag() const {
     static int Tag = 0;
static void addVisitorIfNecessary(const ExplodedNode *Node, const Stmt *S,
     SVal RetVal = Node->getSVal(S);
 
     // Handle cases where a reference is returned and then immediately used.
     if (cast<Expr>(S)->isGLValue())
       if (Optional<Loc> LValue = RetVal.getAs<Loc>())
         RetVal = State->getSVal(*LValue);
 
     // See if the return value is NULL. If so, suppress the report.
-    SubEngine *Eng = State->getStateManager().getOwningEngine();
-    assert(Eng && "Cannot file a bug report without an owning engine");
-    AnalyzerOptions &Options = Eng->getAnalysisManager().options;
+    AnalyzerOptions &Options = State->getAnalysisManager().options;
 
     bool EnableNullFPSuppression = false;
     if (InEnableNullFPSuppression && Options.shouldSuppressNullReturnPaths())
       if (Optional<Loc> RetLoc = RetVal.getAs<Loc>())
         EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
 
     BR.markInteresting(CalleeContext);
     BR.addVisitor(llvm::make_unique<ReturnVisitor>(CalleeContext,
TrackConstraintBRVisitor::VisitNode(const ExplodedNode *N,
 
   return nullptr;
 }
 
 SuppressInlineDefensiveChecksVisitor::
 SuppressInlineDefensiveChecksVisitor(DefinedSVal Value, const ExplodedNode *N)
     : V(Value) {
   // Check if the visitor is disabled.
-  SubEngine *Eng = N->getState()->getStateManager().getOwningEngine();
-  assert(Eng && "Cannot file a bug report without an owning engine");
-  AnalyzerOptions &Options = Eng->getAnalysisManager().options;
+  AnalyzerOptions &Options = N->getState()->getAnalysisManager().options;
   if (!Options.shouldSuppressInlinedDefensiveChecks())
     IsSatisfied = true;
 
   assert(N->getState()->isNull(V).isConstrainedTrue() &&
          "The visitor only tracks the cases where V is constrained to 0");
 }
 
 void SuppressInlineDefensiveChecksVisitor::Profile(
if (Inner && ExplodedGraph::isInterestingLValueExpr(Inner)) {
     // In case of C++ references, we want to differentiate between a null
     // reference and reference to null pointer.
     // If the LVal is null, check if we are dealing with null reference.
     // For those, we want to track the location of the reference.
     const MemRegion *R = (RR && LVIsNull) ? RR :
         LVNode->getSVal(Inner).getAsRegion();
 
     if (R) {
+
       // Mark both the variable region and its contents as interesting.
       SVal V = LVState->getRawSVal(loc::MemRegionVal(R));
       report.addVisitor(
           llvm::make_unique<NoStoreFuncVisitor>(cast<SubRegion>(R)));
 
       MacroNullReturnSuppressionVisitor::addMacroVisitorIfNecessary(
           N, R, EnableNullFPSuppression, report, V);
 

cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp

 //= ProgramState.cpp - Path-Sensitive "State" for tracking values --*- C++ -*--=
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements ProgramState and ProgramStateManager.
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h"
 #include "llvm/Support/raw_ostream.h"
for (TaintMapImpl::iterator I = TM.begin(), E = TM.end(); I != E; ++I) {
     Out << I->first << " : " << I->second << NL;
   }
 }
 
 void ProgramState::dumpTaint() const {
   printTaint(llvm::errs());
 }
 
+AnalysisManager& ProgramState::getAnalysisManager() const {
+  return stateMgr->getOwningEngine()->getAnalysisManager();
+}
+
 //===----------------------------------------------------------------------===//
 // Generic Data Map.
 //===----------------------------------------------------------------------===//
 
 void *const* ProgramState::FindGDM(void *K) const {
   return GDM.lookup(K);
 }