diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h @@ -16,6 +16,7 @@ #include "clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h" #include "clang/Tooling/DependencyScanning/DependencyScanningService.h" #include "clang/Tooling/DependencyScanning/ModuleDepCollector.h" +#include "clang/Tooling/Tooling.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" #include @@ -79,9 +80,9 @@ /// dependencies. This filesystem persists across multiple compiler /// invocations. llvm::IntrusiveRefCntPtr DepFS; - /// The file manager that is reused across multiple invocations by this - /// worker. If null, the file manager will not be reused. - llvm::IntrusiveRefCntPtr Files; + /// The container of file managers that is reused across multiple invocations + /// by this worker. If null, the file manager will not be reused. + llvm::IntrusiveRefCntPtr Files; ScanningOutputFormat Format; /// Whether to optimize the modules' command-line arguments. bool OptimizeArgs; diff --git a/clang/include/clang/Tooling/Tooling.h b/clang/include/clang/Tooling/Tooling.h --- a/clang/include/clang/Tooling/Tooling.h +++ b/clang/include/clang/Tooling/Tooling.h @@ -65,6 +65,43 @@ class CompilationDatabase; +class ToolFileManager : public RefCountedBase { + // ToolFileManager is a container who stores file managers for each CWD + std::map> fileManagers; + IntrusiveRefCntPtr FS; + FileSystemOptions FSO; + +public: + ToolFileManager(const FileSystemOptions &FileSystemOpts, + IntrusiveRefCntPtr FS) + : FS(FS), FSO(FileSystemOpts){}; + + ~ToolFileManager() = default; + + llvm::IntrusiveRefCntPtr getOrCreateFileManager() { + auto Success = FS->getCurrentWorkingDirectory(); + if (!Success) + llvm::report_fatal_error("Cannot get current working directory!\n"); + std::string Cwd = Success.get(); + if (!fileManagers.count(Cwd)) { + fileManagers[Cwd] = new FileManager(FSO, FS); + } + return fileManagers[Cwd]; + } + + void setVirtualFileSystem(IntrusiveRefCntPtr NewFS) { + // To call FileManager::setVirtualFileSystem() directly might be unsound + // unless the new FS is equivalent to the old one. So here we simply + // invalidate all previous file managers to ensure correctness. + if (this->FS != NewFS) { + for (auto it = fileManagers.begin(); it != fileManagers.end(); it++) + it->second->setVirtualFileSystem(NewFS); + fileManagers.clear(); + this->FS = std::move(NewFS); + } + } +}; + /// Retrieves the flags of the `-cc1` job in `Compilation` that has only source /// files as its inputs. /// Returns nullptr if there are no such jobs or multiple of them. Note that @@ -84,7 +121,7 @@ /// Perform an action for an invocation. virtual bool runInvocation(std::shared_ptr Invocation, - FileManager *Files, + ToolFileManager *Files, std::shared_ptr PCHContainerOps, DiagnosticConsumer *DiagConsumer) = 0; }; @@ -101,7 +138,7 @@ /// Invokes the compiler with a FrontendAction created by create(). bool runInvocation(std::shared_ptr Invocation, - FileManager *Files, + ToolFileManager *Files, std::shared_ptr PCHContainerOps, DiagnosticConsumer *DiagConsumer) override; @@ -245,12 +282,13 @@ /// Callers have to ensure that they are installed in a compatible location /// (see clang driver implementation) or mapped in via mapVirtualFile. /// \param FAction The action to be executed. - /// \param Files The FileManager used for the execution. Class does not take - /// ownership. + /// \param Files The ToolFileManager used for the execution. Class does not + /// take ownership. /// \param PCHContainerOps The PCHContainerOperations for loading and creating /// clang modules. ToolInvocation(std::vector CommandLine, - std::unique_ptr FAction, FileManager *Files, + std::unique_ptr FAction, + ToolFileManager *Files, std::shared_ptr PCHContainerOps = std::make_shared()); @@ -258,11 +296,11 @@ /// /// \param CommandLine The command line arguments to clang. /// \param Action The action to be executed. - /// \param Files The FileManager used for the execution. + /// \param Files The ToolFileManager used for the execution. /// \param PCHContainerOps The PCHContainerOperations for loading and creating /// clang modules. ToolInvocation(std::vector CommandLine, ToolAction *Action, - FileManager *Files, + ToolFileManager *Files, std::shared_ptr PCHContainerOps); ~ToolInvocation(); @@ -292,7 +330,7 @@ std::vector CommandLine; ToolAction *Action; bool OwnsAction; - FileManager *Files; + ToolFileManager *Files; std::shared_ptr PCHContainerOps; DiagnosticConsumer *DiagConsumer = nullptr; DiagnosticOptions *DiagOpts = nullptr; @@ -317,15 +355,15 @@ /// clang modules. /// \param BaseFS VFS used for all underlying file accesses when running the /// tool. - /// \param Files The file manager to use for underlying file operations when - /// running the tool. + /// \param Files The container of file managers to use for underlying file + /// operations when running the tool. ClangTool(const CompilationDatabase &Compilations, ArrayRef SourcePaths, std::shared_ptr PCHContainerOps = std::make_shared(), IntrusiveRefCntPtr BaseFS = llvm::vfs::getRealFileSystem(), - IntrusiveRefCntPtr Files = nullptr); + IntrusiveRefCntPtr Files = nullptr); ~ClangTool(); @@ -373,7 +411,7 @@ /// Returns the file manager used in the tool. /// /// The file manager is shared between all translation units. - FileManager &getFiles() { return *Files; } + FileManager &getFiles() { return *Files->getOrCreateFileManager(); } llvm::ArrayRef getSourcePaths() const { return SourcePaths; } @@ -384,7 +422,7 @@ llvm::IntrusiveRefCntPtr OverlayFileSystem; llvm::IntrusiveRefCntPtr InMemoryFileSystem; - llvm::IntrusiveRefCntPtr Files; + llvm::IntrusiveRefCntPtr Files; // Contains a list of pairs (, ). std::vector> MappedFileContents; diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -145,7 +145,7 @@ OptimizeArgs(OptimizeArgs), ModuleName(ModuleName) {} bool runInvocation(std::shared_ptr Invocation, - FileManager *FileMgr, + ToolFileManager *ToolFileMgr, std::shared_ptr PCHContainerOps, DiagnosticConsumer *DiagConsumer) override { // Make a deep copy of the original Clang invocation. @@ -166,7 +166,7 @@ ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false; ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false; - + auto FileMgr = ToolFileMgr->getOrCreateFileManager().get(); FileMgr->getFileSystemOpts().WorkingDir = std::string(WorkingDirectory); ScanInstance.setFileManager(FileMgr); ScanInstance.createSourceManager(*FileMgr); @@ -199,7 +199,7 @@ // Support for virtual file system overlays on top of the caching // filesystem. - FileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation( + ToolFileMgr->setVirtualFileSystem(createVFSFromCompilerInvocation( ScanInstance.getInvocation(), ScanInstance.getDiagnostics(), DepFS)); // Pass the skip mappings which should speed up excluded conditional block @@ -291,7 +291,7 @@ DepFS = new DependencyScanningWorkerFilesystem(Service.getSharedCache(), RealFS, PPSkipMappings); if (Service.canReuseFileManager()) - Files = new FileManager(FileSystemOptions(), RealFS); + Files = new ToolFileManager(FileSystemOptions(), RealFS); } static llvm::Error @@ -320,8 +320,8 @@ if (Files) Files->setVirtualFileSystem(RealFS); - llvm::IntrusiveRefCntPtr CurrentFiles = - Files ? Files : new FileManager(FileSystemOptions(), RealFS); + llvm::IntrusiveRefCntPtr CurrentFiles = + Files ? Files : new ToolFileManager(FileSystemOptions(), RealFS); Optional> ModifiedCommandLine; if (ModuleName.hasValue()) { diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -207,8 +207,8 @@ SmallString<16> FileNameStorage; StringRef FileNameRef = FileName.toNullTerminatedStringRef(FileNameStorage); - llvm::IntrusiveRefCntPtr Files( - new FileManager(FileSystemOptions(), VFS)); + llvm::IntrusiveRefCntPtr Files( + new ToolFileManager(FileSystemOptions(), VFS)); ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster(); ToolInvocation Invocation( getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef), @@ -320,13 +320,14 @@ ToolInvocation::ToolInvocation( std::vector CommandLine, ToolAction *Action, - FileManager *Files, std::shared_ptr PCHContainerOps) + ToolFileManager *Files, + std::shared_ptr PCHContainerOps) : CommandLine(std::move(CommandLine)), Action(Action), OwnsAction(false), Files(Files), PCHContainerOps(std::move(PCHContainerOps)) {} ToolInvocation::ToolInvocation( std::vector CommandLine, - std::unique_ptr FAction, FileManager *Files, + std::unique_ptr FAction, ToolFileManager *Files, std::shared_ptr PCHContainerOps) : CommandLine(std::move(CommandLine)), Action(new SingleFrontendActionFactory(std::move(FAction))), @@ -359,16 +360,17 @@ &*DiagOpts, DiagConsumer ? DiagConsumer : &DiagnosticPrinter, false); // Although `Diagnostics` are used only for command-line parsing, the custom // `DiagConsumer` might expect a `SourceManager` to be present. - SourceManager SrcMgr(*Diagnostics, *Files); + SourceManager SrcMgr(*Diagnostics, *Files->getOrCreateFileManager()); Diagnostics->setSourceManager(&SrcMgr); const std::unique_ptr Driver( - newDriver(&*Diagnostics, BinaryName, &Files->getVirtualFileSystem())); + newDriver(&*Diagnostics, BinaryName, + &Files->getOrCreateFileManager()->getVirtualFileSystem())); // The "input file not found" diagnostics from the driver are useful. // The driver is only aware of the VFS working directory, but some clients // change this at the FileManager level instead. // In this case the checks have false positives, so skip them. - if (!Files->getFileSystemOpts().WorkingDir.empty()) + if (!Files->getOrCreateFileManager()->getFileSystemOpts().WorkingDir.empty()) Driver->setCheckInputsExist(false); const std::unique_ptr Compilation( Driver->BuildCompilation(llvm::makeArrayRef(Argv))); @@ -400,13 +402,13 @@ } bool FrontendActionFactory::runInvocation( - std::shared_ptr Invocation, FileManager *Files, + std::shared_ptr Invocation, ToolFileManager *Files, std::shared_ptr PCHContainerOps, DiagnosticConsumer *DiagConsumer) { // Create a compiler instance to handle the actual work. CompilerInstance Compiler(std::move(PCHContainerOps)); Compiler.setInvocation(std::move(Invocation)); - Compiler.setFileManager(Files); + Compiler.setFileManager(Files->getOrCreateFileManager().get()); // The FrontendAction can have lifetime requirements for Compiler or its // members, and we need to ensure it's deleted earlier than Compiler. So we @@ -418,11 +420,11 @@ if (!Compiler.hasDiagnostics()) return false; - Compiler.createSourceManager(*Files); + Compiler.createSourceManager(*Files->getOrCreateFileManager()); const bool Success = Compiler.ExecuteAction(*ScopedToolAction); - Files->clearStatCache(); + Files->getOrCreateFileManager()->clearStatCache(); return Success; } @@ -430,13 +432,14 @@ ArrayRef SourcePaths, std::shared_ptr PCHContainerOps, IntrusiveRefCntPtr BaseFS, - IntrusiveRefCntPtr Files) + IntrusiveRefCntPtr Files) : Compilations(Compilations), SourcePaths(SourcePaths), PCHContainerOps(std::move(PCHContainerOps)), OverlayFileSystem(new llvm::vfs::OverlayFileSystem(std::move(BaseFS))), InMemoryFileSystem(new llvm::vfs::InMemoryFileSystem), Files(Files ? Files - : new FileManager(FileSystemOptions(), OverlayFileSystem)) { + : new clang::tooling::ToolFileManager(FileSystemOptions(), + OverlayFileSystem)) { OverlayFileSystem->pushOverlay(InMemoryFileSystem); appendArgumentsAdjuster(getClangStripOutputAdjuster()); appendArgumentsAdjuster(getClangSyntaxOnlyAdjuster()); @@ -601,7 +604,7 @@ ASTBuilderAction(std::vector> &ASTs) : ASTs(ASTs) {} bool runInvocation(std::shared_ptr Invocation, - FileManager *Files, + ToolFileManager *Files, std::shared_ptr PCHContainerOps, DiagnosticConsumer *DiagConsumer) override { std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation( @@ -609,7 +612,7 @@ CompilerInstance::createDiagnostics(&Invocation->getDiagnosticOpts(), DiagConsumer, /*ShouldOwnClient=*/false), - Files); + Files->getOrCreateFileManager().get()); if (!AST) return false; @@ -655,8 +658,8 @@ llvm::IntrusiveRefCntPtr InMemoryFileSystem( new llvm::vfs::InMemoryFileSystem); OverlayFileSystem->pushOverlay(InMemoryFileSystem); - llvm::IntrusiveRefCntPtr Files( - new FileManager(FileSystemOptions(), OverlayFileSystem)); + llvm::IntrusiveRefCntPtr Files( + new ToolFileManager(FileSystemOptions(), OverlayFileSystem)); ToolInvocation Invocation( getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileName), FileName), diff --git a/clang/test/Tooling/multiple-source-include-different-header-with-same-relative-path.cpp b/clang/test/Tooling/multiple-source-include-different-header-with-same-relative-path.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Tooling/multiple-source-include-different-header-with-same-relative-path.cpp @@ -0,0 +1,42 @@ +// This test case presents a circumstance that the information related one file +// is misused by another. +// +// The path tree of the test directory is as follors. +// . +// ├── a +// │ ├── a.c +// │ └── config.h +// ├── b +// │ ├── b.c +// │ └── config.h +// └── compile_commands.json +// +// Both source files (a/a.c and b/b.c) includes the config.h file of their own +// directory with `#include "config.h"`. However, the two config.h files are +// different. File a/config.h is longer than b/config.h. Both a/a.c and b/b.c +// are compiled in their own directory, which is recorded in the compilation +// database. +// +// When using ClangTool to parse these two source files one by one, since the +// file name of both header files are the same, the FileManager will confuse +// with them and using the file entry of a/config.h for b/config.h. And the +// wrong file length will lead to a buffer overflow when reading the file. +// +// In this test case, to avoid buffer overflow, we use the leading '\0' in an +// empty buffer to trigger the problem. We set a/config.h as an empty line +// comment, and leave b/config.h empty. Firstly, a/config.h is read and cached, +// then when reading b/config.h, if the size of a/config.h is used, the first +// two chars are read and the first one must be a '\0'. + +// RUN: rm -rf %t +// RUN: mkdir -p %t/a +// RUN: mkdir -p %t/b +// RUN: echo '#include "config.h"' > %t/a/a.c +// RUN: echo '#include "config.h"' > %t/b/b.c +// RUN: echo '//' > %t/a/config.h +// RUN: echo '' > %t/b/config.h +// RUN: echo '[{"arguments": ["cc", "-c", "-o", "a.o", "a.c"], "directory": "%t/a", "file": "a.c"}, {"arguments": ["cc", "-c", "-o", "b.o", "b.c"], "directory": "%t/b", "file": "b.c"}]' | sed -e 's/\\/\\\\/g' > %t/compile_commands.json + +// The following two test RUNs should have no output. +// RUN: cd %t && clang-check a/a.c b/b.c 2>&1 | count 0 +// RUN: cd %t && clang-check b/b.c a/a.c 2>&1 | count 0 diff --git a/clang/unittests/Tooling/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScannerTest.cpp --- a/clang/unittests/Tooling/DependencyScannerTest.cpp +++ b/clang/unittests/Tooling/DependencyScannerTest.cpp @@ -51,18 +51,18 @@ TestDependencyScanningAction(std::vector &Deps) : Deps(Deps) {} bool runInvocation(std::shared_ptr Invocation, - FileManager *FileMgr, + ToolFileManager *FileMgr, std::shared_ptr PCHContainerOps, DiagnosticConsumer *DiagConsumer) override { CompilerInstance Compiler(std::move(PCHContainerOps)); Compiler.setInvocation(std::move(Invocation)); - Compiler.setFileManager(FileMgr); + Compiler.setFileManager(FileMgr->getOrCreateFileManager().get()); Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false); if (!Compiler.hasDiagnostics()) return false; - Compiler.createSourceManager(*FileMgr); + Compiler.createSourceManager(*FileMgr->getOrCreateFileManager()); Compiler.addDependencyCollector(std::make_shared( Compiler.getInvocation().getDependencyOutputOpts(), Deps)); diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp --- a/clang/unittests/Tooling/ToolingTest.cpp +++ b/clang/unittests/Tooling/ToolingTest.cpp @@ -179,8 +179,8 @@ llvm::IntrusiveRefCntPtr InMemoryFileSystem( new llvm::vfs::InMemoryFileSystem); OverlayFileSystem->pushOverlay(InMemoryFileSystem); - llvm::IntrusiveRefCntPtr Files( - new FileManager(FileSystemOptions(), OverlayFileSystem)); + llvm::IntrusiveRefCntPtr Files( + new ToolFileManager(FileSystemOptions(), OverlayFileSystem)); std::vector Args; Args.push_back("tool-executable"); Args.push_back("-Idef"); @@ -205,8 +205,8 @@ llvm::IntrusiveRefCntPtr InMemoryFileSystem( new llvm::vfs::InMemoryFileSystem); OverlayFileSystem->pushOverlay(InMemoryFileSystem); - llvm::IntrusiveRefCntPtr Files( - new FileManager(FileSystemOptions(), OverlayFileSystem)); + llvm::IntrusiveRefCntPtr Files( + new ToolFileManager(FileSystemOptions(), OverlayFileSystem)); std::vector Args; Args.push_back("tool-executable"); Args.push_back("-Idef"); @@ -231,8 +231,8 @@ llvm::IntrusiveRefCntPtr InMemoryFileSystem( new llvm::vfs::InMemoryFileSystem); OverlayFileSystem->pushOverlay(InMemoryFileSystem); - llvm::IntrusiveRefCntPtr Files( - new FileManager(FileSystemOptions(), OverlayFileSystem)); + llvm::IntrusiveRefCntPtr Files( + new ToolFileManager(FileSystemOptions(), OverlayFileSystem)); std::vector Args; Args.push_back("tool-executable"); @@ -260,8 +260,8 @@ llvm::IntrusiveRefCntPtr InMemoryFileSystem( new llvm::vfs::InMemoryFileSystem); OverlayFileSystem->pushOverlay(InMemoryFileSystem); - llvm::IntrusiveRefCntPtr Files( - new FileManager(FileSystemOptions(), OverlayFileSystem)); + llvm::IntrusiveRefCntPtr Files( + new ToolFileManager(FileSystemOptions(), OverlayFileSystem)); std::vector Args; Args.push_back("tool-executable"); @@ -306,8 +306,8 @@ llvm::IntrusiveRefCntPtr InMemoryFileSystem( new llvm::vfs::InMemoryFileSystem); OverlayFileSystem->pushOverlay(InMemoryFileSystem); - llvm::IntrusiveRefCntPtr Files( - new FileManager(FileSystemOptions(), OverlayFileSystem)); + llvm::IntrusiveRefCntPtr Files( + new ToolFileManager(FileSystemOptions(), OverlayFileSystem)); std::vector Args; Args.push_back("tool-executable"); // Note: intentional error; user probably meant -ferror-limit=0.