[clang] 83dcb34 - clang/Modules: Error if ReadASTBlock does not find the main module

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2019-11-12T08:40:53-08:00
New Revision: 83dcb34b6bf4c175040b18d3e8c3c468418009fc

URL: https://github.com/llvm/llvm-project/commit/83dcb34b6bf4c175040b18d3e8c3c468418009fc
DIFF: https://github.com/llvm/llvm-project/commit/83dcb34b6bf4c175040b18d3e8c3c468418009fc.diff

LOG: clang/Modules: Error if ReadASTBlock does not find the main module

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 757dbbeee3cc..f499996470c3 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -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'">,

diff  --git a/clang/include/clang/Serialization/Module.h b/clang/include/clang/Serialization/Module.h
index 1979c53a7133..b5afdf90c6af 100644
--- a/clang/include/clang/Serialization/Module.h
+++ b/clang/include/clang/Serialization/Module.h
@@ -159,6 +159,9 @@ class ModuleFile {
   /// 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;
 

diff  --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index f7e3805fd04c..9111b60a7179 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -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;
       }