[PCH/Modules] Check that the specific module cache path the PCH was built with, is...

the one in the current compiler invocation. If they differ reject the PCH.

This protects against the badness occurring from getting modules loaded from different module caches (see crashes).

rdar://19889860

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@229909 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticSerializationKinds.td b/include/clang/Basic/DiagnosticSerializationKinds.td
index fe5fd00d737657cdbc6d4a0cfc9e64bf10f5e9cc..f28f9983d0e0141a54e13195dbbff5ecd141bbd9 100644 (file)
--- a/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -41,6 +41,8 @@ def err_pch_langopt_value_mismatch : Error<
   "%0 differs in PCH file vs. current file">;
 def err_pch_diagopt_mismatch : Error<"%0 is currently enabled, but was not in "
   "the PCH file">;
+def err_pch_modulecache_mismatch : Error<"PCH was compiled with module cache "
+  "path '%0', but the path is currently '%1'">;
   
 def err_pch_version_too_old : Error<
     "PCH file uses an older PCH format that is no longer supported">;

diff --git a/include/clang/Frontend/CompilerInstance.h b/include/clang/Frontend/CompilerInstance.h
index f40e115c23de33a92045ece06bffe7fc9cfc7583..cee87b0475fb15f3e69adddcdfc150a0afe4cfc2 100644 (file)
--- a/include/clang/Frontend/CompilerInstance.h
+++ b/include/clang/Frontend/CompilerInstance.h
@@ -575,6 +575,8 @@ public:
   /// and replace any existing one with it.
   void createPreprocessor(TranslationUnitKind TUKind);
 
+  std::string getSpecificModuleCachePath();
+
   /// Create the AST context.
   void createASTContext();
 

diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index d7ecf26606946e9a366a4093182d7a817f23b498..27af9995aa9b26161a0e00562e4511c6ee5eb065 100644 (file)
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -153,6 +153,7 @@ public:
   /// \returns true to indicate the header search options are invalid, or false
   /// otherwise.
   virtual bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
+                                       StringRef SpecificModuleCachePath,
                                        bool Complain) {
     return false;
   }
@@ -230,6 +231,7 @@ public:
                              bool Complain) override;
 
   bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
+                               StringRef SpecificModuleCachePath,
                                bool Complain) override;
   bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
                                bool Complain,
@@ -261,6 +263,9 @@ public:
                              bool Complain) override;
   bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
                                std::string &SuggestedPredefines) override;
+  bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
+                               StringRef SpecificModuleCachePath,
+                               bool Complain) override;
   void ReadCounter(const serialization::ModuleFile &M, unsigned Value) override;
 
 private:
@@ -1496,7 +1501,8 @@ public:
                                   FileManager &FileMgr,
                                   const LangOptions &LangOpts,
                                   const TargetOptions &TargetOpts,
-                                  const PreprocessorOptions &PPOpts);
+                                  const PreprocessorOptions &PPOpts,
+                                  std::string ExistingModuleCachePath);
 
   /// \brief Returns the suggested contents of the predefines buffer,
   /// which contains a (typically-empty) subset of the predefines

diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp
index 187e2b78b8f7032dd1ad7c9944118294865856a8..5d554b717b3346e9ad22162d925ef2e69b132c51 100644 (file)
--- a/lib/Frontend/CompilerInstance.cpp
+++ b/lib/Frontend/CompilerInstance.cpp
@@ -329,14 +329,7 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
 
   PP->setPreprocessedOutput(getPreprocessorOutputOpts().ShowCPP);
 
-  // Set up the module path, including the hash for the
-  // module-creation options.
-  SmallString<256> SpecificModuleCache(
-                           getHeaderSearchOpts().ModuleCachePath);
-  if (!getHeaderSearchOpts().DisableModuleHash)
-    llvm::sys::path::append(SpecificModuleCache,
-                            getInvocation().getModuleHash());
-  PP->getHeaderSearchInfo().setModuleCachePath(SpecificModuleCache);
+  PP->getHeaderSearchInfo().setModuleCachePath(getSpecificModuleCachePath());
 
   // Handle generating dependencies, if requested.
   const DependencyOutputOptions &DepOpts = getDependencyOutputOpts();
@@ -373,6 +366,17 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
   }
 }
 
