r308962 - Fix incorrect use of current directory to find moved paths in ASTReader.

Manuel Klimek via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 25 03:22:07 PDT 2017


Author: klimek
Date: Tue Jul 25 03:22:06 2017
New Revision: 308962

URL: http://llvm.org/viewvc/llvm-project?rev=308962&view=rev
Log:
Fix incorrect use of current directory to find moved paths in ASTReader.

CurrentDir was set as the path of the current module, but that can change as
part of a chain of loaded modules.

When we try to locate a file mentioned in a module that does not exist, we use
a heuristic to look at the relative path between the original location of the
module and the file we look for, and use that relatively to the CurrentDir.

This only works if CurrentDir is the same as the (current) path of the module
file the file was mentioned in; if it is not, we look at the path relatively to
the wrong directory, and can end up reading random unrelated files that happen
to have the same name.

This patch fixes this by using the BaseDirectory of the module file the file
we look for was mentioned in instead of the CurrentDir heuristic.

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

Added:
    cfe/trunk/test/Modules/path-resolution.modulemap
Modified:
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=308962&r1=308961&r2=308962&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Tue Jul 25 03:22:06 2017
@@ -867,9 +867,6 @@ private:
   SmallVector<ImportedSubmodule, 2> ImportedModules;
   //@}
 
-  /// \brief The directory that the PCH we are reading is stored in.
-  std::string CurrentDir;
-
   /// \brief The system include root to be used when loading the
   /// precompiled header.
   std::string isysroot;

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=308962&r1=308961&r2=308962&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Jul 25 03:22:06 2017
@@ -2060,14 +2060,12 @@ InputFile ASTReader::getInputFile(Module
   StringRef Filename = FI.Filename;
 
   const FileEntry *File = FileMgr.getFile(Filename, /*OpenFile=*/false);
-
   // If we didn't find the file, resolve it relative to the
   // original directory from which this AST file was created.
-  if (File == nullptr && !F.OriginalDir.empty() && !CurrentDir.empty() &&
-      F.OriginalDir != CurrentDir) {
-    std::string Resolved = resolveFileRelativeToOriginalDir(Filename,
-                                                            F.OriginalDir,
-                                                            CurrentDir);
+  if (File == nullptr && !F.OriginalDir.empty() && !F.BaseDirectory.empty() &&
+      F.OriginalDir != F.BaseDirectory) {
+    std::string Resolved = resolveFileRelativeToOriginalDir(
+        Filename, F.OriginalDir, F.BaseDirectory);
     if (!Resolved.empty())
       File = FileMgr.getFile(Resolved);
   }
@@ -4065,13 +4063,6 @@ ASTReader::ReadASTCore(StringRef FileNam
 
   assert(M && "Missing module file");
 
-  // FIXME: This seems rather a hack. Should CurrentDir be part of the
-  // module?
-  if (FileName != "-") {
-    CurrentDir = llvm::sys::path::parent_path(FileName);
-    if (CurrentDir.empty()) CurrentDir = ".";
-  }
-
   ModuleFile &F = *M;
   BitstreamCursor &Stream = F.Stream;
   Stream = BitstreamCursor(PCHContainerRdr.ExtractPCH(*F.Buffer));

Added: cfe/trunk/test/Modules/path-resolution.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/path-resolution.modulemap?rev=308962&view=auto
==============================================================================
--- cfe/trunk/test/Modules/path-resolution.modulemap (added)
+++ cfe/trunk/test/Modules/path-resolution.modulemap Tue Jul 25 03:22:06 2017
@@ -0,0 +1,70 @@
+// RUN: rm -rf %t
+//
+// First, create two modules a and b, with a dependency b -> a, both within
+// the same directory p1.
+//
+// RUN: mkdir -p %t/p1
+// RUN: cd %t/p1
+//
+// RUN: grep "<AM>" %s > %t/p1/a.modulemap
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fmodule-map-file-home-is-cwd \
+// RUN:   -fmodules-embed-all-files -fmodules-local-submodule-visibility \
+// RUN:   -fmodule-name="a" -o a.pcm a.modulemap
+//
+// RUN: grep "<BM>" %s > %t/p1/b.modulemap
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fmodule-map-file-home-is-cwd \
+// RUN:   -fmodules-embed-all-files -fmodules-local-submodule-visibility \
+// RUN:   -fmodule-name="b" -o b.pcm b.modulemap
+//
+// Next, move the whole tree p1 -> p2.
+//
+// RUN: cd %t
+// RUN: mv %t/p1 %t/p2
+// RUN: cd %t/p2
+//
+// Compile a new module c in the newly generated tree that depends on b; c.pcm
+// has to be within a subdirectory so a.modulemap will be one step up (../) from
+// c.pcm.
+//
+// RUN: mkdir %t/p2/c
+// RUN: grep "<CM>" %s > %t/p2/c/c.modulemap
+// RUN: grep "<CH>" %s > %t/p2/c/c.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fmodule-map-file-home-is-cwd \
+// RUN:   -fmodules-embed-all-files -fmodules-local-submodule-visibility \
+// RUN:   -fmodule-name="c" -fmodule-file=b.pcm -o c/c.pcm c/c.modulemap
+//
+// Delete a.modulemap from its original location, and instead inject a different
+// (unrelated) a.modulemap in the path p2/p2.
+//
+// RUN: rm %t/p2/a.modulemap
+// RUN: mkdir -p %t/p2/p2
+// RUN: touch %t/p2/p2/a.modulemap
+//
+// Now compile a file c.cpp that uses c.h and the module c; it is important
+// to first load b.pcm and a.pcm before c.pcm on the command line to trigger
+// the right order of module loading. This used to trigger clang to find the
+// p2/p2/a.modulemap via the path c/../p2/a.modulemap, which is not the correct
+// relative path from c.
+//
+// RUN: grep "<CC>" %s > %t/p2/c/c.cpp
+// RUN: %clang_cc1 -I. -x c++ -fmodules \
+// RUN:     -fmodule-file=b.pcm -fmodule-file=a.pcm -fmodule-file=c/c.pcm  \
+// RUN:     -o c/c.o -emit-obj c/c.cpp
+
+module "a" {                // <AM>
+}                           // <AM>
+
+module "b" {                // <BM>
+  use "a"                   // <BM>
+}                           // <BM>
+
+module "c" {                // <CM>
+  header "c/c.h"            // <CM>
+  use "a"                   // <CM>
+  use "b"                   // <CM>
+}                           // <CM>
+
+inline void c() {}          // <CH>
+
+#include "c/c.h"            // <CC>
+void foo() { c(); }         // <CC>




More information about the cfe-commits mailing list