Author: bruno
Date: Thu Aug 29 16:14:08 2019
New Revision: 370422
URL: http://llvm.org/viewvc/llvm-project?rev=370422&view=rev
Log:
[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
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/lib/Serialization/ASTReader.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=370422&r1=370421&r2=370422&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Thu Aug 29
16:14:08 2019
@@ -77,13 +77,13 @@ def remark_module_import : Remark<
InGroup<ModuleImport>;
def err_imported_module_not_found : Error<
- "module '%0' in AST file '%1' (imported by AST file '%2') "
+ "module '%0' in AST file '%1' %select{(imported by AST file '%2') |}4"
"is not defined in any loaded module map file; "
"maybe you need to load '%3'?">, DefaultFatal;
def note_imported_by_pch_module_not_found : Note<
"consider adding '%0' to the header search path">;
def err_imported_module_modmap_changed : Error<
- "module '%0' imported by AST file '%1' found in a different module map
file"
+ "module '%0' %select{in|imported by}4 AST file '%1' found in a different
module map file"
" (%2) than when the importing AST file was built (%3)">, DefaultFatal;
def err_imported_module_relocated : Error<
"module '%0' was built in directory '%1' but now resides in "
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=370422&r1=370421&r2=370422&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Aug 29 16:14:08 2019
@@ -3823,7 +3823,6 @@ ASTReader::ReadModuleMapFileBlock(Record
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(Record
} 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(Record
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
