r209534 - Stopgap fix for finding module for a file mapped in the VFS

Ben Langmuir blangmuir at apple.com
Fri May 23 11:15:48 PDT 2014


Author: benlangmuir
Date: Fri May 23 13:15:47 2014
New Revision: 209534

URL: http://llvm.org/viewvc/llvm-project?rev=209534&view=rev
Log:
Stopgap fix for finding module for a file mapped in the VFS

If we lookup a path using its 'real' path first, we need to ensure that
when we run header search we still use the VFS-mapped path or we will
not be able to find the corresponding module for the header.

The real problem is that we tie the name of a file to its underlying
FileEntry, which is uniqued by inode, so we only ever get the first name
it is looked up by. This doesn't work with modules, which rely on a
specific file system structure.  I'm hoping to have time to write up a
proposal for fixing this more permanently soon, but as a stopgap this
patch updates the name of the file's directory if it comes from a VFS
mapping.

Added:
    cfe/trunk/test/VFS/Inputs/import_some_frame.h
    cfe/trunk/test/VFS/Inputs/public_header2.h
    cfe/trunk/test/VFS/Inputs/some_frame_module.map
    cfe/trunk/test/VFS/real-path-found-first.m
Modified:
    cfe/trunk/include/clang/Basic/FileSystemStatCache.h
    cfe/trunk/include/clang/Basic/VirtualFileSystem.h
    cfe/trunk/lib/Basic/FileManager.cpp
    cfe/trunk/lib/Basic/FileSystemStatCache.cpp
    cfe/trunk/lib/Basic/VirtualFileSystem.cpp
    cfe/trunk/test/VFS/Inputs/actual_module.map
    cfe/trunk/test/VFS/Inputs/public_header.h
    cfe/trunk/test/VFS/Inputs/vfsoverlay.yaml

