[CTU][Analyzer]Add DisplayCTUProgress analyzer switch

[martong]

Summary

With a new switch we may be able to print to stderr if a new TU is being loaded
during CTU. This is very important for higher level scripts (like CodeChecker)
to be able to parse this output so they can create e.g. a zip file in case of
a Clang crash which contains all the related TU files.

[Szelethus]

The idea is great!

I think this should rather be an -analyzer-config flag, since the actual analysis changes with a new output (refer to AnalyzerOption's doxygen comments). Please note that I'm about to land some patches that modifies AnalyzerOptions.def quite a bit, but rebasing shouldn't be too bad anyways.

edit: They are now in trunk.

Also, almost everywhere CTU is capitalized, so I guess it should be in the field name too.

test/Analysis/ctu-main.cpp

6 ↗

(On Diff #176159)

I think these RUN lines would really benefit from introducing line breaks.

[a_sidorin]

Hi Gabor,
In addition to Umann remarks, there is a small comment inline.

lib/CrossTU/CrossTranslationUnit.cpp

239 ↗

(On Diff #176159)

I think we can remove parens from the message.

[Szelethus]

Also, AnalyzerOptions.def was recently clan-formatted, feel free to run it again after the changes you make in it.

[martong]

• Rename AnalyzerDisplayCtuProgress to Opts.AnalyzerDisplayCTUProgress
• Change the CTU progress message

[martong]

Also, almost everywhere CTU is capitalized, so I guess it should be in the field name too.

Ok, I renamed it to have CTU all capitalized.

lib/CrossTU/CrossTranslationUnit.cpp

239 ↗

(On Diff #176159)

Ok, I removed the parens. And also changed the text from CTU loaded AST for source file to CTU loaded AST file since we dump the path to the AST, not the path to the source file from which we generated the AST file.

test/Analysis/ctu-main.cpp

6 ↗

(On Diff #176159)

Yes, I agree. Unfortunately, I could not figure out how to break them. Using a simple "\" at around the 80th column gives Test has unterminated run lines (with '\'). If I use block comments with "\" the same happens. If I use block comments and don't use the "\" then the second line is not interpreted. Is it really possible to break RUN lines? I could not find anything about that in the online docs.

[martong]

• Change the CTU progress message in the test too

[martong]

• Break long RUN line

test/Analysis/ctu-main.cpp

6 ↗

(On Diff #176159)

Oh, I just have found your other comment to the other patch. So yes, it is indeed possible to break this line. I updated the patch accordingly. The other long lines which are already there I am going to change in an independent patch: (https://reviews.llvm.org/D55129).

[Szelethus]

test/Analysis/ctu-main.cpp

6 ↗

(On Diff #176159)

Is there any particular reason why we use %clang_cc1 -analyze as opposed to %clang_analyze_cc1? If not, lets change it.

By the way, thanks! This looks neat.

[Szelethus]

The code not directly related to adding the actual flag looks great, but this still should be an -analyzer-config option.

[xazax.hun]

Having an analyzer config option makes sense.

[martong]

• Use clang_analyze_cc1
• Change to be an analyzer config option

[martong]

should be an -analyzer-config option.

Ok, just changed it to be.

[Szelethus]

Thanks! LGTM! :)

[xazax.hun]

While Static Analyzer is the only client of CTU library at the moment, we might have more in the future. I would not use the phrase ANALYZE in the log message. Once this is resolved the rest looks good.

[martong]

While Static Analyzer is the only client of CTU library at the moment, we might have more in the future. I would not use the phrase ANALYZE in the log message. Once this is resolved the rest looks good.

Ok, I removed the "ANALYZE " prefix from the log message.

[martong]

• Remove 'ANALYZE ' prefix from the log message

cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h

public:
   ///
   /// \return The declaration with the definition will be returned.
   /// If no suitable definition is found in the index file or multiple
   /// definitions found error will be returned.
   ///
   /// Note that the AST files should also be in the \p CrossTUDir.
   llvm::Expected<const FunctionDecl *>
   getCrossTUDefinition(const FunctionDecl *FD, StringRef CrossTUDir,
-                       StringRef IndexName);
+                       StringRef IndexName, bool DisplayCTUProgress = false);
 
   /// This function loads a function definition from an external AST
   ///        file.
   ///
   /// A function definition with the same declaration will be looked up in the
   /// index file which should be in the \p CrossTUDir directory, called
   /// \p IndexName. In case the declaration is found in the index the
   /// corresponding AST file will be loaded.
   ///
   /// \return Returns an ASTUnit that contains the definition of the looked up
   /// function.
   ///
   /// Note that the AST files should also be in the \p CrossTUDir.
   llvm::Expected<ASTUnit *> loadExternalAST(StringRef LookupName,
                                             StringRef CrossTUDir,
-                                            StringRef IndexName);
+                                            StringRef IndexName,
+                                            bool DisplayCTUProgress = false);
 
   /// This function merges a definition from a separate AST Unit into
   ///        the current one which was created by the compiler instance that
   ///        was passed to the constructor.
   ///
   /// \return Returns the resulting definition or an error.
   llvm::Expected<const FunctionDecl *> importDefinition(const FunctionDecl *FD);
 

cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

ANALYZER_OPTION(
     "experimental feature to inline functions from another translation units.",
     false)
 
 ANALYZER_OPTION(bool, ShouldDisplayMacroExpansions, "expand-macros",
                 "Whether macros related to the bugpath should be "
                 "expanded and included in the plist output.",
                 false)
 
+ANALYZER_OPTION(bool, DisplayCTUProgress, "display-ctu-progress",
+                "Whether to emit verbose output about "
+                "the analyzer's progress related to ctu.",
+                false)
+
 //===----------------------------------------------------------------------===//
 // Unsinged analyzer options.
 //===----------------------------------------------------------------------===//
 
 ANALYZER_OPTION(
     unsigned, AlwaysInlineSize, "ipa-always-inline-size",
     "The size of the functions (in basic blocks), which should be considered "
     "to be small enough to always inline.",

cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp

for (const Decl *D : DC->decls()) {
     return ResultDecl;
   }
   return nullptr;
 }
 
 llvm::Expected<const FunctionDecl *>
 CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,
                                                   StringRef CrossTUDir,
-                                                  StringRef IndexName) {
+                                                  StringRef IndexName,
+                                                  bool DisplayCTUProgress) {
   assert(FD && "FD is missing, bad call to this function!");
   assert(!FD->hasBody() && "FD has a definition in current translation unit!");
   ++NumGetCTUCalled;
   const std::string LookupFnName = getLookupName(FD);
   if (LookupFnName.empty())
     return llvm::make_error<IndexError>(
         index_error_code::failed_to_generate_usr);
   llvm::Expected<ASTUnit *> ASTUnitOrError =
-      loadExternalAST(LookupFnName, CrossTUDir, IndexName);
+      loadExternalAST(LookupFnName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
     return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
   if (!Unit)
     return llvm::make_error<IndexError>(
         index_error_code::failed_to_get_external_ast);
   assert(&Unit->getFileManager() ==
          &Unit->getASTContext().getSourceManager().getFileManager());
Context.getDiagnostics().Report(diag::err_multiple_def_index)
         << IE.getLineNum();
     break;
   default:
     break;
   }
 }
 
 llvm::Expected<ASTUnit *> CrossTranslationUnitContext::loadExternalAST(
-    StringRef LookupName, StringRef CrossTUDir, StringRef IndexName) {
+    StringRef LookupName, StringRef CrossTUDir, StringRef IndexName,
+    bool DisplayCTUProgress) {
   // FIXME: The current implementation only supports loading functions with
   //        a lookup name from a single translation unit. If multiple
   //        translation units contains functions with the same lookup name an
   //        error will be returned.
   ASTUnit *Unit = nullptr;
   auto FnUnitCacheEntry = FunctionASTUnitMap.find(LookupName);
   if (FnUnitCacheEntry == FunctionASTUnitMap.end()) {
     if (FunctionFileMap.empty()) {
if (ASTCacheEntry == FileASTUnitMap.end()) {
       IntrusiveRefCntPtr<DiagnosticsEngine> Diags(
           new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
 
       std::unique_ptr<ASTUnit> LoadedUnit(ASTUnit::LoadFromASTFile(
           ASTFileName, CI.getPCHContainerOperations()->getRawReader(),
           ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()));
       Unit = LoadedUnit.get();
       FileASTUnitMap[ASTFileName] = std::move(LoadedUnit);
+      if (DisplayCTUProgress) {
+        llvm::errs() << "CTU loaded AST file: "
+                     << ASTFileName << "\n";
+      }
     } else {
       Unit = ASTCacheEntry->second.get();
     }
     FunctionASTUnitMap[LookupName] = Unit;
   } else {
     Unit = FnUnitCacheEntry->second;
   }
   return Unit;

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

RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.IsNaiveCTUEnabled)
     return {};
 
   cross_tu::CrossTranslationUnitContext &CTUCtx =
       *Engine->getCrossTranslationUnitContext();
   llvm::Expected<const FunctionDecl *> CTUDeclOrError =
-      CTUCtx.getCrossTUDefinition(FD, Opts.CTUDir,
-                                  Opts.CTUIndexName);
+      CTUCtx.getCrossTUDefinition(FD, Opts.CTUDir, Opts.CTUIndexName,
+                                  Opts.DisplayCTUProgress);
 
   if (!CTUDeclOrError) {
     handleAllErrors(CTUDeclOrError.takeError(),
                     [&](const cross_tu::IndexError &IE) {
                       CTUCtx.emitCrossTUDiagnostics(IE);
                     });
     return {};
   }

cfe/trunk/test/Analysis/analyzer-config.c

 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
 // CHECK-NEXT: ctu-index-name = externalFnMap.txt
+// CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
 // CHECK-NEXT: expand-macros = false
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: inline-lambdas = true

 // CHECK-NEXT: serialize-stats = false
 // CHECK-NEXT: stable-report-filename = false
 // CHECK-NEXT: suppress-c++-stdlib = true
 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 48
+// CHECK-NEXT: num-entries = 49

cfe/trunk/test/Analysis/ctu-main.cpp

 // RUN: rm -rf %t && mkdir %t
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-chain.cpp.ast %S/Inputs/ctu-chain.cpp
 // RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/
 // RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
 // RUN:   -analyzer-config ctu-dir=%t/ctudir \
 // RUN:   -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -analyzer-config display-ctu-progress=true 2>&1 %s | FileCheck %s
+
+// CHECK: CTU loaded AST file: {{.*}}/ctu-other.cpp
+// CHECK: CTU loaded AST file: {{.*}}/ctu-chain.cpp
 
 #include "ctu-hdr.h"
 
 void clang_analyzer_eval(int);
 
 int f(int);
 int g(int);
 int h(int);