Modules: Ignore diagnostic mismatches on validated modules
This is a minor update to bded4ae30659, to avoid erroring out when
trying to import a module whose diagnostics don't match, as long as it
has already been validated elsewhere in this process. If it was
validated before, we should just use it.
The testcase, which Adrian helped me generate, does the following:
- Assume two modules:
1. Mismatch, which is not marked as a system module.
2. System, which *is* marked as a system module. It imports
Mismatch.
- When imported on its own, Mismatch will be considered a "user"
module; inside of System, Mismatch will be considered a "system"
module.
- With a cold cache, build the following translation units:
1. Generate Mismatch as a user module with -Wno-system-headers
(the default).
@import Mismatch; // Built.
2. Import Mismatch and generate System with -Wsystem-headers.
@import Mismatch; // Imported in place.
@import System; // Built; needs to import import Mismatch.
The diagnostic validation for the direct import of Mismatch in (2)
doesn't care that -Wsystem-headers has been turned on, because it won't
affect how Mismatch (a user module) is treated. However, the build of
the System cares that Mismatch was built without -Wsystem-headers, since
now Mismatch is being re-interpreted as a system module.
To support this change, I've restored the previous behaviour of checking
the signature for new modules inside ModuleManager::addModule. We need
to be sure that the checks in ASTReader::findAndReadUnhashedControlBlock
aren't letting mismatched signatures slip through (since that will
trigger a rebuild in the *importing* module). Without this change,
test/Modules/outofdate-rebuild.m spews ASan errors.
diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index b4946ab..c5edca7 100644
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -4050,16 +4050,18 @@
// If this module was validated as a system module, and it is now a
// non-system module, emit a warning about ignoring differences in
// diagnostic options.
+ //
+ // In any case, if it has been validated in this process somehow, we're
+ // stuck with it (and it should be good enough!).
bool IsSystem;
if (BufferMgr->isValidatedByAncestor(F.FileName, IsSystem)) {
if (IsSystem) {
Module *M = getTopImportImplicitModule(ModuleMgr, PP);
assert(M && "missing module");
- if (!M->IsSystem) {
+ if (!M->IsSystem)
Diag(diag::warn_module_system_bit_conflict) << F.FileName;
- return Success;
- }
}
+ return Success;
}
}
diff --git a/lib/Serialization/ModuleManager.cpp b/lib/Serialization/ModuleManager.cpp
index cefacc1..d3c1a5d 100644
--- a/lib/Serialization/ModuleManager.cpp
+++ b/lib/Serialization/ModuleManager.cpp
@@ -93,13 +93,7 @@
New->FileName = FileName.str();
New->File = Entry;
New->ImportLoc = ImportLoc;
- Chain.push_back(New);
- if (!New->isModule())
- PCHChain.push_back(New);
- if (!ImportedBy)
- Roots.push_back(New);
NewModule = true;
- ModuleEntry = New;
New->InputFilesValidationTimestamp = 0;
if (New->Kind == MK_ImplicitModule) {
@@ -146,6 +140,34 @@
// Initialize the stream.
PCHContainerRdr.ExtractPCH(New->Buffer->getMemBufferRef(), New->StreamFile);
+
+ if (ExpectedSignature != ASTFileSignature({{0}})) {
+ // Check the signature before hooking it up, so we can still delete it
+ // safely.
+ New->Signature = ReadSignature(New->StreamFile);
+ if (New->Signature != ExpectedSignature) {
+ ErrorStr = New->Signature != ASTFileSignature({{0}})
+ ? "signature mismatch"
+ : "could not read module signature";
+
+ bool IsSystem;
+ if (!FileMgr.getPCMCache()->isValidatedByAncestor(New->FileName,
+ IsSystem)) {
+ FileMgr.invalidateCache(New->File);
+ FileMgr.getPCMCache()->removeFromConsistentBuffer(New->FileName);
+ }
+ delete New;
+ return OutOfDate;
+ }
+ }
+
+ // We're keeping this module. Hook it up.
+ Chain.push_back(New);
+ if (!New->isModule())
+ PCHChain.push_back(New);
+ if (!ImportedBy)
+ Roots.push_back(New);
+ ModuleEntry = New;
} else if (ExpectedSignature != ASTFileSignature({{0}})) {
// Only check the signature here if the module has already been loaded. New
// modules may be invalidated and rebuilt, and the logic for that is in
diff --git a/test/Modules/Inputs/warning-mismatch/Mismatch.h b/test/Modules/Inputs/warning-mismatch/Mismatch.h
new file mode 100644
index 0000000..a07b0ee
--- /dev/null
+++ b/test/Modules/Inputs/warning-mismatch/Mismatch.h
@@ -0,0 +1 @@
+struct Mismatch { int i; };
diff --git a/test/Modules/Inputs/warning-mismatch/System.h b/test/Modules/Inputs/warning-mismatch/System.h
new file mode 100644
index 0000000..8e69e70
--- /dev/null
+++ b/test/Modules/Inputs/warning-mismatch/System.h
@@ -0,0 +1,2 @@
+#import "Mismatch.h"
+struct System { int i; };
diff --git a/test/Modules/Inputs/warning-mismatch/module.modulemap b/test/Modules/Inputs/warning-mismatch/module.modulemap
new file mode 100644
index 0000000..c22cde4
--- /dev/null
+++ b/test/Modules/Inputs/warning-mismatch/module.modulemap
@@ -0,0 +1,7 @@
+module System [system] {
+ header "System.h"
+}
+
+module Mismatch {
+ header "Mismatch.h"
+}
diff --git a/test/Modules/warning-mismatch.m b/test/Modules/warning-mismatch.m
new file mode 100644
index 0000000..7c83664
--- /dev/null
+++ b/test/Modules/warning-mismatch.m
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t.cache
+// RUN: echo "@import Mismatch;" >%t.m
+// RUN: %clang_cc1 -Wno-system-headers -fdisable-module-hash -fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps -I %S/Inputs/warning-mismatch %t.m -fsyntax-only -Rmodule-build
+// RUN: echo -----------------------------------------------
+// RUN: %clang_cc1 -Wsystem-headers -fdisable-module-hash -fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps -I %S/Inputs/warning-mismatch %s -fsyntax-only -Rmodule-build
+
+// This testcase triggers a warning flag mismatch in an already validated header.
+@import Mismatch;
+@import System;
