From 8bf778eb9c0afb0a4c63a97ce504f50759c08d5f Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Wed, 6 Feb 2013 22:40:31 +0000 Subject: [PATCH] Detect when we end up trying to load conflicting module files. This can happen when one abuses precompiled headers by passing more -D options when using a precompiled hedaer than when it was built. This is intentionally permitted by precompiled headers (and is exploited by some build environments), but causes problems for modules. First part of , detecting when something when horribly wrong. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174554 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticCommonKinds.td | 4 ++++ lib/Frontend/CompilerInstance.cpp | 21 ++++++++++++++++--- lib/Serialization/ASTReader.cpp | 13 +++++++++++- test/Modules/Inputs/ignored_macros.h | 8 +++++++ test/Modules/Inputs/module.map | 4 ++++ test/Modules/ignored_macros.m | 22 ++++++++++++++++++++ 6 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 test/Modules/Inputs/ignored_macros.h create mode 100644 test/Modules/ignored_macros.m diff --git a/include/clang/Basic/DiagnosticCommonKinds.td b/include/clang/Basic/DiagnosticCommonKinds.td index 7d965503c4..7dccb7326a 100644 --- a/include/clang/Basic/DiagnosticCommonKinds.td +++ b/include/clang/Basic/DiagnosticCommonKinds.td @@ -118,4 +118,8 @@ def err_unable_to_rename_temp : Error< "unable to rename temporary '%0' to output file '%1': '%2'">; def err_unable_to_make_temp : Error< "unable to make temporary file: %0">; + +// Modules +def err_module_file_conflict : Error<"module '%0' found in both '%1' and '%2'">; + } diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp index d4a351394a..b6115ec6ff 100644 --- a/lib/Frontend/CompilerInstance.cpp +++ b/lib/Frontend/CompilerInstance.cpp @@ -915,9 +915,9 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, // Search for a module with the given name. Module = PP->getHeaderSearchInfo().lookupModule(ModuleName); std::string ModuleFileName; - if (Module) + if (Module) { ModuleFileName = PP->getHeaderSearchInfo().getModuleFileName(Module); - else + } else ModuleFileName = PP->getHeaderSearchInfo().getModuleFileName(ModuleName); if (ModuleFileName.empty()) { @@ -987,6 +987,20 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, return ModuleLoadResult(); } + // If there is already a module file associated with this module, make sure + // it is the same as the module file we're looking for. Otherwise, we + // have two module files for the same module. + if (const FileEntry *CurModuleFile = Module? Module->getASTFile() : 0) { + if (CurModuleFile != ModuleFile) { + getDiagnostics().Report(ModuleNameLoc, diag::err_module_file_conflict) + << ModuleName + << CurModuleFile->getName() + << ModuleFile->getName(); + ModuleBuildFailed = true; + return ModuleLoadResult(); + } + } + // If we don't already have an ASTReader, create one now. if (!ModuleManager) { if (!hasASTContext()) @@ -1085,8 +1099,9 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, .findModule((Path[0].first->getName())); } - if (Module) + if (Module) { Module->setASTFile(ModuleFile); + } // Cache the result of this top-level module lookup for later. Known = KnownModules.insert(std::make_pair(Path[0].first, Module)).first; diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index dd2d5d12bc..aa4c448e98 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -3408,7 +3408,18 @@ bool ASTReader::ReadSubmoduleBlock(ModuleFile &F) { Error("too many submodules"); return true; } - + + if (const FileEntry *CurFile = CurrentModule->getASTFile()) { + if (CurFile != F.File) { + if (!Diags.isDiagnosticInFlight()) { + Diag(diag::err_module_file_conflict) + << CurrentModule->getTopLevelModuleName() + << CurFile->getName() + << F.File->getName(); + } + return true; + } + } CurrentModule->setASTFile(F.File); CurrentModule->IsFromModuleFile = true; CurrentModule->IsSystem = IsSystem || CurrentModule->IsSystem; diff --git a/test/Modules/Inputs/ignored_macros.h b/test/Modules/Inputs/ignored_macros.h new file mode 100644 index 0000000000..250b58c2f7 --- /dev/null +++ b/test/Modules/Inputs/ignored_macros.h @@ -0,0 +1,8 @@ +struct Point { + double x, y; +}; + +#ifdef IGNORED +int *has_ignored(void); +#endif + diff --git a/test/Modules/Inputs/module.map b/test/Modules/Inputs/module.map index 55496fa35a..234bfcc2b0 100644 --- a/test/Modules/Inputs/module.map +++ b/test/Modules/Inputs/module.map @@ -159,3 +159,7 @@ module autolink { module weird_objc { header "weird_objc.h" } + +module ignored_macros { + header "ignored_macros.h" +} diff --git a/test/Modules/ignored_macros.m b/test/Modules/ignored_macros.m new file mode 100644 index 0000000000..1f9c27c2a3 --- /dev/null +++ b/test/Modules/ignored_macros.m @@ -0,0 +1,22 @@ +// First trial: pass -DIGNORED=1 to both. It should be ignored in both +// RUN: rm -rf %t.modules +// RUN: %clang_cc1 -fmodule-cache-path %t.modules -DIGNORED=1 -fmodules -I %S/Inputs -emit-pch -o %t.pch -x objective-c-header %s -verify +// RUN: %clang_cc1 -fmodule-cache-path %t.modules -DIGNORED=1 -fmodules -I %S/Inputs -include-pch %t.pch %s -verify + +// Second trial: pass -DIGNORED=1 only to the second invocation. +// RUN: rm -rf %t.modules +// RUN: %clang_cc1 -fmodule-cache-path %t.modules -fmodules -I %S/Inputs -emit-pch -o %t.pch -x objective-c-header %s -verify +// RUN: not %clang_cc1 -fmodule-cache-path %t.modules -DIGNORED=1 -fmodules -I %S/Inputs -include-pch %t.pch %s > %t.err 2>&1 +// RUN: FileCheck -check-prefix=CHECK-CONFLICT %s < %t.err +// CHECK-CONFLICT: module 'ignored_macros' found in both + +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER +@import ignored_macros; +#endif + +@import ignored_macros; + +struct Point p;