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

Edoardo P. via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 1 14:19:44 PDT 2016


Ping.

Deadline to merge the changes to the 3.8 branch is today.

Cheers,
Edward-san

2016-05-24 20:25 GMT+02:00 Edoardo P. <ed0.88.prez at gmail.com>:
> Ping,
>
> who's going to merge? I have no commit access.
>
> Cheers,
> Edward-san
>
>
> 2016-05-20 18:34 GMT+02:00 Tom Stellard <tom at stellard.net>:
>> Hi,
>>
>> This looks fine to me, go ahead and merge.
>>
>> -Tom
>>
>> On Thu, May 19, 2016 at 08:29:14PM +0200, Alexander Kornienko wrote:
>>> 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
>>> >
>
>
>
> --
> Mathematics is the language with which God has written the universe. (Galilei)



-- 
Mathematics is the language with which God has written the universe. (Galilei)


More information about the cfe-commits mailing list