https://github.com/naveen-seth updated
https://github.com/llvm/llvm-project/pull/133462
>From a23ba4d51332180ff5d1b5bc9de2d0c6c04cbf66 Mon Sep 17 00:00:00 2001
From: naveen-seth <naveen.ha...@outlook.com>
Date: Fri, 28 Mar 2025 06:59:06 +0100
Subject: [PATCH] [clang][modules] Guard against bad -fmodule-file mappings
(#132059)
Fix #132059.
Providing incorrect mappings via `-fmodule-file=<name>=<path/to/bmi>`
can crash the compiler when loading a module that imports an
incorrectly mapped module.
The crash occurs during AST body deserialization, when the compiler
attempts to resolve remappings using the `ModuleFile` from the
incorrectly mapped module's BMI file.
The cause is an invalid access into an incorrectly loaded
`ModuleFile`.
This commit fixes the issue by verifying that the mapped BMI files
correspond to the mapped-from modules as soon as the module name is
read from the BMI's control block, and it errors out if there is a
mismatch.
---
.../Basic/DiagnosticSerializationKinds.td | 2 +
clang/include/clang/Serialization/ASTReader.h | 17 +++++
clang/lib/Frontend/ASTUnit.cpp | 1 +
clang/lib/Frontend/ChainedIncludesSource.cpp | 1 +
clang/lib/Frontend/CompilerInstance.cpp | 15 ++--
clang/lib/Serialization/ASTReader.cpp | 73 +++++++++++++------
.../fmodule-file-bad-transitive-mapping.cpp | 46 ++++++++++++
7 files changed, 124 insertions(+), 31 deletions(-)
create mode 100644 clang/test/Modules/fmodule-file-bad-transitive-mapping.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 3914d3930bec7..16cc946e3f3d9 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -98,6 +98,8 @@ def err_imported_module_relocated : Error<
def err_module_different_modmap : Error<
"module '%0' %select{uses|does not use}1 additional module map '%2'"
"%select{| not}1 used when the module was built">;
+def err_module_mismatch : Error<
+ "tried loading module '%0' from '%1' but found module '%2' instead">,
DefaultFatal;
def err_ast_file_macro_def_undef : Error<
"macro '%0' was %select{defined|undef'd}1 in the AST file '%2' but "
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 2779b3d1cf2ea..57f2a08e09359 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -423,6 +423,9 @@ class ASTReader
/// configuration.
ConfigurationMismatch,
+ /// The AST file contains a different module than expected.
+ ModuleMismatch,
+
/// The AST file has errors.
HadErrors
};
@@ -1512,11 +1515,13 @@ class ASTReader
SourceLocation ImportLoc, ModuleFile *ImportedBy,
SmallVectorImpl<ImportedModule> &Loaded,
off_t ExpectedSize, time_t ExpectedModTime,
+ StringRef ExpectedModuleName,
ASTFileSignature ExpectedSignature,
unsigned ClientLoadCapabilities);
ASTReadResult ReadControlBlock(ModuleFile &F,
SmallVectorImpl<ImportedModule> &Loaded,
const ModuleFile *ImportedBy,
+ StringRef ExpectedModuleName,
unsigned ClientLoadCapabilities);
static ASTReadResult
ReadOptionsBlock(llvm::BitstreamCursor &Stream, StringRef Filename,
@@ -1819,6 +1824,18 @@ class ASTReader
unsigned ClientLoadCapabilities,
ModuleFile **NewLoadedModuleFile = nullptr);
+ /// \overload
+ ///
+ /// Calls the above function and checks if the AST file contains the expected
+ /// module. Returns ASTReadResult::Failure on mismatch.
+ ///
+ /// \param ExpectedModuleName The expected name of the new loaded module.
+ ASTReadResult ReadAST(StringRef FileName, ModuleKind Type,
+ SourceLocation ImportLoc,
+ unsigned ClientLoadCapabilities,
+ StringRef ExpectedModuleName,
+ ModuleFile **NewLoadedModuleFile = nullptr);
+
/// Make the entities in the given module and any of its (non-explicit)
/// submodules visible to name lookup.
///
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 0a5f1cfd1a264..7500be81ea976 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -887,6 +887,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
case ASTReader::OutOfDate:
case ASTReader::VersionMismatch:
case ASTReader::ConfigurationMismatch:
+ case ASTReader::ModuleMismatch:
case ASTReader::HadErrors:
AST->getDiagnostics().Report(diag::err_fe_unable_to_load_pch);
return nullptr;
diff --git a/clang/lib/Frontend/ChainedIncludesSource.cpp
b/clang/lib/Frontend/ChainedIncludesSource.cpp
index a7096e27796a0..ee0363249124b 100644
--- a/clang/lib/Frontend/ChainedIncludesSource.cpp
+++ b/clang/lib/Frontend/ChainedIncludesSource.cpp
@@ -81,6 +81,7 @@ createASTReader(CompilerInstance &CI, StringRef pchFile,
case ASTReader::OutOfDate:
case ASTReader::VersionMismatch:
case ASTReader::ConfigurationMismatch:
+ case ASTReader::ModuleMismatch:
case ASTReader::HadErrors:
break;
}
diff --git a/clang/lib/Frontend/CompilerInstance.cpp
b/clang/lib/Frontend/CompilerInstance.cpp
index 91093d3ccb84c..95b3ae34f36fe 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -676,6 +676,7 @@ IntrusiveRefCntPtr<ASTReader>
CompilerInstance::createPCHExternalASTSource(
case ASTReader::OutOfDate:
case ASTReader::VersionMismatch:
case ASTReader::ConfigurationMismatch:
+ case ASTReader::ModuleMismatch:
case ASTReader::HadErrors:
// No suitable PCH file could be found. Return an error.
break;
@@ -1909,13 +1910,12 @@ ModuleLoadResult
CompilerInstance::findOrCompileModuleAndReadAST(
: Source == MS_PrebuiltModulePath
? 0
: ASTReader::ARR_ConfigurationMismatch;
- switch (getASTReader()->ReadAST(ModuleFilename,
- Source == MS_PrebuiltModulePath
- ? serialization::MK_PrebuiltModule
- : Source == MS_ModuleBuildPragma
- ? serialization::MK_ExplicitModule
- : serialization::MK_ImplicitModule,
- ImportLoc, ARRFlags)) {
+ switch (getASTReader()->ReadAST(
+ ModuleFilename,
+ Source == MS_PrebuiltModulePath ? serialization::MK_PrebuiltModule
+ : Source == MS_ModuleBuildPragma ? serialization::MK_ExplicitModule
+ : serialization::MK_ImplicitModule,
+ ImportLoc, ARRFlags, ModuleName)) {
case ASTReader::Success: {
if (M)
return M;
@@ -1952,6 +1952,7 @@ ModuleLoadResult
CompilerInstance::findOrCompileModuleAndReadAST(
// Fall through to error out.
[[fallthrough]];
case ASTReader::VersionMismatch:
+ case ASTReader::ModuleMismatch:
case ASTReader::HadErrors:
ModuleLoader::HadFatalFailure = true;
// FIXME: The ASTReader will already have complained, but can we shoehorn
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0cd2cedb48dd9..afae2f3a0e217 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2926,6 +2926,8 @@ static bool isDiagnosedResult(ASTReader::ASTReadResult
ARR, unsigned Caps) {
case ASTReader::VersionMismatch: return !(Caps &
ASTReader::ARR_VersionMismatch);
case ASTReader::ConfigurationMismatch:
return !(Caps & ASTReader::ARR_ConfigurationMismatch);
+ case ASTReader::ModuleMismatch:
+ return true;
case ASTReader::HadErrors: return true;
case ASTReader::Success: return false;
}
@@ -3020,11 +3022,10 @@ ASTReader::ASTReadResult ASTReader::ReadOptionsBlock(
}
}
-ASTReader::ASTReadResult
-ASTReader::ReadControlBlock(ModuleFile &F,
- SmallVectorImpl<ImportedModule> &Loaded,
- const ModuleFile *ImportedBy,
- unsigned ClientLoadCapabilities) {
+ASTReader::ASTReadResult ASTReader::ReadControlBlock(
+ ModuleFile &F, SmallVectorImpl<ImportedModule> &Loaded,
+ const ModuleFile *ImportedBy, StringRef ExpectedModuleName,
+ unsigned ClientLoadCapabilities) {
BitstreamCursor &Stream = F.Stream;
if (llvm::Error Err = Stream.EnterSubBlock(CONTROL_BLOCK_ID)) {
@@ -3315,7 +3316,7 @@ ASTReader::ReadControlBlock(ModuleFile &F,
// Load the AST file.
auto Result = ReadASTCore(ImportedFile, ImportedKind, ImportLoc, &F,
- Loaded, StoredSize, StoredModTime,
+ Loaded, StoredSize, StoredModTime,
ImportedName,
StoredSignature, Capabilities);
// If we diagnosed a problem, produce a backtrace.
@@ -3338,7 +3339,10 @@ ASTReader::ReadControlBlock(ModuleFile &F,
case OutOfDate: return OutOfDate;
case VersionMismatch: return VersionMismatch;
case ConfigurationMismatch: return ConfigurationMismatch;
- case HadErrors: return HadErrors;
+ case ModuleMismatch:
+ return ModuleMismatch;
+ case HadErrors:
+ return HadErrors;
case Success: break;
}
break;
@@ -3363,6 +3367,14 @@ ASTReader::ReadControlBlock(ModuleFile &F,
if (Listener)
Listener->ReadModuleName(F.ModuleName);
+ // Return if the AST unexpectedly contains a different module
+ if (F.Kind == MK_PrebuiltModule && !ExpectedModuleName.empty() &&
+ F.ModuleName != ExpectedModuleName) {
+ Diag(diag::err_module_mismatch)
+ << ExpectedModuleName << F.FileName << F.ModuleName;
+ return ASTReadResult::ModuleMismatch;
+ }
+
// Validate the AST as soon as we have a name so we can exit early on
// failure.
if (ASTReadResult Result = readUnhashedControlBlockOnce())
@@ -4684,6 +4696,15 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef
FileName, ModuleKind Type,
SourceLocation ImportLoc,
unsigned ClientLoadCapabilities,
ModuleFile **NewLoadedModuleFile) {
+ return ReadAST(FileName, Type, ImportLoc, ClientLoadCapabilities, "",
+ NewLoadedModuleFile);
+}
+
+ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind
Type,
+ SourceLocation ImportLoc,
+ unsigned ClientLoadCapabilities,
+ StringRef ExpectedModuleName,
+ ModuleFile **NewLoadedModuleFile) {
llvm::TimeTraceScope scope("ReadAST", FileName);
llvm::SaveAndRestore SetCurImportLocRAII(CurrentImportLoc, ImportLoc);
@@ -4702,8 +4723,8 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef
FileName, ModuleKind Type,
SmallVector<ImportedModule, 4> Loaded;
if (ASTReadResult ReadResult =
ReadASTCore(FileName, Type, ImportLoc,
- /*ImportedBy=*/nullptr, Loaded, 0, 0, ASTFileSignature(),
- ClientLoadCapabilities)) {
+ /*ImportedBy=*/nullptr, Loaded, 0, 0, ExpectedModuleName,
+ ASTFileSignature(), ClientLoadCapabilities)) {
ModuleMgr.removeModules(ModuleMgr.begin() + NumModules);
// If we find that any modules are unusable, the global index is going
@@ -4954,25 +4975,26 @@ static unsigned moduleKindForDiagnostic(ModuleKind
Kind) {
llvm_unreachable("unknown module kind");
}
-ASTReader::ASTReadResult
-ASTReader::ReadASTCore(StringRef FileName,
- ModuleKind Type,
- SourceLocation ImportLoc,
- ModuleFile *ImportedBy,
- SmallVectorImpl<ImportedModule> &Loaded,
- off_t ExpectedSize, time_t ExpectedModTime,
- ASTFileSignature ExpectedSignature,
- unsigned ClientLoadCapabilities) {
+ASTReader::ASTReadResult ASTReader::ReadASTCore(
+ StringRef FileName, ModuleKind Type, SourceLocation ImportLoc,
+ ModuleFile *ImportedBy, SmallVectorImpl<ImportedModule> &Loaded,
+ off_t ExpectedSize, time_t ExpectedModTime, StringRef ExpectedModuleName,
+ ASTFileSignature ExpectedSignature, unsigned ClientLoadCapabilities) {
ModuleFile *M;
std::string ErrorStr;
- ModuleManager::AddModuleResult AddResult
- = ModuleMgr.addModule(FileName, Type, ImportLoc, ImportedBy,
- getGeneration(), ExpectedSize, ExpectedModTime,
- ExpectedSignature, readASTFileSignature,
- M, ErrorStr);
+ ModuleManager::AddModuleResult AddResult = ModuleMgr.addModule(
+ FileName, Type, ImportLoc, ImportedBy, getGeneration(), ExpectedSize,
+ ExpectedModTime, ExpectedSignature, readASTFileSignature, M, ErrorStr);
switch (AddResult) {
case ModuleManager::AlreadyLoaded:
+ // Return if the AST unexpectedly contains a different module
+ if (Type == MK_PrebuiltModule && !ExpectedModuleName.empty() &&
+ M->ModuleName != ExpectedModuleName) {
+ Diag(diag::err_module_mismatch)
+ << ExpectedModuleName << FileName << M->ModuleName;
+ return ASTReadResult::ModuleMismatch;
+ }
Diag(diag::remark_module_import)
<< M->ModuleName << M->FileName << (ImportedBy ? true : false)
<< (ImportedBy ? StringRef(ImportedBy->ModuleName) : StringRef());
@@ -5053,7 +5075,8 @@ ASTReader::ReadASTCore(StringRef FileName,
switch (Entry.ID) {
case CONTROL_BLOCK_ID:
HaveReadControlBlock = true;
- switch (ReadControlBlock(F, Loaded, ImportedBy, ClientLoadCapabilities))
{
+ switch (ReadControlBlock(F, Loaded, ImportedBy, ExpectedModuleName,
+ ClientLoadCapabilities)) {
case Success:
// Check that we didn't try to load a non-module AST file as a module.
//
@@ -5075,6 +5098,8 @@ ASTReader::ReadASTCore(StringRef FileName,
case OutOfDate: return OutOfDate;
case VersionMismatch: return VersionMismatch;
case ConfigurationMismatch: return ConfigurationMismatch;
+ case ModuleMismatch:
+ return ModuleMismatch;
case HadErrors: return HadErrors;
}
break;
diff --git a/clang/test/Modules/fmodule-file-bad-transitive-mapping.cpp
b/clang/test/Modules/fmodule-file-bad-transitive-mapping.cpp
new file mode 100644
index 0000000000000..5cf6d14b4a504
--- /dev/null
+++ b/clang/test/Modules/fmodule-file-bad-transitive-mapping.cpp
@@ -0,0 +1,46 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// Related to issue #132059
+
+// Precompile the module dependencies correctly
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface a.cppm -o a.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface b.cppm -o b.pcm
-fmodule-file=A=a.pcm
+
+// Test that providing incorrect mappings via
-fmodule-file=<name>=<path/to/bmi>,
+// where the BMI is built for a different module than the one specified and
+// transitively imports the specified module, does not crash the compiler.
+// RUN: not %clang_cc1 -std=c++20 main1.cpp -fmodule-file=A=b.pcm
+
+//--- a.cppm
+export module A;
+
+export int a() {
+ return 41;
+}
+
+//--- b.cppm
+export module B;
+import A;
+
+export int b() {
+ return a() + 1;
+}
+
+//--- main1.cpp
+import A;
+
+int main() {
+ return a();
+}
+
+// Test again for the case where the BMI is first loaded correctly
+// RUN: not %clang_cc1 -std=c++20 main2.cpp-fmodule-file=B=b.pcm
-fmodule-file=A=b.pcm
+
+//--- main2.cpp
+import B;
+
+int main() {
+ return b();
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
