Is it possible to port this commit to 3.8.1? Cheers, Edward-san
2016-02-26 10:19 GMT+01:00 Haojian Wu via cfe-commits <cfe-commits@lists.llvm.org>: > 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; > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits -- Mathematics is the language with which God has written the universe. (Galilei) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits