[analyzer] CheckerContext: Make the Preprocessor available

[Charusso]

Summary

This patch hooks the Preprocessor trough BugReporter to the
CheckerContext so the checkers could look for macro definitions.

[Charusso]

It is needed for my work, and also I have seen other checkers in need of that.

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

196

I have dropped the const because the methods of Preprocessor are non-const because of the BumpPtrAllocator.

[NoQ]

We probably need this because every time we try to deal with macros we struggle quite a bit.

I'm not sure though - because we somehow survived without this for like 10 years. Eg. BugReporterVisitors.cpp:

250 static bool isFunctionMacroExpansion(SourceLocation Loc,
251                                 const SourceManager &SM) {
252   if (!Loc.isMacroID())
253     return false;
254   while (SM.isMacroArgExpansion(Loc))
255     Loc = SM.getImmediateExpansionRange(Loc).getBegin();
256   std::pair<FileID, unsigned> TLInfo = SM.getDecomposedLoc(Loc);
257   SrcMgr::SLocEntry SE = SM.getSLocEntry(TLInfo.first);
258   const SrcMgr::ExpansionInfo &EInfo = SE.getExpansion();
259   return EInfo.isFunctionMacroExpansion();
260 }

I'd love to see some actual use before committing.

[NoQ]

Also path-insensitive checkers will probably benefit a lot more from this info.

[Charusso]

In D69731#1730784, @NoQ wrote:

I'm not sure though - because we somehow survived without this for like 10 years. Eg. BugReporterVisitors.cpp: [...]
I'd love to see some actual use before committing.

"Teaser":

const Preprocessor &PP = C.getPreprocessor();
Optional<bool> WantSafeFunctions;
  
if (PP.isMacroDefined("__STDC_LIB_EXT1__")) {
  MacroDefinition MD = PP.getMacroDefinition("__STDC_WANT_LIB_EXT1__");
  if (const MacroInfo *MI = MD.getMacroInfo()) {
    const Token &T = MI->tokens().back();
    StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
    llvm::APInt IntValue;
    ValueStr.getAsInteger(10, IntValue);
    WantSafeFunctions = IntValue.getZExtValue();
  }
}

[NoQ]

It sounds like the code is querying the temporary internal state of the preprocessor which is supposed to be queried during preprocessing, not after it. Say, PP.isMacroDefined("__STDC_LIB_EXT1__") clearly depends on the location, as a macro can be un-defined and re-defined as many times as possible. But it doesn't receive the location as an argument, so it's probably implied to be "the location that the preprocessor is currently preprocessing"(?) Like, i've no idea how does the Preprocessor class work, but the code clearly looks too primitive to solve the problem. Clang-Tidy's PPCallbacks subsystem looks much more realistic.

[Charusso]

In D69731#1730899, @NoQ wrote:

Clang-Tidy's PPCallbacks subsystem looks much more realistic.

I wanted to make it available from the AnalysisManager so that I can obtain the macro definitions once before the analysis starts. Nor the Clang Tidy's PPCallback, nor the AnalysisManager could retrieve the necessary information at that point in the analysis. However, at that point when we analyze something all the necessary information is available. Most of the Tidy checkers grab their own PP reference to use it later on. The Tidy devs agree to retrieve such information even in the main check() call which going to rerun in every TranslationUnit. So even it is so primitive, there is no better solution as I know. I hope we could find something better / cache it. At the moment this code-snippet I have just showed you runs in every evalCall().

[Charusso]

I am thinking of a callback which is something like:

void checkBeginAnalysis(const Decl *D, BugReporter &BR) const;

so it would be easy and meaningful to have a place for the Preprocessor logic. Do you think it would worth it?

[Charusso]

• Use less const, it prevented the usage of non-const methods.

[Charusso]

In D69731#1730956, @Charusso wrote:

I am thinking of a callback which is something like:

void checkBeginAnalysis(const Decl *D, BugReporter &BR) const;

so it would be easy and meaningful to have a place for the Preprocessor logic. Do you think it would worth it?

