From 4625c18b5f4841c1f853d33cce0fa5b6abf74399 Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes Date: Thu, 29 Aug 2019 23:14:08 +0000 Subject: [Modules] Make ReadModuleMapFileBlock errors reliable This prevents a crash when an error should be emitted instead. During implicit module builds, there are cases where ReadASTCore is called with ImportedBy set to nullptr, which breaks expectations in ReadModuleMapFileBlock, leading to crashes. Fix this by improving ReadModuleMapFileBlock to handle ImportedBy correctly. This only happens non deterministically in the wild, when the underlying file system changes while concurrent compiler invocations use implicit modules, forcing rebuilds which see an inconsistent filesystem state. That said, there's no much to do w.r.t. writing tests here. rdar://problem/48828801 llvm-svn: 370422 --- clang/lib/Serialization/ASTReader.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'clang/lib') diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 6a18af8a082..aaa59fcf506 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3823,7 +3823,6 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, const FileEntry *ModMap = M ? Map.getModuleMapFileForUniquing(M) : nullptr; // Don't emit module relocation error if we have -fno-validate-pch if (!PP.getPreprocessorOpts().DisablePCHValidation && !ModMap) { - assert(ImportedBy && "top-level import should be verified"); if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) { if (auto *ASTFE = M ? M->getASTFile() : nullptr) { // This module was defined by an imported (explicit) module. @@ -3832,12 +3831,13 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, } else { // This module was built with a different module map. Diag(diag::err_imported_module_not_found) - << F.ModuleName << F.FileName << ImportedBy->FileName - << F.ModuleMapPath; + << F.ModuleName << F.FileName + << (ImportedBy ? ImportedBy->FileName : "") << F.ModuleMapPath + << !ImportedBy; // In case it was imported by a PCH, there's a chance the user is // just missing to include the search path to the directory containing // the modulemap. - if (ImportedBy->Kind == MK_PCH) + if (ImportedBy && ImportedBy->Kind == MK_PCH) Diag(diag::note_imported_by_pch_module_not_found) << llvm::sys::path::parent_path(F.ModuleMapPath); } @@ -3851,11 +3851,13 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, auto StoredModMap = FileMgr.getFile(F.ModuleMapPath); if (!StoredModMap || *StoredModMap != ModMap) { assert(ModMap && "found module is missing module map file"); - assert(ImportedBy && "top-level import should be verified"); + assert((ImportedBy || F.Kind == MK_ImplicitModule) && + "top-level import should be verified"); + bool NotImported = F.Kind == MK_ImplicitModule && !ImportedBy; if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) Diag(diag::err_imported_module_modmap_changed) - << F.ModuleName << ImportedBy->FileName - << ModMap->getName() << F.ModuleMapPath; + << F.ModuleName << (NotImported ? F.FileName : ImportedBy->FileName) + << ModMap->getName() << F.ModuleMapPath << NotImported; return OutOfDate; } -- cgit v1.2.3