[clang-tools-extra] r261991 - [clang-tidy] Fix a crash issue when clang-tidy runs with compilation database.
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Thu May 19 07:45:27 PDT 2016
+Tom who manages 3.8.1
+Alex who's owner of clang-tidy: is this ok for 3.8.1?
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
More information about the cfe-commits
mailing list