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