[clang-tools-extra] r261991 - [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Thu May 19 11:29:14 PDT 2016
On Thu, May 19, 2016 at 4:45 PM, Hans Wennborg <hans at chromium.org> wrote:
> +Tom who manages 3.8.1
> +Alex who's owner of clang-tidy: is this ok for 3.8.1?
>
Yes, would be nice to have this in 3.8.1. This fixes a rather annoying
problem.
>
> On Thu, May 19, 2016 at 1:56 AM, Edoardo P. via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> > 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 at 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 at 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 at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160519/087290b1/attachment-0001.html>
More information about the cfe-commits
mailing list