[clang-tools-extra] r261991 - [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 26 01:19:34 PST 2016


Author: hokein
Date: Fri Feb 26 03:19:33 2016
New Revision: 261991

URL: http://llvm.org/viewvc/llvm-project?rev=261991&view=rev
Log:
[clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.

Summary:
The clang-tidy will trigger an assertion if it's not in the building directory.

TEST:
cd <llvm-repo>/
./build/bin/clang-tidy --checks=-*,modernize-use-nullptr -p build tools/clang/tools/extra/clang-tidy/ClangTidy.cpp

The crash issue is gone after applying this patch.

Fixes PR24834, PR26241

Reviewers: bkramer, alexfh

Subscribers: rizsotto.mailinglist, cfe-commits

Differential Revision: http://reviews.llvm.org/D17335

Added:
    clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/
    clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json
    clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
    clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
    clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=261991&r1=261990&r2=261991&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Fri Feb 26 03:19:33 2016
@@ -107,6 +107,10 @@ public:
     DiagPrinter->BeginSourceFile(LangOpts);
   }
 
+  SourceManager& getSourceManager() {
+    return SourceMgr;
+  }
+
   void reportDiagnostic(const ClangTidyError &Error) {
     const ClangTidyMessage &Message = Error.Message;
     SourceLocation Loc = getLocation(Message.FilePath, Message.FileOffset);
@@ -124,7 +128,10 @@ public:
       auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
                   << Message.Message << Name;
       for (const tooling::Replacement &Fix : Error.Fix) {
-        SourceLocation FixLoc = getLocation(Fix.getFilePath(), Fix.getOffset());
+        SmallString<128> FixAbsoluteFilePath = Fix.getFilePath();
+        Files.makeAbsolutePath(FixAbsoluteFilePath);
+        SourceLocation FixLoc =
+            getLocation(FixAbsoluteFilePath, Fix.getOffset());
         SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Fix.getLength());
         Diag << FixItHint::CreateReplacement(SourceRange(FixLoc, FixEndLoc),
                                              Fix.getReplacementText());
@@ -232,6 +239,13 @@ ClangTidyASTConsumerFactory::CreateASTCo
   Context.setCurrentFile(File);
   Context.setASTContext(&Compiler.getASTContext());
 
+  auto WorkingDir = Compiler.getSourceManager()
+                        .getFileManager()
+                        .getVirtualFileSystem()
+                        ->getCurrentWorkingDirectory();
+  if (WorkingDir)
+    Context.setCurrentBuildDirectory(WorkingDir.get());
+
   std::vector<std::unique_ptr<ClangTidyCheck>> Checks;
   CheckFactories->createChecks(&Context, Checks);
 
@@ -446,8 +460,24 @@ runClangTidy(std::unique_ptr<ClangTidyOp
 void handleErrors(const std::vector<ClangTidyError> &Errors, bool Fix,
                   unsigned &WarningsAsErrorsCount) {
   ErrorReporter Reporter(Fix);
-  for (const ClangTidyError &Error : Errors)
+  vfs::FileSystem &FileSystem =
+      *Reporter.getSourceManager().getFileManager().getVirtualFileSystem();
+  auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory();
+  if (!InitialWorkingDir)
+    llvm::report_fatal_error("Cannot get current working path.");
+
+  for (const ClangTidyError &Error : Errors) {
+    if (!Error.BuildDirectory.empty()) {
+      // By default, the working directory of file system is the current
+      // clang-tidy running directory.
+      //
+      // Change the directory to the one used during the analysis.
+      FileSystem.setCurrentWorkingDirectory(Error.BuildDirectory);
+    }
     Reporter.reportDiagnostic(Error);
+    // Return to the initial directory to correctly resolve next Error.
+    FileSystem.setCurrentWorkingDirectory(InitialWorkingDir.get());
+  }
   Reporter.Finish();
   WarningsAsErrorsCount += Reporter.getWarningsAsErrorsCount();
 }

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=261991&r1=261990&r2=261991&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Fri Feb 26 03:19:33 2016
@@ -116,8 +116,9 @@ ClangTidyMessage::ClangTidyMessage(Strin
 
 ClangTidyError::ClangTidyError(StringRef CheckName,
                                ClangTidyError::Level DiagLevel,
-                               bool IsWarningAsError)
-    : CheckName(CheckName), DiagLevel(DiagLevel),
+                               bool IsWarningAsError,
+                               StringRef BuildDirectory)
+    : CheckName(CheckName), BuildDirectory(BuildDirectory), DiagLevel(DiagLevel),
       IsWarningAsError(IsWarningAsError) {}
 
 // Returns true if GlobList starts with the negative indicator ('-'), removes it
@@ -335,7 +336,8 @@ void ClangTidyDiagnosticConsumer::Handle
     bool IsWarningAsError =
         DiagLevel == DiagnosticsEngine::Warning &&
         Context.getWarningAsErrorFilter().contains(CheckName);
-    Errors.push_back(ClangTidyError(CheckName, Level, IsWarningAsError));
+    Errors.push_back(ClangTidyError(CheckName, Level, IsWarningAsError,
+                                    Context.getCurrentBuildDirectory()));
   }
 
   ClangTidyDiagnosticRenderer Converter(

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h?rev=261991&r1=261990&r2=261991&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h Fri Feb 26 03:19:33 2016
@@ -57,13 +57,23 @@ struct ClangTidyError {
     Error = DiagnosticsEngine::Error
   };
 
-  ClangTidyError(StringRef CheckName, Level DiagLevel, bool IsWarningAsError);
+  ClangTidyError(StringRef CheckName, Level DiagLevel, bool IsWarningAsError,
+                 StringRef BuildDirectory);
 
   std::string CheckName;
   ClangTidyMessage Message;
   tooling::Replacements Fix;
   SmallVector<ClangTidyMessage, 1> Notes;
 
+  // A build directory of the diagnostic source file.
+  //
+  // It's an absolute path which is `directory` field of the source file in
+  // compilation database. If users don't specify the compilation database
+  // directory, it is the current directory where clang-tidy runs.
+  //
+  // Note: it is empty in unittest.
+  std::string BuildDirectory;
+
   Level DiagLevel;
   bool IsWarningAsError;
 };
@@ -198,6 +208,16 @@ public:
   void setCheckProfileData(ProfileData *Profile);
   ProfileData *getCheckProfileData() const { return Profile; }
 
+  /// \brief Should be called when starting to process new translation unit.
+  void setCurrentBuildDirectory(StringRef BuildDirectory) {
+    CurrentBuildDirectory = BuildDirectory;
+  }
+
+  /// \brief Returns build directory of the current translation unit.
+  const std::string &getCurrentBuildDirectory() {
+    return CurrentBuildDirectory;
+  }
+
 private:
   // Calls setDiagnosticsEngine() and storeError().
   friend class ClangTidyDiagnosticConsumer;
@@ -222,6 +242,8 @@ private:
 
   ClangTidyStats Stats;
 
+  std::string CurrentBuildDirectory;
+
   llvm::DenseMap<unsigned, std::string> CheckNamesByDiagnosticID;
 
   ProfileData *Profile;

Added: clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json?rev=261991&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json (added)
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/compilation-database/template.json Fri Feb 26 03:19:33 2016
@@ -0,0 +1,32 @@
+[
+{
+  "directory": "test_dir/a",
+  "command": "clang++ -o test.o test_dir/a/a.cpp",
+  "file": "test_dir/a/a.cpp"
+},
+{
+  "directory": "test_dir/a",
+  "command": "clang++ -o test.o test_dir/a/b.cpp",
+  "file": "test_dir/a/b.cpp"
+},
+{
+  "directory": "test_dir/",
+  "command": "clang++ -o test.o test_dir/b/b.cpp",
+  "file": "test_dir/b/b.cpp"
+},
+{
+  "directory": "test_dir/b",
+  "command": "clang++ -o test.o ../b/c.cpp",
+  "file": "test_dir/b/c.cpp"
+},
+{
+  "directory": "test_dir/b",
+  "command": "clang++ -I../include -o test.o ../b/d.cpp",
+  "file": "test_dir/b/d.cpp"
+},
+{
+  "directory": "test_dir/",
+  "command": "clang++ -o test.o test_dir/b/not-exist.cpp",
+  "file": "test_dir/b/not-exist.cpp"
+}
+]

Added: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp?rev=261991&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-run-with-database.cpp Fri Feb 26 03:19:33 2016
@@ -0,0 +1,24 @@
+// REQUIRES: shell
+// RUN: mkdir -p %T/compilation-database-test/include
+// RUN: mkdir -p %T/compilation-database-test/a
+// RUN: mkdir -p %T/compilation-database-test/b
+// RUN: echo 'int *AA = 0;' > %T/compilation-database-test/a/a.cpp
+// RUN: echo 'int *AB = 0;' > %T/compilation-database-test/a/b.cpp
+// RUN: echo 'int *BB = 0;' > %T/compilation-database-test/b/b.cpp
+// RUN: echo 'int *BC = 0;' > %T/compilation-database-test/b/c.cpp
+// RUN: echo 'int *HP = 0;' > %T/compilation-database-test/include/header.h
+// RUN: echo '#include "header.h"' > %T/compilation-database-test/b/d.cpp
+// RUN: sed 's|test_dir|%T/compilation-database-test|g' %S/Inputs/compilation-database/template.json > %T/compile_commands.json
+// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T %T/compilation-database-test/b/not-exist -header-filter=.*
+// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T %T/compilation-database-test/a/a.cpp %T/compilation-database-test/a/b.cpp %T/compilation-database-test/b/b.cpp %T/compilation-database-test/b/c.cpp %T/compilation-database-test/b/d.cpp -header-filter=.* -fix
+// RUN: FileCheck -input-file=%T/compilation-database-test/a/a.cpp %s -check-prefix=CHECK-FIX1
+// RUN: FileCheck -input-file=%T/compilation-database-test/a/b.cpp %s -check-prefix=CHECK-FIX2
+// RUN: FileCheck -input-file=%T/compilation-database-test/b/b.cpp %s -check-prefix=CHECK-FIX3
+// RUN: FileCheck -input-file=%T/compilation-database-test/b/c.cpp %s -check-prefix=CHECK-FIX4
+// RUN: FileCheck -input-file=%T/compilation-database-test/include/header.h %s -check-prefix=CHECK-FIX5
+
+// CHECK-FIX1: int *AA = nullptr;
+// CHECK-FIX2: int *AB = nullptr;
+// CHECK-FIX3: int *BB = nullptr;
+// CHECK-FIX4: int *BC = nullptr;
+// CHECK-FIX5: int *HP = nullptr;




More information about the cfe-commits mailing list