r340937 - [Tooling] Do not restore working dir in ClangTool

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 29 09:35:31 PDT 2018


Author: ibiryukov
Date: Wed Aug 29 09:35:31 2018
New Revision: 340937

URL: http://llvm.org/viewvc/llvm-project?rev=340937&view=rev
Log:
[Tooling] Do not restore working dir in ClangTool

Summary:
Resolve all relative paths before running the tool instead.

This fixes the usage of ClangTool in AllTUsExecutor. The executor will
try running multiple ClangTool instances in parallel with compile
commands that usually have the same working directory.

Changing working directory is a global operation, so we end up
changing working directory in the middle of running other actions,
which leads to spurious compile errors.

Reviewers: ioeric, sammccall

Reviewed By: ioeric

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D51407

Modified:
    cfe/trunk/include/clang/Tooling/Tooling.h
    cfe/trunk/lib/Tooling/Tooling.cpp

Modified: cfe/trunk/include/clang/Tooling/Tooling.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Tooling.h?rev=340937&r1=340936&r2=340937&view=diff
==============================================================================
--- cfe/trunk/include/clang/Tooling/Tooling.h (original)
+++ cfe/trunk/include/clang/Tooling/Tooling.h Wed Aug 29 09:35:31 2018
@@ -459,6 +459,10 @@ inline std::unique_ptr<FrontendActionFac
 /// \param File Either an absolute or relative path.
 std::string getAbsolutePath(StringRef File);
 
+/// An overload of getAbsolutePath that works over the provided \p FS.
+llvm::Expected<std::string> getAbsolutePath(vfs::FileSystem &FS,
+                                            StringRef File);
+
 /// Changes CommandLine to contain implicit flags that would have been
 /// defined had the compiler driver been invoked through the path InvokedAs.
 ///

Modified: cfe/trunk/lib/Tooling/Tooling.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=340937&r1=340936&r2=340937&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Tooling.cpp (original)
+++ cfe/trunk/lib/Tooling/Tooling.cpp Wed Aug 29 09:35:31 2018
@@ -199,7 +199,8 @@ bool runToolOnCodeWithArgs(
                                FileName, ToolName);
 }
 
-std::string getAbsolutePath(StringRef File) {
+llvm::Expected<std::string> getAbsolutePath(vfs::FileSystem &FS,
+                                            StringRef File) {
   StringRef RelativePath(File);
   // FIXME: Should '.\\' be accepted on Win32?
   if (RelativePath.startswith("./")) {
@@ -207,13 +208,16 @@ std::string getAbsolutePath(StringRef Fi
   }
 
   SmallString<1024> AbsolutePath = RelativePath;
-  std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath);
-  assert(!EC);
-  (void)EC;
+  if (auto EC = FS.makeAbsolute(AbsolutePath))
+    return llvm::errorCodeToError(EC);
   llvm::sys::path::native(AbsolutePath);
   return AbsolutePath.str();
 }
 
+std::string getAbsolutePath(StringRef File) {
+  return llvm::cantFail(getAbsolutePath(*vfs::getRealFileSystem(), File));
+}
+
 void addTargetAndModeForProgramName(std::vector<std::string> &CommandLine,
                                     StringRef InvokedAs) {
   if (!CommandLine.empty() && !InvokedAs.empty()) {
@@ -411,15 +415,6 @@ int ClangTool::run(ToolAction *Action) {
   // This just needs to be some symbol in the binary.
   static int StaticSymbol;
 
-  std::string InitialDirectory;
-  if (llvm::ErrorOr<std::string> CWD =
-          OverlayFileSystem->getCurrentWorkingDirectory()) {
-    InitialDirectory = std::move(*CWD);
-  } else {
-    llvm::report_fatal_error("Cannot detect current path: " +
-                             Twine(CWD.getError().message()));
-  }
-
   // First insert all absolute paths into the in-memory VFS. These are global
   // for all compile commands.
   if (SeenWorkingDirectories.insert("/").second)
@@ -431,9 +426,22 @@ int ClangTool::run(ToolAction *Action) {
 
   bool ProcessingFailed = false;
   bool FileSkipped = false;
+  // Compute all absolute paths before we run any actions, as those will change
+  // the working directory.
+  std::vector<std::string> AbsolutePaths;
+  AbsolutePaths.reserve(SourcePaths.size());
   for (const auto &SourcePath : SourcePaths) {
-    std::string File(getAbsolutePath(SourcePath));
+    auto AbsPath = getAbsolutePath(*OverlayFileSystem, SourcePath);
+    if (!AbsPath) {
+      llvm::errs() << "Skipping " << SourcePath
+                   << ". Error while getting an absolute path: "
+                   << llvm::toString(AbsPath.takeError()) << "\n";
+      continue;
+    }
+    AbsolutePaths.push_back(std::move(*AbsPath));
+  }
 
+  for (llvm::StringRef File : AbsolutePaths) {
     // Currently implementations of CompilationDatabase::getCompileCommands can
     // change the state of the file system (e.g.  prepare generated headers), so
     // this method needs to run right before we invoke the tool, as the next
@@ -498,11 +506,6 @@ int ClangTool::run(ToolAction *Action) {
         llvm::errs() << "Error while processing " << File << ".\n";
         ProcessingFailed = true;
       }
-      // Return to the initial directory to correctly resolve next file by
-      // relative path.
-      if (OverlayFileSystem->setCurrentWorkingDirectory(InitialDirectory.c_str()))
-        llvm::report_fatal_error("Cannot chdir into \"" +
-                                 Twine(InitialDirectory) + "\n!");
     }
   }
   return ProcessingFailed ? 1 : (FileSkipped ? 2 : 0);




More information about the cfe-commits mailing list