+std::string CompilerInstance::getSpecificModuleCachePath() {
+  // Set up the module path, including the hash for the
+  // module-creation options.
+  SmallString<256> SpecificModuleCache(
+                           getHeaderSearchOpts().ModuleCachePath);
+  if (!getHeaderSearchOpts().DisableModuleHash)
+    llvm::sys::path::append(SpecificModuleCache,
+                            getInvocation().getModuleHash());
+  return SpecificModuleCache.str();
+}
+
 // ASTContext
 
 void CompilerInstance::createASTContext() {

diff --git a/lib/Frontend/FrontendAction.cpp b/lib/Frontend/FrontendAction.cpp
index 0bb5d87407b56074f459bf51dd465f1cb9f31796..75670d9a202f65bfb7c95e5e56e5d52d0d1c18df 100644 (file)
--- a/lib/Frontend/FrontendAction.cpp
+++ b/lib/Frontend/FrontendAction.cpp
@@ -262,6 +262,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
     FileManager &FileMgr = CI.getFileManager();
     PreprocessorOptions &PPOpts = CI.getPreprocessorOpts();
     StringRef PCHInclude = PPOpts.ImplicitPCHInclude;
+    std::string SpecificModuleCachePath = CI.getSpecificModuleCachePath();
     if (const DirectoryEntry *PCHDir = FileMgr.getDirectory(PCHInclude)) {
       std::error_code EC;
       SmallString<128> DirNative;
@@ -273,7 +274,8 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
         if (ASTReader::isAcceptableASTFile(Dir->path(), FileMgr,
                                            CI.getLangOpts(),
                                            CI.getTargetOpts(),
-                                           CI.getPreprocessorOpts())) {
+                                           CI.getPreprocessorOpts(),
+                                           SpecificModuleCachePath)) {
           PPOpts.ImplicitPCHInclude = Dir->path();
           Found = true;
           break;

diff --git a/lib/Frontend/FrontendActions.cpp b/lib/Frontend/FrontendActions.cpp
index e7ecb2952ac55334a2a9a0864b642e8b86726142..c8673e50a058661b76d4c0db12ee8be7eb0de13b 100644 (file)
--- a/lib/Frontend/FrontendActions.cpp
+++ b/lib/Frontend/FrontendActions.cpp
@@ -501,9 +501,11 @@ namespace {
     }
 
     bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
+                                 StringRef SpecificModuleCachePath,
                                  bool Complain) override {
       Out.indent(2) << "Header search options:\n";
       Out.indent(4) << "System root [-isysroot=]: '" << HSOpts.Sysroot << "'\n";
+      Out.indent(4) << "Module Cache: '" << SpecificModuleCachePath << "'\n";
       DUMP_BOOLEAN(HSOpts.UseBuiltinIncludes,
                    "Use builtin include directories [-nobuiltininc]");
       DUMP_BOOLEAN(HSOpts.UseStandardSystemIncludes,

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 3125b8e43e9450152ab21a0b7a7588f4ab65d0b2..7774b7b1c6e74024d9928bd9ecb272bfcb02e0f6 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -108,9 +108,12 @@ ChainedASTReaderListener::ReadFileSystemOptions(const FileSystemOptions &FSOpts,
 }
 
 bool ChainedASTReaderListener::ReadHeaderSearchOptions(
-    const HeaderSearchOptions &HSOpts, bool Complain) {
-  return First->ReadHeaderSearchOptions(HSOpts, Complain) ||
-         Second->ReadHeaderSearchOptions(HSOpts, Complain);
+    const HeaderSearchOptions &HSOpts, StringRef SpecificModuleCachePath,
+    bool Complain) {
+  return First->ReadHeaderSearchOptions(HSOpts, SpecificModuleCachePath,
+                                        Complain) ||
+         Second->ReadHeaderSearchOptions(HSOpts, SpecificModuleCachePath,
+                                         Complain);
 }
 bool ChainedASTReaderListener::ReadPreprocessorOptions(
     const PreprocessorOptions &PPOpts, bool Complain,
@@ -464,7 +467,7 @@ collectMacroDefinitions(const PreprocessorOptions &PPOpts,
     Macros[MacroName] = std::make_pair(MacroBody, false);
   }
 }
-         
+
 /// \brief Check the preprocessor options deserialized from the control block
 /// against the preprocessor options in an existing preprocessor.
 ///
@@ -591,6 +594,36 @@ bool PCHValidator::ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
                                   PP.getLangOpts());
 }
 
+/// Check the header search options deserialized from the control block
+/// against the header search options in an existing preprocessor.
+///
+/// \param Diags If non-null, produce diagnostics for any mismatches incurred.
+static bool checkHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
+                                     StringRef SpecificModuleCachePath,
+                                     StringRef ExistingModuleCachePath,
+                                     DiagnosticsEngine *Diags,
+                                     const LangOptions &LangOpts) {
+  if (LangOpts.Modules) {
+    if (SpecificModuleCachePath != ExistingModuleCachePath) {
+      if (Diags)
+        Diags->Report(diag::err_pch_modulecache_mismatch)
+          << SpecificModuleCachePath << ExistingModuleCachePath;
+      return true;
+    }
+  }
+
+  return false;
+}
+
+bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
+                                           StringRef SpecificModuleCachePath,
+                                           bool Complain) {
+  return checkHeaderSearchOptions(HSOpts, SpecificModuleCachePath,
+                                  PP.getHeaderSearchInfo().getModuleCachePath(),
+                                  Complain ? &Reader.Diags : nullptr,
+                                  PP.getLangOpts());
+}
+
 void PCHValidator::ReadCounter(const ModuleFile &M, unsigned Value) {
   PP.setCounterValue(Value);
 }
