dexonsmith created this revision.
dexonsmith added reviewers: aprantl, bruno, Bigcheese.
Herald added a subscriber: ributzka.
dexonsmith added parent revisions: D70055: clang/Modules: Clean up modules on 
error in ReadAST, D70056: clang/Modules: Split loop in ReadAST between failable 
and not, D70057: clang/Modules: Add missing diagnostics for malformed AST 
files, D70058: clang/Modules: Delay err_module_file_conflict if a diagnostic is 
in flight.

If ReadASTBlock does not find the main module, 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 finds in
CompilerInstance::loadModule that the main module mysteriously does not
have Module::IsFromModuleFile set.  That will emit a confusing
warn_missing_submodule, which was never intended for a top-level 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.


https://reviews.llvm.org/D70063

Files:
  clang/include/clang/Serialization/Module.h
  clang/lib/Serialization/ASTReader.cpp


Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4219,6 +4219,13 @@
     if (ASTReadResult Result = ReadASTBlock(F, ClientLoadCapabilities))
       return removeModulesAndReturn(Result);
 
+    if (F.isModule() && !F.DidReadMainModule) {
+      // The AST block is malformed if it does not contain a definition for the
+      // main module.
+      Error("missing main SUBMODULE_DEFINITION in AST file");
+      return removeModulesAndReturn(Failure);
+    }
+
     // Read the extension blocks.
     while (!SkipCursorToBlock(F.Stream, EXTENSION_BLOCK_ID)) {
       if (ASTReadResult Result = ReadExtensionBlock(F))
@@ -5494,6 +5501,7 @@
           }
         }
 
+        F.DidReadMainModule = true;
         CurrentModule->setASTFile(F.File);
         CurrentModule->PresumedModuleMapFile = F.ModuleMapPath;
       }
Index: clang/include/clang/Serialization/Module.h
===================================================================
--- clang/include/clang/Serialization/Module.h
+++ clang/include/clang/Serialization/Module.h
@@ -159,6 +159,9 @@
   /// Whether the PCH has a corresponding object file.
   bool PCHHasObjectFile = false;
 
+  /// Whether the main module has been read from the AST file.
+  bool DidReadMainModule = false;
+
   /// The file entry for the module file.
   const FileEntry *File = nullptr;
 


Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4219,6 +4219,13 @@
     if (ASTReadResult Result = ReadASTBlock(F, ClientLoadCapabilities))
       return removeModulesAndReturn(Result);
 
+    if (F.isModule() && !F.DidReadMainModule) {
+      // The AST block is malformed if it does not contain a definition for the
+      // main module.
+      Error("missing main SUBMODULE_DEFINITION in AST file");
+      return removeModulesAndReturn(Failure);
+    }
+
     // Read the extension blocks.
     while (!SkipCursorToBlock(F.Stream, EXTENSION_BLOCK_ID)) {
       if (ASTReadResult Result = ReadExtensionBlock(F))
@@ -5494,6 +5501,7 @@
           }
         }
 
+        F.DidReadMainModule = true;
         CurrentModule->setASTFile(F.File);
         CurrentModule->PresumedModuleMapFile = F.ModuleMapPath;
       }
Index: clang/include/clang/Serialization/Module.h
===================================================================
--- clang/include/clang/Serialization/Module.h
+++ clang/include/clang/Serialization/Module.h
@@ -159,6 +159,9 @@
   /// Whether the PCH has a corresponding object file.
   bool PCHHasObjectFile = false;
 
+  /// Whether the main module has been read from the AST file.
+  bool DidReadMainModule = false;
+
   /// The file entry for the module file.
   const FileEntry *File = nullptr;
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D70063: cl... Duncan P. N. Exon Smith via Phabricator via cfe-commits

Reply via email to