r215455 - Verify all the module map files for a pcm are the same on load

Ben Langmuir blangmuir at apple.com
Tue Aug 12 09:42:34 PDT 2014


Author: benlangmuir
Date: Tue Aug 12 11:42:33 2014
New Revision: 215455

URL: http://llvm.org/viewvc/llvm-project?rev=215455&view=rev
Log:
Verify all the module map files for a pcm are the same on load

We already verified the primary module map file (either the one that
defines the top-level module, or the one that allows inferring it if it
is an inferred framework module). Now we also verify any other module
map files that define submodules, such as when there is a
module.private.modulemap file.

Added:
    cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/
    cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Headers/
    cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Headers/AddRemovePrivate.h
    cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/
    cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.modulemap
    cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap
    cfe/trunk/test/Modules/add-remove-private.m
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
    cfe/trunk/include/clang/Lex/ModuleMap.h
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/lib/Lex/ModuleMap.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=215455&r1=215454&r2=215455&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Tue Aug 12 11:42:33 2014
@@ -56,6 +56,9 @@ def err_imported_module_not_found : Erro
 def err_imported_module_modmap_changed : Error<
     "module '%0' imported by AST file '%1' found in a different module map file"
     " (%2) than when the importing AST file was built (%3)">, DefaultFatal;
+def err_module_different_modmap : Error<
+    "module '%0' %select{uses|does not use}1 additional module map '%2'"
+    "%select{| not}1 used when the module was built">;
 def warn_module_conflict : Warning<
     "module '%0' conflicts with already-imported module '%1': %2">, 
     InGroup<ModuleConflict>;

Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=215455&r1=215454&r2=215455&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Tue Aug 12 11:42:33 2014
@@ -107,6 +107,8 @@ public:
     }
   };
 
+  typedef llvm::SmallPtrSet<const FileEntry *, 1> AdditionalModMapsSet;
+
 private:
   typedef llvm::DenseMap<const FileEntry *, SmallVector<KnownHeader, 1> >
   HeadersMap;
@@ -150,6 +152,8 @@ private:
   /// inference.
   llvm::DenseMap<const Module *, const FileEntry *> InferredModuleAllowedBy;
 
+  llvm::DenseMap<const Module *, AdditionalModMapsSet> AdditionalModMaps;
+
   /// \brief Describes whether we haved parsed a particular file as a module
   /// map.
   llvm::DenseMap<const FileEntry *, bool> ParsedModuleMap;
@@ -349,7 +353,7 @@ public:
   ///
   /// \returns The file entry for the module map file containing the given
   /// module, or NULL if the module definition was inferred.
-  const FileEntry *getContainingModuleMapFile(Module *Module) const;
+  const FileEntry *getContainingModuleMapFile(const Module *Module) const;
 
   /// \brief Get the module map file that (along with the module name) uniquely
   /// identifies this module.
@@ -360,10 +364,25 @@ public:
   /// of inferred modules, returns the module map that allowed the inference
   /// (e.g. contained 'module *'). Otherwise, returns
   /// getContainingModuleMapFile().
-  const FileEntry *getModuleMapFileForUniquing(Module *M) const;
+  const FileEntry *getModuleMapFileForUniquing(const Module *M) const;
 
   void setInferredModuleAllowedBy(Module *M, const FileEntry *ModuleMap);
 
+  /// \brief Get any module map files other than getModuleMapFileForUniquing(M)
+  /// that define submodules of a top-level module \p M. This is cheaper than
+  /// getting the module map file for each submodule individually, since the
+  /// expected number of results is very small.
+  AdditionalModMapsSet *getAdditionalModuleMapFiles(const Module *M) {
+    auto I = AdditionalModMaps.find(M);
+    if (I == AdditionalModMaps.end())
+      return nullptr;
+    return &I->second;
+  }
+
+  void addAdditionalModuleMapFile(const Module *M, const FileEntry *ModuleMap) {
+    AdditionalModMaps[M].insert(ModuleMap);
+  }
+
   /// \brief Resolve all of the unresolved exports in the given module.
   ///
   /// \param Mod The module whose exports should be resolved.

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=215455&r1=215454&r2=215455&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Tue Aug 12 11:42:33 2014
@@ -1117,6 +1117,9 @@ private:
   bool ReadSourceManagerBlock(ModuleFile &F);
   llvm::BitstreamCursor &SLocCursorForID(int ID);
   SourceLocation getImportLocation(ModuleFile *F);
