Author: Cyndy Ishida Date: 2025-04-29T08:38:33-07:00 New Revision: bd3dde0f871cd71a797ba5da3070fa3ddbc48828
URL: https://github.com/llvm/llvm-project/commit/bd3dde0f871cd71a797ba5da3070fa3ddbc48828 DIFF: https://github.com/llvm/llvm-project/commit/bd3dde0f871cd71a797ba5da3070fa3ddbc48828.diff LOG: [clang][Modules] Diagnose mismatching pcm dependencies in explicit buiilds (#137068) In an explicit build, the dependency scanner generates invocations with dependencies to module files to use during compilation. The pcm's passed in the invocations should match the ones that were imported by other modules that share the same dependencies. We have seen bugs caused from incorrect invocations that mismatch which module file to load. When this happens report it as a warning, to help with investigations, as that should never occur in a well behaved build scheduled from the dependency scanner. The warning flag is off by default. Added: clang/test/Modules/invalid-module-dep.c Modified: clang/include/clang/Basic/DiagnosticSerializationKinds.td clang/lib/Serialization/ASTReader.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td index 5fc5937b80d35..7965da593f218 100644 --- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -139,6 +139,11 @@ def warn_decls_in_multiple_modules : Warning< InGroup<DiagGroup<"decls-in-multiple-modules">>, DefaultIgnore; +def warn_module_file_mapping_mismatch + : Warning<"loaded module file '%0' conflicts with imported file '%1'">, + InGroup<DiagGroup<"module-file-mapping-mismatch">>, + DefaultIgnore; + def err_failed_to_find_module_file : Error< "failed to find module file for module '%0'">; } // let CategoryName diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index f13a173ec933e..f1ed67afaab7a 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3286,6 +3286,8 @@ ASTReader::ReadControlBlock(ModuleFile &F, time_t StoredModTime = 0; ASTFileSignature StoredSignature; std::string ImportedFile; + std::string StoredFile; + bool IgnoreImportedByNote = false; // For prebuilt and explicit modules first consult the file map for // an override. Note that here we don't search prebuilt module @@ -3311,11 +3313,26 @@ ASTReader::ReadControlBlock(ModuleFile &F, SignatureBytes.end()); Blob = Blob.substr(ASTFileSignature::size); + // Use BaseDirectoryAsWritten to ensure we use the same path in the + // ModuleCache as when writing. + StoredFile = ReadPathBlob(BaseDirectoryAsWritten, Record, Idx, Blob); if (ImportedFile.empty()) { - // Use BaseDirectoryAsWritten to ensure we use the same path in the - // ModuleCache as when writing. - ImportedFile = - ReadPathBlob(BaseDirectoryAsWritten, Record, Idx, Blob); + ImportedFile = StoredFile; + } else if (!getDiags().isIgnored( + diag::warn_module_file_mapping_mismatch, + CurrentImportLoc)) { + auto ImportedFileRef = + PP.getFileManager().getOptionalFileRef(ImportedFile); + auto StoredFileRef = + PP.getFileManager().getOptionalFileRef(StoredFile); + if ((ImportedFileRef && StoredFileRef) && + (*ImportedFileRef != *StoredFileRef)) { + Diag(diag::warn_module_file_mapping_mismatch) + << ImportedFile << StoredFile; + Diag(diag::note_module_file_imported_by) + << F.FileName << !F.ModuleName.empty() << F.ModuleName; + IgnoreImportedByNote = true; + } } } @@ -3349,7 +3366,8 @@ ASTReader::ReadControlBlock(ModuleFile &F, .getModuleCache() .getInMemoryModuleCache() .isPCMFinal(F.FileName); - if (isDiagnosedResult(Result, Capabilities) || recompilingFinalized) + if (!IgnoreImportedByNote && + (isDiagnosedResult(Result, Capabilities) || recompilingFinalized)) Diag(diag::note_module_file_imported_by) << F.FileName << !F.ModuleName.empty() << F.ModuleName; diff --git a/clang/test/Modules/invalid-module-dep.c b/clang/test/Modules/invalid-module-dep.c new file mode 100644 index 0000000000000..2bcda8be5f1b6 --- /dev/null +++ b/clang/test/Modules/invalid-module-dep.c @@ -0,0 +1,64 @@ +/// This tests the expected error case when there is a mismatch between the pcm dependencies passed in +/// the command line with `fmodule-file` and whats encoded in the pcm. + +/// The steps are: +/// 1. Build the module A with no dependencies. The first variant to build is A-1.pcm. +/// 2. Build the same module with files that resolve from diff erent search paths. +/// This variant is named A-2.pcm. +/// 3. Build module B that depends on the earlier module A-1.pcm. +/// 4. Build client that directly depends on both modules (A & B), +/// but depends on a incompatible variant of A (A-2.pcm) for B to use. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -emit-module -x c -fmodules -fno-implicit-modules -isysroot %t/Sysroot \ +// RUN: -I%t/Sysroot/usr/include \ +// RUN: -fmodule-name=A %t/Sysroot/usr/include/A/module.modulemap -o %t/A-1.pcm + +// RUN: %clang_cc1 -emit-module -x c -fmodules -fno-implicit-modules -isysroot %t/Sysroot \ +// RUN: -I%t/BuildDir \ +// RUN: -fmodule-name=A %t/BuildDir/A/module.modulemap -o %t/A-2.pcm + +// RUN: %clang_cc1 -emit-module -x c -fmodules -fno-implicit-modules -isysroot %t/Sysroot \ +// RUN: -I%t/Sysroot/usr/include \ +// RUN: -fmodule-map-file=%t/Sysroot/usr/include/A/module.modulemap \ +// RUN: -fmodule-file=A=%t/A-1.pcm \ +// RUN: -fmodule-name=B %t/Sysroot/usr/include/B/module.modulemap -o %t/B-1.pcm + +// RUN: %clang_cc1 -x c -fmodules -fno-implicit-modules -isysroot %t/Sysroot \ +// RUN: -I%t/BuildDir -I%t/Sysroot/usr/include \ +// RUN: -fmodule-map-file=%t/BuildDir/A/module.modulemap \ +// RUN: -fmodule-map-file=%t/Sysroot/usr/include/B/module.modulemap \ +// RUN: -fmodule-file=A=%t/A-2.pcm -fmodule-file=B=%t/B-1.pcm \ +// RUN: -Wmodule-file-mapping-mismatch -verify %s + + +#include <A/A.h> +#include <B/B.h> // expected-warning {{conflicts with imported file}} \ + // expected-note {{imported by module 'B'}} \ + // expected-error {{out of date and needs to be rebuilt}} + +//--- Sysroot/usr/include/A/module.modulemap +module A [system] { + umbrella header "A.h" +} +//--- Sysroot/usr/include/A/A.h +typedef int A_t; + +//--- Sysroot/usr/include/B/module.modulemap +module B [system] { + umbrella header "B.h" +} + +//--- Sysroot/usr/include/B/B.h +#include <A/A.h> +typedef int B_t; + +//--- BuildDir/A/module.modulemap +module A [system] { + umbrella header "A.h" +} + +//--- BuildDir/A/A.h +typedef int A_t; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