@@ -4191,16 +4224,19 @@ namespace {
     const LangOptions &ExistingLangOpts;
     const TargetOptions &ExistingTargetOpts;
     const PreprocessorOptions &ExistingPPOpts;
+    std::string ExistingModuleCachePath;
     FileManager &FileMgr;
-    
+
   public:
     SimplePCHValidator(const LangOptions &ExistingLangOpts,
                        const TargetOptions &ExistingTargetOpts,
                        const PreprocessorOptions &ExistingPPOpts,
+                       StringRef ExistingModuleCachePath,
                        FileManager &FileMgr)
       : ExistingLangOpts(ExistingLangOpts),
         ExistingTargetOpts(ExistingTargetOpts),
         ExistingPPOpts(ExistingPPOpts),
+        ExistingModuleCachePath(ExistingModuleCachePath),
         FileMgr(FileMgr)
     {
     }
@@ -4214,6 +4250,13 @@ namespace {
                            bool Complain) override {
       return checkTargetOptions(ExistingTargetOpts, TargetOpts, nullptr);
     }
+    bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
+                                 StringRef SpecificModuleCachePath,
+                                 bool Complain) override {
+      return checkHeaderSearchOptions(HSOpts, SpecificModuleCachePath,
+                                      ExistingModuleCachePath,
+                                      nullptr, ExistingLangOpts);
+    }
     bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
                                  bool Complain,
                                  std::string &SuggestedPredefines) override {
@@ -4408,8 +4451,10 @@ bool ASTReader::isAcceptableASTFile(StringRef Filename,
                                     FileManager &FileMgr,
                                     const LangOptions &LangOpts,
                                     const TargetOptions &TargetOpts,
-                                    const PreprocessorOptions &PPOpts) {
-  SimplePCHValidator validator(LangOpts, TargetOpts, PPOpts, FileMgr);
+                                    const PreprocessorOptions &PPOpts,
+                                    std::string ExistingModuleCachePath) {
+  SimplePCHValidator validator(LangOpts, TargetOpts, PPOpts,
+                               ExistingModuleCachePath, FileMgr);
   return !readASTFileControlBlock(Filename, FileMgr, validator);
 }
 
@@ -4780,8 +4825,10 @@ bool ASTReader::ParseHeaderSearchOptions(const RecordData &Record,
   HSOpts.UseStandardSystemIncludes = Record[Idx++];
   HSOpts.UseStandardCXXIncludes = Record[Idx++];
   HSOpts.UseLibcxx = Record[Idx++];
+  std::string SpecificModuleCachePath = ReadString(Record, Idx);
 
-  return Listener.ReadHeaderSearchOptions(HSOpts, Complain);
+  return Listener.ReadHeaderSearchOptions(HSOpts, SpecificModuleCachePath,
+                                          Complain);
 }
 
 bool ASTReader::ParsePreprocessorOptions(const RecordData &Record,

diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index ee1b631f1ec4114dbb4cf71d99e1c99937d9e227..c0ce5f8e5ca7715aa0a06ac3a3331251ff617b06 100644 (file)
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -1344,6 +1344,8 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
   Record.push_back(HSOpts.UseStandardSystemIncludes);
   Record.push_back(HSOpts.UseStandardCXXIncludes);
   Record.push_back(HSOpts.UseLibcxx);
+  // Write out the specific module cache path that contains the module files.
+  AddString(PP.getHeaderSearchInfo().getModuleCachePath(), Record);
   Stream.EmitRecord(HEADER_SEARCH_OPTIONS, Record);
 
   // Preprocessor options.

diff --git a/test/Modules/ignored_macros.m b/test/Modules/ignored_macros.m
index e8ee50ace315d0dae4091486af38273f12f7b593..669db4ddc2da1aaaa233585c945ec457d5a193dc 100644 (file)
--- a/test/Modules/ignored_macros.m
+++ b/test/Modules/ignored_macros.m
@@ -10,7 +10,7 @@
 // RUN: %clang_cc1 -fmodules-cache-path=%t.modules -fmodules -I %S/Inputs -emit-pch -o %t.pch -x objective-c-header %s -verify
 // RUN: not %clang_cc1 -fmodules-cache-path=%t.modules -DIGNORED=1 -fmodules -I %S/Inputs -include-pch %t.pch %s > %t.err 2>&1
 // RUN: FileCheck -check-prefix=CHECK-CONFLICT %s < %t.err
-// CHECK-CONFLICT: module 'ignored_macros' found in both
+// CHECK-CONFLICT: PCH was compiled with module cache path
 
 // Third trial: pass -DIGNORED=1 only to the second invocation, but
 // make it ignored. There should be no failure, IGNORED is defined in

diff --git a/test/Modules/implementation-of-module.m b/test/Modules/implementation-of-module.m
index b3984042015602035de743286609b3a312ebdc30..818cce8c36debdcf55c9b386ef795c7c757bf1c1 100644 (file)
--- a/test/Modules/implementation-of-module.m
+++ b/test/Modules/implementation-of-module.m
@@ -16,7 +16,7 @@
 // RUN: %clang_cc1 -x objective-c-header -fmodules -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
 // RUN:     -fmodule-implementation-of category_right -emit-pch -o %t.pch
 // RUN: %clang_cc1 -x objective-c-header -fmodules -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
-// RUN:     -DWITH_PREFIX -include-pch %t.pch -fmodule-implementation-of category_right
+// RUN:     -DWITH_PREFIX -fmodules-ignore-macro=WITH_PREFIX -include-pch %t.pch -fmodule-implementation-of category_right
 
 #ifndef WITH_PREFIX
 

diff --git a/test/PCH/Inputs/modules/Foo.h b/test/PCH/Inputs/modules/Foo.h
new file mode 100644 (file)
index 0000000..d661dbc
--- /dev/null
+++ b/test/PCH/Inputs/modules/Foo.h
@@ -0,0 +1 @@
+void make_foo(void);

diff --git a/test/PCH/Inputs/modules/module.modulemap b/test/PCH/Inputs/modules/module.modulemap
new file mode 100644 (file)
index 0000000..5590b43
--- /dev/null
+++ b/test/PCH/Inputs/modules/module.modulemap
@@ -0,0 +1,3 @@
+module Foo {
+  header "Foo.h"
+}

diff --git a/test/PCH/cxx-templates.cpp b/test/PCH/cxx-templates.cpp
index 52ea8752b36034ae3c23f7d3ae95074171c2fbf0..d50eee0623c5c4334f84b73c49fe1344d84539de 100644 (file)
--- a/test/PCH/cxx-templates.cpp
+++ b/test/PCH/cxx-templates.cpp
@@ -10,7 +10,7 @@
 // Test with modules.
 // RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fmodules -x c++-header -emit-pch -o %t %S/cxx-templates.h
 // RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fmodules -include-pch %t -verify %s -ast-dump  -o -
-// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fmodules -include-pch %t %s -emit-llvm -o - -error-on-deserialized-decl doNotDeserialize -DNO_ERRORS | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fmodules -include-pch %t %s -emit-llvm -o - -error-on-deserialized-decl doNotDeserialize -DNO_ERRORS -fmodules-ignore-macro=NO_ERRORS | FileCheck %s
 
 // Test with pch and delayed template parsing.
 // RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -fcxx-exceptions -fdelayed-template-parsing -fexceptions -x c++-header -emit-pch -o %t %S/cxx-templates.h

diff --git a/test/PCH/module-hash-difference.m b/test/PCH/module-hash-difference.m
new file mode 100644 (file)
index 0000000..073540e
--- /dev/null
+++ b/test/PCH/module-hash-difference.m
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t.mcp
+// RUN: rm -rf %t.err
+// RUN: %clang_cc1 -emit-pch -o %t.pch %s -I %S/Inputs/modules -fmodules -fmodules-cache-path=%t.mcp
+// RUN: not %clang_cc1 -fsyntax-only -include-pch %t.pch %s -I %S/Inputs/modules -fmodules -fmodules-cache-path=%t.mcp -fdisable-module-hash 2> %t.err
+// RUN: FileCheck -input-file=%t.err %s
+
+// CHECK: error: PCH was compiled with module cache path {{.*}}, but the path is currently {{.*}}
+@import Foo;