+  ASTReadResult ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F,
+                                       const ModuleFile *ImportedBy,
+                                       unsigned ClientLoadCapabilities);
   ASTReadResult ReadSubmoduleBlock(ModuleFile &F,
                                    unsigned ClientLoadCapabilities);
   static bool ParseLanguageOptions(const RecordData &Record, bool Complain,

Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=215455&r1=215454&r2=215455&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Tue Aug 12 11:42:33 2014
@@ -797,7 +797,7 @@ void ModuleMap::addHeader(Module *Mod, c
 }
 
 const FileEntry *
-ModuleMap::getContainingModuleMapFile(Module *Module) const {
+ModuleMap::getContainingModuleMapFile(const Module *Module) const {
   if (Module->DefinitionLoc.isInvalid())
     return nullptr;
 
@@ -805,7 +805,7 @@ ModuleMap::getContainingModuleMapFile(Mo
            SourceMgr.getFileID(Module->DefinitionLoc));
 }
 
-const FileEntry *ModuleMap::getModuleMapFileForUniquing(Module *M) const {
+const FileEntry *ModuleMap::getModuleMapFileForUniquing(const Module *M) const {
   if (M->IsInferred) {
     assert(InferredModuleAllowedBy.count(M) && "missing inferred module map");
     return InferredModuleAllowedBy.find(M)->second;
@@ -1347,8 +1347,11 @@ void ModuleMapParser::parseModuleDecl()
     // This module map defines a submodule. Go find the module of which it
     // is a submodule.
     ActiveModule = nullptr;
+    const Module *TopLevelModule = nullptr;
     for (unsigned I = 0, N = Id.size() - 1; I != N; ++I) {
       if (Module *Next = Map.lookupModuleQualified(Id[I].first, ActiveModule)) {
+        if (I == 0)
+          TopLevelModule = Next;
         ActiveModule = Next;
         continue;
       }
@@ -1363,7 +1366,14 @@ void ModuleMapParser::parseModuleDecl()
       HadError = true;
       return;
     }
-  } 
+
+    if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
+      assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) &&
+             "submodule defined in same file as 'module *' that allowed its "
+             "top-level module");
+      Map.addAdditionalModuleMapFile(TopLevelModule, ModuleMapFile);
+    }
+  }
   
   StringRef ModuleName = Id.back().first;
   SourceLocation ModuleNameLoc = Id.back().second;

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=215455&r1=215454&r2=215455&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Aug 12 11:42:33 2014
@@ -2468,45 +2468,9 @@ ASTReader::ReadControlBlock(ModuleFile &
       break;
 
     case MODULE_MAP_FILE:
-      F.ModuleMapPath = Blob;
-
-      // Try to resolve ModuleName in the current header search context and
-      // verify that it is found in the same module map file as we saved. If the
-      // top-level AST file is a main file, skip this check because there is no
-      // usable header search context.
-      assert(!F.ModuleName.empty() &&
-             "MODULE_NAME should come before MOUDLE_MAP_FILE");
-      if (F.Kind == MK_Module &&
-          (*ModuleMgr.begin())->Kind != MK_MainFile) {
-        Module *M = PP.getHeaderSearchInfo().lookupModule(F.ModuleName);
-        if (!M) {
-          assert(ImportedBy && "top-level import should be verified");
-          if ((ClientLoadCapabilities & ARR_Missing) == 0)
-            Diag(diag::err_imported_module_not_found)
-              << F.ModuleName << ImportedBy->FileName;
-          return Missing;
-        }
-
-        HeaderSearch &HS = PP.getHeaderSearchInfo();
-        const FileEntry *StoredModMap = FileMgr.getFile(F.ModuleMapPath);
-        const FileEntry *ModMap =
-            HS.getModuleMap().getModuleMapFileForUniquing(M);
-        if (StoredModMap == nullptr || StoredModMap != ModMap) {
-          assert(ModMap && "found module is missing module map file");
-          assert(M->Name == F.ModuleName && "found module with different name");
-          assert(ImportedBy && "top-level import should be verified");
-          if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
-            Diag(diag::err_imported_module_modmap_changed)
-              << F.ModuleName << ImportedBy->FileName
-              << ModMap->getName() << F.ModuleMapPath;
-          return OutOfDate;
-        }
-      }
-
-      if (Listener)
-        Listener->ReadModuleMapFile(F.ModuleMapPath);
-      break;
-
+      if (ASTReadResult Result =
+              ReadModuleMapFileBlock(Record, F, ImportedBy, ClientLoadCapabilities))
+        return Result;
     case INPUT_FILE_OFFSETS:
       F.InputFileOffsets = (const uint32_t *)Blob.data();
       F.InputFilesLoaded.resize(Record[0]);
@@ -3315,6 +3279,89 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
   }
 }
 