Modified: cfe/trunk/include/clang/Basic/FileSystemStatCache.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileSystemStatCache.h?rev=209534&r1=209533&r2=209534&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileSystemStatCache.h (original)
+++ cfe/trunk/include/clang/Basic/FileSystemStatCache.h Fri May 23 13:15:47 2014
@@ -38,6 +38,10 @@ struct FileData {
   bool IsDirectory;
   bool IsNamedPipe;
   bool InPCH;
+  bool IsVFSMapped; // FIXME: remove this when files support multiple names
+  FileData()
+      : Size(0), ModTime(0), IsDirectory(false), IsNamedPipe(false),
+        InPCH(false), IsVFSMapped(false) {}
 };
 
 /// \brief Abstract interface for introducing a FileManager cache for 'stat'

Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=209534&r1=209533&r2=209534&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Fri May 23 13:15:47 2014
@@ -40,6 +40,9 @@ class Status {
   llvm::sys::fs::perms Perms;
 
 public:
+  bool IsVFSMapped; // FIXME: remove when files support multiple names
+
+public:
   Status() : Type(llvm::sys::fs::file_type::status_error) {}
   Status(const llvm::sys::fs::file_status &Status);
   Status(StringRef Name, StringRef RealName, llvm::sys::fs::UniqueID UID,

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=209534&r1=209533&r2=209534&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Fri May 23 13:15:47 2014
@@ -274,6 +274,16 @@ const FileEntry *FileManager::getFile(St
 
   NamedFileEnt.setValue(&UFE);
   if (UFE.isValid()) { // Already have an entry with this inode, return it.
+
+    // FIXME: this hack ensures that if we look up a file by a virtual path in
+    // the VFS that the getDir() will have the virtual path, even if we found
+    // the file by a 'real' path first. This is required in order to find a
+    // module's structure when its headers/module map are mapped in the VFS.
+    // We should remove this as soon as we can properly support a file having
+    // multiple names.
+    if (DirInfo != UFE.Dir && Data.IsVFSMapped)
+      UFE.Dir = DirInfo;
+
     // If the stat process opened the file, close it to avoid a FD leak.
     if (F)
       delete F;

Modified: cfe/trunk/lib/Basic/FileSystemStatCache.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileSystemStatCache.cpp?rev=209534&r1=209533&r2=209534&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileSystemStatCache.cpp (original)
+++ cfe/trunk/lib/Basic/FileSystemStatCache.cpp Fri May 23 13:15:47 2014
@@ -39,6 +39,7 @@ static void copyStatusToFileData(const v
   Data.IsDirectory = Status.isDirectory();
   Data.IsNamedPipe = Status.getType() == llvm::sys::fs::file_type::fifo_file;
   Data.InPCH = false;
+  Data.IsVFSMapped = Status.IsVFSMapped;
 }
 
 /// FileSystemStatCache::get - Get the 'stat' information for the specified

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=209534&r1=209533&r2=209534&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Fri May 23 13:15:47 2014
@@ -31,13 +31,13 @@ using llvm::sys::fs::UniqueID;
 Status::Status(const file_status &Status)
     : UID(Status.getUniqueID()), MTime(Status.getLastModificationTime()),
       User(Status.getUser()), Group(Status.getGroup()), Size(Status.getSize()),
-      Type(Status.type()), Perms(Status.permissions()) {}
+      Type(Status.type()), Perms(Status.permissions()), IsVFSMapped(false)  {}
 
 Status::Status(StringRef Name, StringRef ExternalName, UniqueID UID,
                sys::TimeValue MTime, uint32_t User, uint32_t Group,
                uint64_t Size, file_type Type, perms Perms)
     : Name(Name), UID(UID), MTime(MTime), User(User), Group(Group), Size(Size),
-      Type(Type), Perms(Perms) {}
+      Type(Type), Perms(Perms), IsVFSMapped(false) {}
 
 bool Status::equivalent(const Status &Other) const {
   return getUniqueID() == Other.getUniqueID();
@@ -801,6 +801,8 @@ ErrorOr<Status> VFSFromYAML::status(cons
     assert(!S || S->getName() == F->getExternalContentsPath());
     if (S && !F->useExternalName(UseExternalNames))
       S->setName(PathStr);
+    if (S)
+      S->IsVFSMapped = true;
     return S;
   } else { // directory
     DirectoryEntry *DE = cast<DirectoryEntry>(*Result);

Modified: cfe/trunk/test/VFS/Inputs/actual_module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/actual_module.map?rev=209534&r1=209533&r2=209534&view=diff
==============================================================================
--- cfe/trunk/test/VFS/Inputs/actual_module.map (original)
+++ cfe/trunk/test/VFS/Inputs/actual_module.map Fri May 23 13:15:47 2014
@@ -2,3 +2,7 @@ module not_real {
   header "not_real.h"
   export *
 }
+module import_some_frame {
+  header "import_some_frame.h"
+  export *
+}

Added: cfe/trunk/test/VFS/Inputs/import_some_frame.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/import_some_frame.h?rev=209534&view=auto
==============================================================================
--- cfe/trunk/test/VFS/Inputs/import_some_frame.h (added)
+++ cfe/trunk/test/VFS/Inputs/import_some_frame.h Fri May 23 13:15:47 2014
@@ -0,0 +1,2 @@
+#import <SomeFramework/public_header.h>
+#import <SomeFramework/public_header2.h>

Modified: cfe/trunk/test/VFS/Inputs/public_header.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/public_header.h?rev=209534&r1=209533&r2=209534&view=diff
==============================================================================
--- cfe/trunk/test/VFS/Inputs/public_header.h (original)
+++ cfe/trunk/test/VFS/Inputs/public_header.h Fri May 23 13:15:47 2014
@@ -1 +1,2 @@
+#import <SomeFramework/public_header2.h>
 void from_framework(void);

Added: cfe/trunk/test/VFS/Inputs/public_header2.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/public_header2.h?rev=209534&view=auto
==============================================================================
--- cfe/trunk/test/VFS/Inputs/public_header2.h (added)
+++ cfe/trunk/test/VFS/Inputs/public_header2.h Fri May 23 13:15:47 2014
@@ -0,0 +1 @@
+// public_header2.h

Added: cfe/trunk/test/VFS/Inputs/some_frame_module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/some_frame_module.map?rev=209534&view=auto
==============================================================================
--- cfe/trunk/test/VFS/Inputs/some_frame_module.map (added)
+++ cfe/trunk/test/VFS/Inputs/some_frame_module.map Fri May 23 13:15:47 2014
@@ -0,0 +1,5 @@
+framework module SomeFramework {
+  umbrella header "public_header.h"
+  export *
+  module * { export * }
+}

Modified: cfe/trunk/test/VFS/Inputs/vfsoverlay.yaml
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/vfsoverlay.yaml?rev=209534&r1=209533&r2=209534&view=diff
==============================================================================
--- cfe/trunk/test/VFS/Inputs/vfsoverlay.yaml (original)
+++ cfe/trunk/test/VFS/Inputs/vfsoverlay.yaml Fri May 23 13:15:47 2014
@@ -6,14 +6,26 @@
         { 'name': 'not_real.h', 'type': 'file',
           'external-contents': 'INPUT_DIR/actual_header.h'
         },
+        { 'name': 'import_some_frame.h', 'type': 'file',
+          'external-contents': 'INPUT_DIR/import_some_frame.h'
+        },
         { 'name': 'module.map', 'type': 'file',
           'external-contents': 'INPUT_DIR/actual_module.map'
         },
         { 'name': 'include_real.h', 'type': 'file',
           'external-contents': 'INPUT_DIR/include_real.h'
         },
-        { 'name': 'SomeFramework.framework/Headers/public_header.h', 'type': 'file',
-          'external-contents': 'INPUT_DIR/public_header.h'
+        { 'name': 'SomeFramework.framework', 'type': 'directory',
+          'contents': [
+            { 'name': 'Headers', 'type': 'directory',
+              'contents': [
+                { 'name': 'public_header.h', 'type': 'file',
+                  'external-contents': 'INPUT_DIR/public_header.h' },
+                { 'name': 'public_header2.h', 'type': 'file',
+                  'external-contents': 'INPUT_DIR/public_header2.h' }
+              ]
+            }
+          ]
         },
         { 'name': 'Foo.framework/Headers/Foo.h', 'type': 'file',
           'external-contents': 'INPUT_DIR/Foo.h'

Added: cfe/trunk/test/VFS/real-path-found-first.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/real-path-found-first.m?rev=209534&view=auto
==============================================================================
--- cfe/trunk/test/VFS/real-path-found-first.m (added)
+++ cfe/trunk/test/VFS/real-path-found-first.m Fri May 23 13:15:47 2014
@@ -0,0 +1,74 @@
+// This test is for cases where we lookup a file by its 'real' path before we
+// use its VFS-mapped path. If we accidentally use the real path in header
+// search, we will not find a module for the headers.  To test that we
+// intentionally rebuild modules, since the precompiled module file refers to
+// the dependency files by real path.
+
+// REQUIRES: shell
+// RUN: rm -rf %t %t-cache %t.pch
+// RUN: mkdir -p %t/SomeFramework.framework/Modules
+// RUN: cp %S/Inputs/some_frame_module.map %t/SomeFramework.framework/Modules/module.modulemap
+// RUN: sed -e "s:INPUT_DIR:%S/Inputs:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsoverlay.yaml > %t.yaml
+
+// Build
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -fsyntax-only %s -verify -Wauto-import \
+// RUN:     -Werror=non-modular-include-in-framework-module
+
+// Rebuild
+// RUN: echo ' ' >> %t/SomeFramework.framework/Modules/module.modulemap
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -fsyntax-only %s -verify -Wauto-import \
+// RUN:     -Werror=non-modular-include-in-framework-module
+
+// Load from PCH
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -emit-pch  %s -o %t.pch \
+// RUN:     -Werror=non-modular-include-in-framework-module \
+// RUN:     -fmodules-ignore-macro=WITH_PREFIX
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -include-pch %t.pch -fsyntax-only  %s \
+// RUN:     -Werror=non-modular-include-in-framework-module -DWITH_PREFIX \
+// RUN:     -fmodules-ignore-macro=WITH_PREFIX
+
+// While indexing
+// RUN: c-index-test -index-file %s -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -fsyntax-only -Wauto-import \
+// RUN:     -Werror=non-modular-include-in-framework-module | FileCheck %s
+// RUN: echo ' ' >> %t/SomeFramework.framework/Modules/module.modulemap
+// RUN: c-index-test -index-file %s -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -fsyntax-only -Wauto-import \
+// RUN:     -Werror=non-modular-include-in-framework-module | FileCheck %s
+// CHECK: warning: treating
+// CHECK-NOT: error
+
+// With a VFS-mapped module map file
+// RUN: mv %t/SomeFramework.framework/Modules/module.modulemap %t/hide_module.map
+// RUN: echo "{ 'version': 0, 'roots': [ { " > %t2.yaml
+// RUN: echo "'name': '%t/SomeFramework.framework/Modules/module.modulemap'," >> %t2.yaml
+// RUN: echo "'type': 'file', 'external-contents': '%t/hide_module.map' } ] }" >> %t2.yaml
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -fsyntax-only %s -verify \
+// RUN:     -Wauto-import -Werror=non-modular-include-in-framework-module
+// RUN: echo ' ' >> %t/hide_module.map
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:     -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -fsyntax-only %s -verify \
+// RUN:     -Wauto-import -Werror=non-modular-include-in-framework-module
+
+// Within a module build
+// RUN: echo '@import import_some_frame;' | \
+// RUN:   %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:      -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -fsyntax-only - \
+// RUN:      -Werror=non-modular-include-in-framework-module -x objective-c -I %t
+// RUN: echo ' ' >> %t/hide_module.map
+// RUN: echo '@import import_some_frame;' | \
+// RUN:   %clang_cc1 -fmodules -fmodules-cache-path=%t-cache -F %t \
+// RUN:      -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -fsyntax-only - \
+// RUN:      -Werror=non-modular-include-in-framework-module -x objective-c -I %t
+
+#ifndef WITH_PREFIX
+#import <SomeFramework/public_header.h> // expected-warning{{treating}}
+#import <SomeFramework/public_header2.h> // expected-warning{{treating}}
+ at import SomeFramework.public_header2;
+#endif





More information about the cfe-commits mailing list