Well, it made me mad to create setters / public fields and to rely on only the AnalysisManager. I am going to publish my first real checker, and it turns out being cool: D69745

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTER_H
 #define LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTER_H
 
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"

 class BugReporterData {
 public:
   virtual ~BugReporterData() = default;
 
   virtual ArrayRef<PathDiagnosticConsumer*> getPathDiagnosticConsumers() = 0;
   virtual ASTContext &getASTContext() = 0;
   virtual SourceManager &getSourceManager() = 0;
   virtual AnalyzerOptions &getAnalyzerOptions() = 0;
+  virtual Preprocessor &getPreprocessor() = 0;
 };
 
 /// BugReporter is a utility class for generating PathDiagnostics for analysis.
 /// It collects the BugReports and BugTypes and knows how to generate
 /// and flush the corresponding diagnostics.
 ///
 /// The base class is used for generating path-insensitive
 class BugReporter {
public:
   EQClasses_iterator EQClasses_end() { return EQClasses.end(); }
 
   ASTContext &getContext() { return D.getASTContext(); }
 
   const SourceManager &getSourceManager() { return D.getSourceManager(); }
 
   const AnalyzerOptions &getAnalyzerOptions() { return D.getAnalyzerOptions(); }
 
+  Preprocessor &getPreprocessor() { return D.getPreprocessor(); }
+
   /// Add the given report to the set of reports tracked by BugReporter.
   ///
   /// The reports are usually generated by the checkers. Further, they are
   /// folded based on the profile value, which is done to coalesce similar
   /// reports.
   virtual void emitReport(std::unique_ptr<BugReport> R);
 
   void EmitBasicReport(const Decl *DeclWithIssue, const CheckerBase *Checker,

clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_ANALYSISMANAGER_H
 #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_ANALYSISMANAGER_H
 
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/PathDiagnostic.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 
 namespace clang {
 
 class CodeInjector;
 
 namespace ento {
   class CheckerManager;
 
 class AnalysisManager : public BugReporterData {
   virtual void anchor();
   AnalysisDeclContextManager AnaCtxMgr;
 
   ASTContext &Ctx;
+  Preprocessor &PP;
   const LangOptions &LangOpts;
   PathDiagnosticConsumers PathConsumers;
 
   // Configurable components creators.
   StoreManagerCreator CreateStoreMgr;
   ConstraintManagerCreator CreateConstraintMgr;
 
   CheckerManager *CheckerMgr;
 
 public:
   AnalyzerOptions &options;
 
-  AnalysisManager(ASTContext &ctx,
+  AnalysisManager(ASTContext &ctx, Preprocessor &PP,
                   const PathDiagnosticConsumers &Consumers,
                   StoreManagerCreator storemgr,
                   ConstraintManagerCreator constraintmgr,
                   CheckerManager *checkerMgr, AnalyzerOptions &Options,
                   CodeInjector *injector = nullptr);
 
   ~AnalysisManager() override;
 
   void ClearContexts() {
     AnaCtxMgr.clear();
   }
 
   AnalysisDeclContextManager& getAnalysisDeclContextManager() {
     return AnaCtxMgr;
   }
 
+  Preprocessor &getPreprocessor() override { return PP; }
+
   StoreManagerCreator getStoreManagerCreator() {
     return CreateStoreMgr;
   }
 
   AnalyzerOptions& getAnalyzerOptions() override {
     return options;
   }
 

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

public:
   BugReporter &getBugReporter() {
     return Eng.getBugReporter();
   }
 
   const SourceManager &getSourceManager() {
     return getBugReporter().getSourceManager();
   }
 
+  Preprocessor &getPreprocessor() { return getBugReporter().getPreprocessor(); }
+
   SValBuilder &getSValBuilder() {
     return Eng.getSValBuilder();
   }
 
   SymbolManager &getSymbolManager() {
     return getSValBuilder().getSymbolManager();
   }
 

clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp

 //===-- AnalysisManager.cpp -------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 
 using namespace clang;
 using namespace ento;
 
 void AnalysisManager::anchor() { }
 
-AnalysisManager::AnalysisManager(ASTContext &ASTCtx,
+AnalysisManager::AnalysisManager(ASTContext &ASTCtx, Preprocessor &PP,
                                  const PathDiagnosticConsumers &PDC,
                                  StoreManagerCreator storemgr,
                                  ConstraintManagerCreator constraintmgr,
                                  CheckerManager *checkerMgr,
                                  AnalyzerOptions &Options,
                                  CodeInjector *injector)
     : AnaCtxMgr(
           ASTCtx, Options.UnoptimizedCFG,
           Options.ShouldIncludeImplicitDtorsInCFG,
           /*addInitializers=*/true,
           Options.ShouldIncludeTemporaryDtorsInCFG,
           Options.ShouldIncludeLifetimeInCFG,
           // Adding LoopExit elements to the CFG is a requirement for loop
           // unrolling.
           Options.ShouldIncludeLoopExitInCFG ||
             Options.ShouldUnrollLoops,
           Options.ShouldIncludeScopesInCFG,
           Options.ShouldSynthesizeBodies,
           Options.ShouldConditionalizeStaticInitializers,
           /*addCXXNewAllocator=*/true,
           Options.ShouldIncludeRichConstructorsInCFG,
           Options.ShouldElideConstructors,
           /*addVirtualBaseBranches=*/true,
           injector),
-      Ctx(ASTCtx), LangOpts(ASTCtx.getLangOpts()),
+      Ctx(ASTCtx), PP(PP), LangOpts(ASTCtx.getLangOpts()),
       PathConsumers(PDC), CreateStoreMgr(storemgr),
       CreateConstraintMgr(constraintmgr), CheckerMgr(checkerMgr),
       options(Options) {
   AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd();
   AnaCtxMgr.getCFGBuildOptions().OmitImplicitValueInitializers = true;
   AnaCtxMgr.getCFGBuildOptions().AddCXXDefaultInitExprInAggregates =
       Options.ShouldIncludeDefaultInitForAggregates;
 }

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

class AnalysisConsumer : public AnalysisASTConsumer,
   AnalysisMode RecVisitorMode;
   /// Bug Reporter to use while recursively visiting Decls.
   BugReporter *RecVisitorBR;
 
   std::vector<std::function<void(CheckerRegistry &)>> CheckerRegistrationFns;
 
 public:
   ASTContext *Ctx;
-  const Preprocessor &PP;
+  Preprocessor &PP;
   const std::string OutDir;
   AnalyzerOptionsRef Opts;
   ArrayRef<std::string> Plugins;
   CodeInjector *Injector;
   cross_tu::CrossTranslationUnitContext CTU;
 
   /// Stores the declarations from the local translation unit.
   /// Note, we pre-compute the local declarations at parse time as an
void DisplayFunction(const Decl *D, AnalysisMode Mode,
     }
   }
 
   void Initialize(ASTContext &Context) override {
     Ctx = &Context;
     checkerMgr = createCheckerManager(
         *Ctx, *Opts, Plugins, CheckerRegistrationFns, PP.getDiagnostics());
 
-    Mgr = std::make_unique<AnalysisManager>(*Ctx, PathConsumers, CreateStoreMgr,
-                                            CreateConstraintMgr,
+    Mgr = std::make_unique<AnalysisManager>(*Ctx, PP, PathConsumers,
+                                            CreateStoreMgr, CreateConstraintMgr,
                                             checkerMgr.get(), *Opts, Injector);
   }
 
   /// Store the top level decls in the set to be processed later on.
   /// (Doing this pre-processing avoids deserialization of data from PCH.)
   bool HandleTopLevelDecl(DeclGroupRef D) override;
   void HandleTopLevelDeclInObjCContainer(DeclGroupRef D) override;
 

clang/unittests/StaticAnalyzer/Reusables.h

 
 protected:
   ExprEngine Eng;
 
 public:
   ExprEngineConsumer(CompilerInstance &C)
       : C(C), ChkMgr(C.getASTContext(), *C.getAnalyzerOpts()), CTU(C),
         Consumers(),
-        AMgr(C.getASTContext(), Consumers,
+        AMgr(C.getASTContext(), C.getPreprocessor(), Consumers,
              CreateRegionStoreManager, CreateRangeConstraintManager, &ChkMgr,
              *C.getAnalyzerOpts()),
         VisitedCallees(), FS(),
         Eng(CTU, AMgr, &VisitedCallees, &FS, ExprEngine::Inline_Regular) {}
 };
 
 } // namespace ento
 } // namespace clang
 
 #endif