+ASTReader::ASTReadResult
+ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F,
+                                  const ModuleFile *ImportedBy,
+                                  unsigned ClientLoadCapabilities) {
+  unsigned Idx = 0;
+  F.ModuleMapPath = ReadString(Record, Idx);
+
+  // Try to resolve ModuleName in the current header search context and
+  // verify that it is found in the same module map file as we saved. If the
+  // top-level AST file is a main file, skip this check because there is no
+  // usable header search context.
+  assert(!F.ModuleName.empty() &&
+         "MODULE_NAME should come before MOUDLE_MAP_FILE");
+  if (F.Kind == MK_Module && (*ModuleMgr.begin())->Kind != MK_MainFile) {
+    Module *M = PP.getHeaderSearchInfo().lookupModule(F.ModuleName);
+    if (!M) {
+      assert(ImportedBy && "top-level import should be verified");
+      if ((ClientLoadCapabilities & ARR_Missing) == 0)
+        Diag(diag::err_imported_module_not_found)
+          << F.ModuleName << ImportedBy->FileName;
+      return Missing;
+    }
+
+    // Check the primary module map file.
+    auto &Map = PP.getHeaderSearchInfo().getModuleMap();
+    const FileEntry *StoredModMap = FileMgr.getFile(F.ModuleMapPath);
+    const FileEntry *ModMap = Map.getModuleMapFileForUniquing(M);
+    if (StoredModMap == nullptr || StoredModMap != ModMap) {
+      assert(ModMap && "found module is missing module map file");
+      assert(M->Name == F.ModuleName && "found module with different name");
+      assert(ImportedBy && "top-level import should be verified");
+      if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
+        Diag(diag::err_imported_module_modmap_changed)
+          << F.ModuleName << ImportedBy->FileName
+          << ModMap->getName() << F.ModuleMapPath;
+      return OutOfDate;
+    }
+
+    llvm::SmallPtrSet<const FileEntry *, 1> AdditionalStoredMaps;
+    for (unsigned I = 0, N = Record[Idx++]; I < N; ++I) {
+      // FIXME: we should use input files rather than storing names.
+      std::string Filename = ReadString(Record, Idx);
+      const FileEntry *F =
+          FileMgr.getFile(Filename, false, false);
+      if (F == nullptr) {
+        if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
+          Error("could not find file '" + Filename +"' referenced by AST file");
+        return OutOfDate;
+      }
+      AdditionalStoredMaps.insert(F);
+    }
+
+    // Check any additional module map files (e.g. module.private.modulemap)
+    // that are not in the pcm.
+    if (auto *AdditionalModuleMaps = Map.getAdditionalModuleMapFiles(M)) {
+      for (const FileEntry *ModMap : *AdditionalModuleMaps) {
+        // Remove files that match
+        // Note: SmallPtrSet::erase is really remove
+        if (!AdditionalStoredMaps.erase(ModMap)) {
+          if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
+            Diag(diag::err_module_different_modmap)
+              << F.ModuleName << /*new*/0 << ModMap->getName();
+          return OutOfDate;
+        }
+      }
+    }
+
+    // Check any additional module map files that are in the pcm, but not
+    // found in header search. Cases that match are already removed.
+    for (const FileEntry *ModMap : AdditionalStoredMaps) {
+      if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
+        Diag(diag::err_module_different_modmap)
+          << F.ModuleName << /*not new*/1 << ModMap->getName();
+      return OutOfDate;
+    }
+  }
+
+  if (Listener)
+    Listener->ReadModuleMapFile(F.ModuleMapPath);
+  return Success;
+}
+
+
 /// \brief Move the given method to the back of the global list of methods.
 static void moveMethodToBackOfGlobalList(Sema &S, ObjCMethodDecl *Method) {
   // Find the entry for this selector in the method pool.
@@ -4136,9 +4183,11 @@ bool ASTReader::readASTFileControlBlock(
     case MODULE_NAME:
       Listener.ReadModuleName(Blob);
       break;
-    case MODULE_MAP_FILE:
-      Listener.ReadModuleMapFile(Blob);
+    case MODULE_MAP_FILE: {
+      unsigned Idx = 0;
+      Listener.ReadModuleMapFile(ReadString(Record, Idx));
       break;
+    }
     case LANGUAGE_OPTIONS:
       if (ParseLanguageOptions(Record, false, Listener))
         return true;

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=215455&r1=215454&r2=215455&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Aug 12 11:42:33 2014
@@ -1134,18 +1134,28 @@ void ASTWriter::WriteControlBlock(Prepro
 
   // Module map file
   if (WritingModule) {
-    BitCodeAbbrev *Abbrev = new BitCodeAbbrev();
-    Abbrev->Add(BitCodeAbbrevOp(MODULE_MAP_FILE));
-    Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Filename
-    unsigned AbbrevCode = Stream.EmitAbbrev(Abbrev);
-
-    const FileEntry *ModMapFile = PP.getHeaderSearchInfo().getModuleMap().
-        getModuleMapFileForUniquing(WritingModule);
-    SmallString<128> ModuleMap(ModMapFile->getName());
-    llvm::sys::fs::make_absolute(ModuleMap);
-    RecordData Record;
-    Record.push_back(MODULE_MAP_FILE);
-    Stream.EmitRecordWithBlob(AbbrevCode, Record, ModuleMap.str());
+    Record.clear();
+    auto addModMap = [&](const FileEntry *F) {
+      SmallString<128> ModuleMap(F->getName());
+      llvm::sys::fs::make_absolute(ModuleMap);
+      AddString(ModuleMap.str(), Record);
+    };
+
+    auto &Map = PP.getHeaderSearchInfo().getModuleMap();
+
+    // Primary module map file.
+    addModMap(Map.getModuleMapFileForUniquing(WritingModule));
+
+    // Additional module map files.
+    if (auto *AdditionalModMaps = Map.getAdditionalModuleMapFiles(WritingModule)) {
+      Record.push_back(AdditionalModMaps->size());
+      for (const FileEntry *F : *AdditionalModMaps)
+        addModMap(F);
+    } else {
+      Record.push_back(0);
+    }
+
+    Stream.EmitRecord(MODULE_MAP_FILE, Record);
   }
 
   // Imports

Added: cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Headers/AddRemovePrivate.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Headers/AddRemovePrivate.h?rev=215455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Headers/AddRemovePrivate.h (added)
+++ cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Headers/AddRemovePrivate.h Tue Aug 12 11:42:33 2014
@@ -0,0 +1 @@
+// AddRemovePrivate.h

Added: cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.modulemap?rev=215455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.modulemap Tue Aug 12 11:42:33 2014
@@ -0,0 +1 @@
+framework module AddRemovePrivate { umbrella header "AddRemovePrivate.h" }

Added: cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap?rev=215455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap Tue Aug 12 11:42:33 2014
@@ -0,0 +1 @@
+explicit module AddRemovePrivate.Private { }

Added: cfe/trunk/test/Modules/add-remove-private.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/add-remove-private.m?rev=215455&view=auto
==============================================================================
--- cfe/trunk/test/Modules/add-remove-private.m (added)
+++ cfe/trunk/test/Modules/add-remove-private.m Tue Aug 12 11:42:33 2014
@@ -0,0 +1,29 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+// RUN: cp -r %S/Inputs/AddRemovePrivate.framework %t/AddRemovePrivate.framework
+
+// Build with module.private.modulemap
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify -DP
+// RUN: cp %t.mcp/AddRemovePrivate.pcm %t/with.pcm
+
+// Build without module.private.modulemap
+// RUN: rm %t/AddRemovePrivate.framework/Modules/module.private.modulemap
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify
+// RUN: not diff %t.mcp/AddRemovePrivate.pcm %t/with.pcm
+// RUN: cp %t.mcp/AddRemovePrivate.pcm %t/without.pcm
+// RUN: not %clang_cc1 -fmodules -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -DP 2>&1 | FileCheck %s
+// CHECK: no submodule named 'Private'
+
+// Build with module.private.modulemap (again)
+// RUN: cp %S/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap %t/AddRemovePrivate.framework/Modules/module.private.modulemap
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify -DP
+// RUN: not diff %t.mcp/AddRemovePrivate.pcm %t/without.pcm
+
+// expected-no-diagnostics
+
+ at import AddRemovePrivate;
+#ifdef P
+ at import AddRemovePrivate.Private;
+#endif





More information about the cfe-commits mailing list