From: Haojian Wu Date: Fri, 26 Feb 2016 09:19:33 +0000 (+0000) Subject: [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f7692a27923bbeb7cfdeb26909c2a3f0894bc132;p=platform%2Fupstream%2Fllvm.git [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 / ./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 llvm-svn: 261991 --- diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index b2dd7e7..9fbe0dd 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -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::CreateASTConsumer( Context.setCurrentFile(File); Context.setASTContext(&Compiler.getASTContext()); + auto WorkingDir = Compiler.getSourceManager() + .getFileManager() + .getVirtualFileSystem() + ->getCurrentWorkingDirectory(); + if (WorkingDir) + Context.setCurrentBuildDirectory(WorkingDir.get()); + std::vector> Checks; CheckFactories->createChecks(&Context, Checks); @@ -446,8 +460,24 @@ runClangTidy(std::unique_ptr OptionsProvider, void handleErrors(const std::vector &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(); } diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index d1cb4eb..48ae922 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -116,8 +116,9 @@ ClangTidyMessage::ClangTidyMessage(StringRef Message, 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::HandleDiagnostic( 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( diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h index cbd376c..0adfe2f 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -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 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 CheckNamesByDiagnosticID; ProfileData *Profile; diff --git a/clang-tools-extra/test/clang-tidy/Inputs/compilation-database/template.json b/clang-tools-extra/test/clang-tidy/Inputs/compilation-database/template.json new file mode 100644 index 0000000..74275d9 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/Inputs/compilation-database/template.json @@ -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" +} +] diff --git a/clang-tools-extra/test/clang-tidy/clang-tidy-run-with-database.cpp b/clang-tools-extra/test/clang-tidy/clang-tidy-run-with-database.cpp new file mode 100644 index 0000000..7304f06 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/clang-tidy-run-with-database.cpp @@ -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;