clang/Modules: Error if ReadASTBlock does not find the main module
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Sun, 10 Nov 2019 21:14:52 +0000 (13:14 -0800)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Tue, 12 Nov 2019 16:40:53 +0000 (08:40 -0800)
If ReadASTBlock does not find its top-level submodule, there's something
wrong the with the PCM.  Error in that case, to avoid hitting problems
further from the source.

Note that the Swift compiler sometimes hits a case in
CompilerInstance::loadModule where the top-level submodule mysteriously
does not have Module::IsFromModuleFile set.  That will emit a confusing
warn_missing_submodule, which was never intended for the main module.
The recent audit of error-handling in ReadAST may have rooted out the
real problem.  If not, this commit will help to clarify the real
problem, and replace a confusing warning with an error pointing at the
malformed PCM file.

We're specifically sniffing out whether the top-level submodule was
found/processed, in case there is a malformed module file that is
missing it.  If there is an error encountered during ReadSubmoduleBlock
the return status should already propagate through.  It would be nice to
detect other missing submodules around here to catch other instances of
warn_missing_submodule closer to the source, but that's left as a future
exercise.

https://reviews.llvm.org/D70063

clang/include/clang/Basic/DiagnosticSerializationKinds.td
clang/include/clang/Serialization/Module.h
clang/lib/Serialization/ASTReader.cpp

index 757dbbe..f499996 100644 (file)
@@ -74,6 +74,8 @@ def note_module_file_imported_by : Note<
   "imported by %select{|module '%2' in }1'%0'">;
 def err_module_file_not_module : Error<
   "AST file '%0' was not built as a module">, DefaultFatal;
+def err_module_file_missing_top_level_submodule : Error<
+  "module file '%0' is missing its top-level submodule">, DefaultFatal;
 
 def remark_module_import : Remark<
   "importing module '%0'%select{| into '%3'}2 from '%1'">,
index 1979c53..b5afdf9 100644 (file)
@@ -159,6 +159,9 @@ public:
   /// Whether the PCH has a corresponding object file.
   bool PCHHasObjectFile = false;
 
+  /// Whether the top-level module has been read from the AST file.
+  bool DidReadTopLevelSubmodule = false;
+
   /// The file entry for the module file.
   const FileEntry *File = nullptr;
 
index f7e3805..9111b60 100644 (file)
@@ -4217,6 +4217,12 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName,
     if (ASTReadResult Result = ReadASTBlock(F, ClientLoadCapabilities))
       return removeModulesAndReturn(Result);
 
+    // The AST block should always have a definition for the main module.
+    if (F.isModule() && !F.DidReadTopLevelSubmodule) {
+      Error(diag::err_module_file_missing_top_level_submodule, F.FileName);
+      return removeModulesAndReturn(Failure);
+    }
+
     // Read the extension blocks.
     while (!SkipCursorToBlock(F.Stream, EXTENSION_BLOCK_ID)) {
       if (ASTReadResult Result = ReadExtensionBlock(F))
@@ -5489,6 +5495,7 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
           }
         }
 
+        F.DidReadTopLevelSubmodule = true;
         CurrentModule->setASTFile(F.File);
         CurrentModule->PresumedModuleMapFile = F.ModuleMapPath;
       }