From c46d0bb9950b1ebb05bd470502973ad207e56640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Mon, 28 Mar 2022 12:26:56 +0200 Subject: [PATCH 01/11] Make C++ parser use the working directory of compilation commands Fixes compilation commands containing relative paths not getting parsed properly. --- .../parser/include/cppparser/filelocutil.h | 24 ++++++++++++------- plugins/cpp/parser/src/cppparser.cpp | 10 ++++++-- plugins/cpp/parser/src/ppincludecallback.cpp | 12 +++++----- plugins/cpp/parser/src/ppmacrocallback.cpp | 6 +++-- 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/plugins/cpp/parser/include/cppparser/filelocutil.h b/plugins/cpp/parser/include/cppparser/filelocutil.h index ccfa96883..f342c20dd 100644 --- a/plugins/cpp/parser/include/cppparser/filelocutil.h +++ b/plugins/cpp/parser/include/cppparser/filelocutil.h @@ -79,23 +79,31 @@ class FileLocUtil } /** - * This function returns the file path in which loc_ location takes place. The - * location is meant to be the expanded location (in case of macro expansion). - * If the file can't be determined then empty string returns. + * This function returns the absolute path of the file in which loc_ location + * takes place. The location is meant to be the expanded location (in case of + * macro expansion). + * If the file can't be determined then an empty string is returned. */ std::string getFilePath(const clang::SourceLocation& loc_) { - clang::SourceLocation expLoc = _clangSrcMan.getExpansionLoc(loc_); - clang::FileID fid = _clangSrcMan.getFileID(expLoc); + return getFilePath( + _clangSrcMan.getFileID(_clangSrcMan.getExpansionLoc(loc_))); + } - if (fid.isInvalid()) + /** + * This function returns the absolute path of the file identified by fid_. + * If the file can't be determined then an empty string is returned. + */ + std::string getFilePath(const clang::FileID& fid_) + { + if (fid_.isInvalid()) return std::string(); - const clang::FileEntry* fileEntry = _clangSrcMan.getFileEntryForID(fid); + const clang::FileEntry* fileEntry = _clangSrcMan.getFileEntryForID(fid_); if (!fileEntry) return std::string(); - return fileEntry->getName(); + return fileEntry->tryGetRealPathName(); } private: diff --git a/plugins/cpp/parser/src/cppparser.cpp b/plugins/cpp/parser/src/cppparser.cpp index 3730cbdcc..86b620847 100644 --- a/plugins/cpp/parser/src/cppparser.cpp +++ b/plugins/cpp/parser/src/cppparser.cpp @@ -308,7 +308,8 @@ int CppParser::parseWorker(const clang::tooling::CompileCommand& command_) clang::tooling::FixedCompilationDatabase::loadFromCommandLine( argc, commandLine.data(), - compilationDbLoadError)); + compilationDbLoadError, + command_.Directory)); if (!compilationDb) { @@ -329,7 +330,12 @@ int CppParser::parseWorker(const clang::tooling::CompileCommand& command_) sourceFullPath = fs::path(command_.Directory) / command_.Filename; VisitorActionFactory factory(_ctx); - clang::tooling::ClangTool tool(*compilationDb, sourceFullPath.string()); + + // Use a PhysicalFileSystem as it's thread-safe + + clang::tooling::ClangTool tool(*compilationDb, sourceFullPath.string(), + std::make_shared(), + llvm::vfs::createPhysicalFileSystem().release()); llvm::IntrusiveRefCntPtr diagOpts = new clang::DiagnosticOptions(); diff --git a/plugins/cpp/parser/src/ppincludecallback.cpp b/plugins/cpp/parser/src/ppincludecallback.cpp index b163732fe..af3dfee8f 100644 --- a/plugins/cpp/parser/src/ppincludecallback.cpp +++ b/plugins/cpp/parser/src/ppincludecallback.cpp @@ -58,10 +58,10 @@ model::CppAstNodePtr PPIncludeCallback::createFileAstNode( void PPIncludeCallback::InclusionDirective( clang::SourceLocation hashLoc_, const clang::Token&, - clang::StringRef fileName_, + clang::StringRef, bool, clang::CharSourceRange filenameRange_, - const clang::FileEntry*, + const clang::FileEntry* file_, clang::StringRef searchPath_, clang::StringRef, const clang::Module*, @@ -75,8 +75,8 @@ void PPIncludeCallback::InclusionDirective( //--- Included file ---// - std::string includedPath = searchPath_.str() + '/' + fileName_.str(); - model::FilePtr included = _ctx.srcMgr.getFile(includedPath); + model::FilePtr included = _ctx.srcMgr.getFile( + file_->tryGetRealPathName().str()); included->parseStatus = model::File::PSFullyParsed; if (included->type != model::File::DIRECTORY_TYPE && included->type != _cppSourceType) @@ -87,8 +87,8 @@ void PPIncludeCallback::InclusionDirective( //--- Includer file ---// - std::string includerPath = presLoc.getFilename(); - model::FilePtr includer = _ctx.srcMgr.getFile(includerPath); + model::FilePtr includer = _ctx.srcMgr.getFile( + _fileLocUtil.getFilePath(presLoc.getFileID())); includer->parseStatus = model::File::PSFullyParsed; if (includer->type != model::File::DIRECTORY_TYPE && includer->type != _cppSourceType) diff --git a/plugins/cpp/parser/src/ppmacrocallback.cpp b/plugins/cpp/parser/src/ppmacrocallback.cpp index d86532e32..bebcefc62 100644 --- a/plugins/cpp/parser/src/ppmacrocallback.cpp +++ b/plugins/cpp/parser/src/ppmacrocallback.cpp @@ -253,11 +253,13 @@ std::string PPMacroCallback::getUSR(const clang::MacroInfo* mi_) std::string locStr = std::to_string(presLoc.getLine()) + ":" + std::to_string(presLoc.getColumn()) + ":"; + + std::string filePath = _fileLocUtil.getFilePath(presLoc.getFileID()); return locStr + (isBuiltInMacro(mi_) - ? presLoc.getFilename() - : std::to_string(_ctx.srcMgr.getFile(presLoc.getFilename())->id)); + ? filePath + : std::to_string(_ctx.srcMgr.getFile(filePath)->id)); } } // parser From 5e257ab76e15e88351c1d30853a3ded6be41c53f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Mon, 28 Mar 2022 07:16:18 +0200 Subject: [PATCH 02/11] Don't make source file path absolute in ClangTool invocation Reverts functional change of 04826d2 as it's no longer necessary. --- plugins/cpp/parser/src/cppparser.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/plugins/cpp/parser/src/cppparser.cpp b/plugins/cpp/parser/src/cppparser.cpp index 86b620847..aec9aa287 100644 --- a/plugins/cpp/parser/src/cppparser.cpp +++ b/plugins/cpp/parser/src/cppparser.cpp @@ -325,15 +325,11 @@ int CppParser::parseWorker(const clang::tooling::CompileCommand& command_) //--- Start the tool ---// - fs::path sourceFullPath(command_.Filename); - if (!sourceFullPath.is_absolute()) - sourceFullPath = fs::path(command_.Directory) / command_.Filename; - VisitorActionFactory factory(_ctx); // Use a PhysicalFileSystem as it's thread-safe - clang::tooling::ClangTool tool(*compilationDb, sourceFullPath.string(), + clang::tooling::ClangTool tool(*compilationDb, command_.Filename, std::make_shared(), llvm::vfs::createPhysicalFileSystem().release()); From ce956a8bbf0670ac8ed0fe8ea5ba6c86b320e609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Mon, 28 Mar 2022 12:06:37 +0200 Subject: [PATCH 03/11] Fix inaccuracies in SourceManager::getFile's comments --- parser/include/parser/sourcemanager.h | 4 ++-- parser/src/sourcemanager.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/parser/include/parser/sourcemanager.h b/parser/include/parser/sourcemanager.h index 9ec5b7a8a..ed4893436 100644 --- a/parser/include/parser/sourcemanager.h +++ b/parser/include/parser/sourcemanager.h @@ -51,8 +51,8 @@ class SourceManager * based on the given path_. The object is read from a cache. If the file is * not in the cache yet then a model::File entry is created, persisted in the * database and placed in the cache. If the file doesn't exist then it returns - * nullptr. - * @param path_ The file path to look up. + * a file with no contents. + * @param path_ The absolute file path to look up. */ model::FilePtr getFile(const std::string& path_); diff --git a/parser/src/sourcemanager.cpp b/parser/src/sourcemanager.cpp index 114db9661..37f9d5162 100644 --- a/parser/src/sourcemanager.cpp +++ b/parser/src/sourcemanager.cpp @@ -166,7 +166,7 @@ model::FilePtr SourceManager::getFile(const std::string& path_) boost::filesystem::path canonicalPath = boost::filesystem::canonical(path_, ec); - //--- If the file can't be found on disk then return nullptr ---// + //--- If the file can't be found on disk then return a blank file ---// bool fileExists = true; if (ec) From 96345c40637727fbec81bc0ea22cce939503db82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Mon, 28 Mar 2022 15:53:31 +0200 Subject: [PATCH 04/11] Add PrefixingFileSystem that emulates chdir even for non-existent dirs Fixes the previous implementation not working if compilation directories are missing. --- plugins/cpp/parser/src/cppparser.cpp | 10 ++- plugins/cpp/parser/src/prefixingfilesystem.h | 81 ++++++++++++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 plugins/cpp/parser/src/prefixingfilesystem.h diff --git a/plugins/cpp/parser/src/cppparser.cpp b/plugins/cpp/parser/src/cppparser.cpp index aec9aa287..759b90d2f 100644 --- a/plugins/cpp/parser/src/cppparser.cpp +++ b/plugins/cpp/parser/src/cppparser.cpp @@ -36,6 +36,7 @@ #include "ppmacrocallback.h" #include "doccommentcollector.h" #include "diagnosticmessagehandler.h" +#include "prefixingfilesystem.h" namespace cc { @@ -327,11 +328,12 @@ int CppParser::parseWorker(const clang::tooling::CompileCommand& command_) VisitorActionFactory factory(_ctx); - // Use a PhysicalFileSystem as it's thread-safe - - clang::tooling::ClangTool tool(*compilationDb, command_.Filename, + clang::tooling::ClangTool tool( + *compilationDb, + command_.Filename, std::make_shared(), - llvm::vfs::createPhysicalFileSystem().release()); + llvm::IntrusiveRefCntPtr( + new PrefixingFileSystem(llvm::vfs::getRealFileSystem()))); llvm::IntrusiveRefCntPtr diagOpts = new clang::DiagnosticOptions(); diff --git a/plugins/cpp/parser/src/prefixingfilesystem.h b/plugins/cpp/parser/src/prefixingfilesystem.h new file mode 100644 index 000000000..9f788e1b3 --- /dev/null +++ b/plugins/cpp/parser/src/prefixingfilesystem.h @@ -0,0 +1,81 @@ +#ifndef CC_PARSER_PREFIXINGFILESYSTEM_H +#define CC_PARSER_PREFIXINGFILESYSTEM_H + +#include + +namespace cc +{ +namespace parser +{ + +/** A Clang Virtual File System implementation that emulates chdir in a + * thread-safe way, and without the need for the working directory to exist. + * It proxies calls to another file system, modifying path arguments to be + * absolute based on the working directory of this one. + */ +class PrefixingFileSystem : public llvm::vfs::FileSystem +{ +public: + explicit PrefixingFileSystem(llvm::IntrusiveRefCntPtr FS_) + : _FS(std::move(FS_)) + {} + + llvm::ErrorOr status(const llvm::Twine& path_) override + { + return _FS->status(toAbsolute(path_)); + } + + llvm::ErrorOr> openFileForRead( + const llvm::Twine& path_) override + { + return _FS->openFileForRead(toAbsolute(path_)); + } + + llvm::vfs::directory_iterator dir_begin( + const llvm::Twine& dir_, + std::error_code& ec_) override + { + return _FS->dir_begin(toAbsolute(dir_), ec_); + } + + std::error_code getRealPath( + const llvm::Twine& path_, + llvm::SmallVectorImpl& output_) const override + { + return _FS->getRealPath(toAbsolute(path_), output_); + } + + std::error_code isLocal(const llvm::Twine& path_, bool& result_) override + { + return _FS->isLocal(toAbsolute(path_), result_); + } + + llvm::ErrorOr getCurrentWorkingDirectory() const override + { + return _workDir; + } + + std::error_code setCurrentWorkingDirectory(const llvm::Twine& path_) override + { + // We don't verify if the working directory exists on purpose. + _workDir = path_.str(); + return std::error_code(); + } + +private: + llvm::IntrusiveRefCntPtr _FS; + std::string _workDir; + + llvm::SmallString<256> toAbsolute(const llvm::Twine& relPath_) const + { + llvm::SmallString<256> absPath; + relPath_.toVector(absPath); + llvm::sys::fs::make_absolute(_workDir, absPath); + return absPath; + } +}; + +} // parser +} // cc + +#endif // CC_PARSER_PREFIXINGFILESYSTEM_H \ No newline at end of file From 19cda254a53879d2e174b0b48e08627196ff1a94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Wed, 30 Mar 2022 13:46:50 +0200 Subject: [PATCH 05/11] Rename PrefixingFileSystem to FakeWorkDirFileSystem This name reflects its purpose better. --- plugins/cpp/parser/src/cppparser.cpp | 6 +++--- .../{prefixingfilesystem.h => fakeworkdirfilesystem.h} | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) rename plugins/cpp/parser/src/{prefixingfilesystem.h => fakeworkdirfilesystem.h} (87%) diff --git a/plugins/cpp/parser/src/cppparser.cpp b/plugins/cpp/parser/src/cppparser.cpp index 759b90d2f..d8dcbcce8 100644 --- a/plugins/cpp/parser/src/cppparser.cpp +++ b/plugins/cpp/parser/src/cppparser.cpp @@ -36,7 +36,7 @@ #include "ppmacrocallback.h" #include "doccommentcollector.h" #include "diagnosticmessagehandler.h" -#include "prefixingfilesystem.h" +#include "fakeworkdirfilesystem.h" namespace cc { @@ -332,8 +332,8 @@ int CppParser::parseWorker(const clang::tooling::CompileCommand& command_) *compilationDb, command_.Filename, std::make_shared(), - llvm::IntrusiveRefCntPtr( - new PrefixingFileSystem(llvm::vfs::getRealFileSystem()))); + llvm::IntrusiveRefCntPtr( + new FakeWorkDirFileSystem(llvm::vfs::getRealFileSystem()))); llvm::IntrusiveRefCntPtr diagOpts = new clang::DiagnosticOptions(); diff --git a/plugins/cpp/parser/src/prefixingfilesystem.h b/plugins/cpp/parser/src/fakeworkdirfilesystem.h similarity index 87% rename from plugins/cpp/parser/src/prefixingfilesystem.h rename to plugins/cpp/parser/src/fakeworkdirfilesystem.h index 9f788e1b3..cba188f7d 100644 --- a/plugins/cpp/parser/src/prefixingfilesystem.h +++ b/plugins/cpp/parser/src/fakeworkdirfilesystem.h @@ -1,5 +1,5 @@ -#ifndef CC_PARSER_PREFIXINGFILESYSTEM_H -#define CC_PARSER_PREFIXINGFILESYSTEM_H +#ifndef CC_PARSER_FAKEWORKDIRFILESYSTEM_H +#define CC_PARSER_FAKEWORKDIRFILESYSTEM_H #include @@ -13,10 +13,10 @@ namespace parser * It proxies calls to another file system, modifying path arguments to be * absolute based on the working directory of this one. */ -class PrefixingFileSystem : public llvm::vfs::FileSystem +class FakeWorkDirFileSystem : public llvm::vfs::FileSystem { public: - explicit PrefixingFileSystem(llvm::IntrusiveRefCntPtr FS_) + explicit FakeWorkDirFileSystem(llvm::IntrusiveRefCntPtr FS_) : _FS(std::move(FS_)) {} @@ -78,4 +78,4 @@ class PrefixingFileSystem : public llvm::vfs::FileSystem } // parser } // cc -#endif // CC_PARSER_PREFIXINGFILESYSTEM_H \ No newline at end of file +#endif // CC_PARSER_FAKEWORKDIRFILESYSTEM_H From 581d2e98176a20ca42a634b02d75bad2db4a6182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Tue, 5 Apr 2022 01:32:22 +0200 Subject: [PATCH 06/11] Remove FakeWorkDirFileSystem It didn't work correctly (`//..` still gives an error), and if it did, it would be making potentially incorrect assumptions about the file system. This reverts commit 19cda254a53879d2e174b0b48e08627196ff1a94. This reverts commit 96345c40637727fbec81bc0ea22cce939503db82. --- plugins/cpp/parser/src/cppparser.cpp | 10 +-- .../cpp/parser/src/fakeworkdirfilesystem.h | 81 ------------------- 2 files changed, 4 insertions(+), 87 deletions(-) delete mode 100644 plugins/cpp/parser/src/fakeworkdirfilesystem.h diff --git a/plugins/cpp/parser/src/cppparser.cpp b/plugins/cpp/parser/src/cppparser.cpp index d8dcbcce8..aec9aa287 100644 --- a/plugins/cpp/parser/src/cppparser.cpp +++ b/plugins/cpp/parser/src/cppparser.cpp @@ -36,7 +36,6 @@ #include "ppmacrocallback.h" #include "doccommentcollector.h" #include "diagnosticmessagehandler.h" -#include "fakeworkdirfilesystem.h" namespace cc { @@ -328,12 +327,11 @@ int CppParser::parseWorker(const clang::tooling::CompileCommand& command_) VisitorActionFactory factory(_ctx); - clang::tooling::ClangTool tool( - *compilationDb, - command_.Filename, + // Use a PhysicalFileSystem as it's thread-safe + + clang::tooling::ClangTool tool(*compilationDb, command_.Filename, std::make_shared(), - llvm::IntrusiveRefCntPtr( - new FakeWorkDirFileSystem(llvm::vfs::getRealFileSystem()))); + llvm::vfs::createPhysicalFileSystem().release()); llvm::IntrusiveRefCntPtr diagOpts = new clang::DiagnosticOptions(); diff --git a/plugins/cpp/parser/src/fakeworkdirfilesystem.h b/plugins/cpp/parser/src/fakeworkdirfilesystem.h deleted file mode 100644 index cba188f7d..000000000 --- a/plugins/cpp/parser/src/fakeworkdirfilesystem.h +++ /dev/null @@ -1,81 +0,0 @@ -#ifndef CC_PARSER_FAKEWORKDIRFILESYSTEM_H -#define CC_PARSER_FAKEWORKDIRFILESYSTEM_H - -#include - -namespace cc -{ -namespace parser -{ - -/** A Clang Virtual File System implementation that emulates chdir in a - * thread-safe way, and without the need for the working directory to exist. - * It proxies calls to another file system, modifying path arguments to be - * absolute based on the working directory of this one. - */ -class FakeWorkDirFileSystem : public llvm::vfs::FileSystem -{ -public: - explicit FakeWorkDirFileSystem(llvm::IntrusiveRefCntPtr FS_) - : _FS(std::move(FS_)) - {} - - llvm::ErrorOr status(const llvm::Twine& path_) override - { - return _FS->status(toAbsolute(path_)); - } - - llvm::ErrorOr> openFileForRead( - const llvm::Twine& path_) override - { - return _FS->openFileForRead(toAbsolute(path_)); - } - - llvm::vfs::directory_iterator dir_begin( - const llvm::Twine& dir_, - std::error_code& ec_) override - { - return _FS->dir_begin(toAbsolute(dir_), ec_); - } - - std::error_code getRealPath( - const llvm::Twine& path_, - llvm::SmallVectorImpl& output_) const override - { - return _FS->getRealPath(toAbsolute(path_), output_); - } - - std::error_code isLocal(const llvm::Twine& path_, bool& result_) override - { - return _FS->isLocal(toAbsolute(path_), result_); - } - - llvm::ErrorOr getCurrentWorkingDirectory() const override - { - return _workDir; - } - - std::error_code setCurrentWorkingDirectory(const llvm::Twine& path_) override - { - // We don't verify if the working directory exists on purpose. - _workDir = path_.str(); - return std::error_code(); - } - -private: - llvm::IntrusiveRefCntPtr _FS; - std::string _workDir; - - llvm::SmallString<256> toAbsolute(const llvm::Twine& relPath_) const - { - llvm::SmallString<256> absPath; - relPath_.toVector(absPath); - llvm::sys::fs::make_absolute(_workDir, absPath); - return absPath; - } -}; - -} // parser -} // cc - -#endif // CC_PARSER_FAKEWORKDIRFILESYSTEM_H From cca922ec6774e61bf20db17f68c7f55141eaf2f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Mon, 4 Apr 2022 20:41:11 +0200 Subject: [PATCH 07/11] Save build directory in database The reparser needs this information to correctly execute its commands. --- model/CMakeLists.txt | 1 + model/include/model/builddirectory.h | 37 ++++++++++++++++++++++++++++ plugins/cpp/parser/src/cppparser.cpp | 7 ++++++ 3 files changed, 45 insertions(+) create mode 100644 model/include/model/builddirectory.h diff --git a/model/CMakeLists.txt b/model/CMakeLists.txt index edcd10726..424bf4277 100644 --- a/model/CMakeLists.txt +++ b/model/CMakeLists.txt @@ -1,5 +1,6 @@ set(ODB_SOURCES include/model/buildaction.h + include/model/builddirectory.h include/model/buildlog.h include/model/buildsourcetarget.h include/model/filecontent.h diff --git a/model/include/model/builddirectory.h b/model/include/model/builddirectory.h new file mode 100644 index 000000000..b597f7734 --- /dev/null +++ b/model/include/model/builddirectory.h @@ -0,0 +1,37 @@ +#ifndef CC_MODEL_BUILDDIRECTORY_H +#define CC_MODEL_BUILDDIRECTORY_H + +#include + +#include + +#include +#include + +namespace cc +{ +namespace model +{ + +struct BuildAction; + +#pragma db object +struct BuildDirectory +{ + #pragma db id auto + std::uint64_t id; + + #pragma db not_null + std::string directory; + + #pragma db not_null + #pragma db on_delete(cascade) + std::shared_ptr action; +}; + +typedef std::shared_ptr BuildDirectoryPtr; + +} // model +} // cc + +#endif // CC_MODEL_BUILDDIRECTORY_H diff --git a/plugins/cpp/parser/src/cppparser.cpp b/plugins/cpp/parser/src/cppparser.cpp index aec9aa287..e681e962b 100644 --- a/plugins/cpp/parser/src/cppparser.cpp +++ b/plugins/cpp/parser/src/cppparser.cpp @@ -17,6 +17,8 @@ #include #include +#include +#include #include #include #include @@ -254,6 +256,7 @@ void CppParser::addCompileCommand( std::vector sources; std::vector targets; + model::BuildDirectory buildDir; for (const auto& srcTarget : extractInputOutputs(command_)) { @@ -277,6 +280,9 @@ void CppParser::addCompileCommand( targets.push_back(std::move(buildTarget)); } + + buildDir.directory = command_.Directory; + buildDir.action = buildAction_; _ctx.srcMgr.persistFiles(); @@ -285,6 +291,7 @@ void CppParser::addCompileCommand( _ctx.db->persist(buildSource); for (model::BuildTarget buildTarget : targets) _ctx.db->persist(buildTarget); + _ctx.db->persist(buildDir); }); } From 10de27657453eb1d484ec6ce2387340ae2252a5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Mon, 4 Apr 2022 20:52:41 +0200 Subject: [PATCH 08/11] Use saved build directory when invoking Clang tool in reparser --- plugins/cpp_reparse/service/src/reparser.cpp | 34 ++++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/plugins/cpp_reparse/service/src/reparser.cpp b/plugins/cpp_reparse/service/src/reparser.cpp index 478505061..ffdea99ba 100644 --- a/plugins/cpp_reparse/service/src/reparser.cpp +++ b/plugins/cpp_reparse/service/src/reparser.cpp @@ -4,6 +4,8 @@ #include #include +#include +#include #include #include #include @@ -19,6 +21,7 @@ namespace { +typedef odb::query BuildDirQuery; typedef odb::query BuildSourceQuery; typedef odb::result BuildSourceResult; typedef odb::query FileQuery; @@ -64,14 +67,14 @@ boost::variant< CppReparser::getCompilationCommandForFile( const core::FileId& fileId_) { - std::string buildCommand; + cc::model::BuildActionPtr buildAction; _transaction([&, this](){ BuildSourceResult bss = _db->query( BuildSourceQuery::file == std::stoull(fileId_)); for (auto bs : bss) - if (buildCommand.empty()) - buildCommand = bs.action->command; + if (!buildAction || buildAction->command.empty()) + buildAction = bs.action; else { LOG(warning) << "Multiple build commands present for source file #" @@ -83,17 +86,32 @@ CppReparser::getCompilationCommandForFile( } }); - if (buildCommand.empty()) + if (!buildAction) { return "Build command not found for the file! Is this not a C++ file, " "or a header?"; } + std::string buildDir; + + _transaction([&, this](){ + cc::model::BuildDirectoryPtr bd = _db->query_one( + BuildDirQuery::action == buildAction->id); + if (bd) + buildDir = bd->directory; + else + { + LOG(debug) << "No build directory for source file #" + << std::stoull(fileId_) << ". Using '/'."; + buildDir = "/"; + } + }); + //--- Assemble compiler command line ---// std::vector commandLine; std::vector split; - boost::split(split, buildCommand, boost::is_any_of(" ")); + boost::split(split, buildAction->command, boost::is_any_of(" ")); commandLine.reserve(split.size()); commandLine.push_back("--"); @@ -133,7 +151,8 @@ CppReparser::getCompilationCommandForFile( FixedCompilationDatabase::loadFromCommandLine( argc, commandLine.data(), - compilationDbLoadError)); + compilationDbLoadError, + buildDir)); if (!compilationDb) { @@ -164,7 +183,8 @@ CppReparser::getASTForTranslationUnitFile( IntrusiveRefCntPtr dbfs( new DatabaseFileSystem(_db)); IntrusiveRefCntPtr overlayFs( - new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem())); + new llvm::vfs::OverlayFileSystem( + llvm::vfs::createPhysicalFileSystem().release())); overlayFs->pushOverlay(dbfs); auto compileDb = std::move( From ecae845f6325fb8e9d6e4a2348a7b9c86e258eda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Mon, 4 Apr 2022 21:07:21 +0200 Subject: [PATCH 09/11] Convert all path arguments to canonical in DatabaseFileSystem --- .../service/src/databasefilesystem.cpp | 42 ++++++++++++++----- .../service/src/databasefilesystem.h | 4 ++ 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/plugins/cpp_reparse/service/src/databasefilesystem.cpp b/plugins/cpp_reparse/service/src/databasefilesystem.cpp index fe560b628..b339918f9 100644 --- a/plugins/cpp_reparse/service/src/databasefilesystem.cpp +++ b/plugins/cpp_reparse/service/src/databasefilesystem.cpp @@ -201,7 +201,7 @@ DatabaseFileSystem::DatabaseFileSystem(std::shared_ptr db_) ErrorOr DatabaseFileSystem::status(const Twine& path_) { - model::FilePtr file = getFile(*_db, path_.str()); + model::FilePtr file = getFile(*_db, toCanonicalOrSame(path_)); if (!file) return std::error_code(ENOENT, std::generic_category()); @@ -211,7 +211,7 @@ ErrorOr DatabaseFileSystem::status(const Twine& path_) ErrorOr> DatabaseFileSystem::openFileForRead(const Twine& path_) { - model::FilePtr file = getFile(*_db, path_.str()); + model::FilePtr file = getFile(*_db, toCanonicalOrSame(path_)); if (!file) return std::error_code(ENOENT, std::generic_category()); if (file->type == model::File::DIRECTORY_TYPE) @@ -231,7 +231,7 @@ DatabaseFileSystem::openFileForRead(const Twine& path_) directory_iterator DatabaseFileSystem::dir_begin(const Twine& dir_, std::error_code& ec_) { - model::FilePtr dirFile = getFile(*_db, dir_.str()); + model::FilePtr dirFile = getFile(*_db, toCanonicalOrSame(dir_)); if (dirFile && dirFile->type == model::File::DIRECTORY_TYPE) return directory_iterator(std::make_shared( *_db, dirFile, ec_)); @@ -248,16 +248,36 @@ ErrorOr DatabaseFileSystem::getCurrentWorkingDirectory() const std::error_code DatabaseFileSystem::setCurrentWorkingDirectory(const Twine& path_) { - SmallString<128> fullPath; - sys::fs::make_absolute(_currentWorkingDirectory, fullPath); - sys::path::append(fullPath, path_); - std::error_code error = sys::fs::make_absolute(fullPath); + llvm::ErrorOr newWorkDir = toCanonical(path_); + if (!newWorkDir.getError()) + _currentWorkingDirectory = *newWorkDir; - if (!error) - _currentWorkingDirectory = fullPath.str().str(); + return newWorkDir.getError(); +} + +llvm::ErrorOr +DatabaseFileSystem::toCanonical(const llvm::Twine& relPath_) const +{ + llvm::SmallString<128> absPath, canonicalPath; + + relPath_.toVector(absPath); + + llvm::sys::fs::make_absolute(_currentWorkingDirectory, absPath); - getCurrentWorkingDirectory(); - return error; + if (std::error_code ec = llvm::sys::fs::real_path(absPath, canonicalPath)) + return ec; + + return canonicalPath.str(); +} + +std::string +DatabaseFileSystem::toCanonicalOrSame(const llvm::Twine& relPath_) const +{ + auto canonical = toCanonical(relPath_); + if (canonical.getError()) + return relPath_.str(); + else + return *canonical; } } // namespace reparse diff --git a/plugins/cpp_reparse/service/src/databasefilesystem.h b/plugins/cpp_reparse/service/src/databasefilesystem.h index 48a3ceb9b..51485e7c7 100644 --- a/plugins/cpp_reparse/service/src/databasefilesystem.h +++ b/plugins/cpp_reparse/service/src/databasefilesystem.h @@ -44,6 +44,10 @@ class DatabaseFileSystem : public llvm::vfs::FileSystem util::OdbTransaction _transaction; std::string _currentWorkingDirectory; + + llvm::ErrorOr toCanonical(const llvm::Twine& relPath_) const; + + std::string toCanonicalOrSame(const llvm::Twine& relPath_) const; }; } //namespace reparse From e0c6dba4657305417b5f3d83d54c5d947a4ab6f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Tue, 5 Apr 2022 01:39:33 +0200 Subject: [PATCH 10/11] If the build directory doesn't exist, use '/' and emit warning --- plugins/cpp/parser/src/cppparser.cpp | 10 +++++++++- plugins/cpp_reparse/service/src/reparser.cpp | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/plugins/cpp/parser/src/cppparser.cpp b/plugins/cpp/parser/src/cppparser.cpp index e681e962b..657e89954 100644 --- a/plugins/cpp/parser/src/cppparser.cpp +++ b/plugins/cpp/parser/src/cppparser.cpp @@ -310,13 +310,21 @@ int CppParser::parseWorker(const clang::tooling::CompileCommand& command_) int argc = commandLine.size(); + std::string buildDir = command_.Directory; + if (!fs::is_directory(buildDir)) + { + LOG(debug) << "Compilation directory " << buildDir + << " is missing, using '/' instead."; + buildDir = "/"; + } + std::string compilationDbLoadError; std::unique_ptr compilationDb( clang::tooling::FixedCompilationDatabase::loadFromCommandLine( argc, commandLine.data(), compilationDbLoadError, - command_.Directory)); + buildDir)); if (!compilationDb) { diff --git a/plugins/cpp_reparse/service/src/reparser.cpp b/plugins/cpp_reparse/service/src/reparser.cpp index ffdea99ba..41abf7ea8 100644 --- a/plugins/cpp_reparse/service/src/reparser.cpp +++ b/plugins/cpp_reparse/service/src/reparser.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -107,6 +108,13 @@ CppReparser::getCompilationCommandForFile( } }); + if (!boost::filesystem::is_directory(buildDir)) + { + LOG(debug) << "Compilation directory " << buildDir + << " is missing, using '/' instead."; + buildDir = "/"; + } + //--- Assemble compiler command line ---// std::vector commandLine; From 03cb8e99c0cc6804155512331f41fd54b83eb75a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20D=C3=B3czi?= Date: Mon, 4 Apr 2022 23:33:06 +0200 Subject: [PATCH 11/11] Fix segfault in PPIncludeCallback::InclusionDirective --- plugins/cpp/parser/src/ppincludecallback.cpp | 3 +++ plugins/cpp/parser/src/ppmacrocallback.cpp | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/cpp/parser/src/ppincludecallback.cpp b/plugins/cpp/parser/src/ppincludecallback.cpp index af3dfee8f..b2f9b730c 100644 --- a/plugins/cpp/parser/src/ppincludecallback.cpp +++ b/plugins/cpp/parser/src/ppincludecallback.cpp @@ -70,6 +70,9 @@ void PPIncludeCallback::InclusionDirective( if (searchPath_.empty()) return; + if (!file_) + return; + clang::SourceLocation expLoc = _clangSrcMgr.getExpansionLoc(hashLoc_); clang::PresumedLoc presLoc = _clangSrcMgr.getPresumedLoc(expLoc); diff --git a/plugins/cpp/parser/src/ppmacrocallback.cpp b/plugins/cpp/parser/src/ppmacrocallback.cpp index bebcefc62..b8d0c8508 100644 --- a/plugins/cpp/parser/src/ppmacrocallback.cpp +++ b/plugins/cpp/parser/src/ppmacrocallback.cpp @@ -253,7 +253,7 @@ std::string PPMacroCallback::getUSR(const clang::MacroInfo* mi_) std::string locStr = std::to_string(presLoc.getLine()) + ":" + std::to_string(presLoc.getColumn()) + ":"; - + std::string filePath = _fileLocUtil.getFilePath(presLoc.getFileID()); return locStr