diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -279,6 +279,13 @@ unevaluated operands of a ``typeid`` expression, as they are now modeled correctly in the CFG. This fixes `Issue 21668 `_. +- When including a PCH from a GCC style directory with multiple alternative PCH + files, Clang now requires all defines set on the command line while generating + the PCH and when including it to match. This matches GCC's behaviour. + Previously Clang would tolerate defines to be set when creating the PCH but + missing when used, or vice versa. This makes sure that Clang picks the + correct one, where it previously would consider multiple ones as potentially + acceptable (and erroneously use whichever one is tried first). Non-comprehensive list of changes in this release ------------------------------------------------- diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1738,7 +1738,8 @@ const LangOptions &LangOpts, const TargetOptions &TargetOpts, const PreprocessorOptions &PPOpts, - StringRef ExistingModuleCachePath); + StringRef ExistingModuleCachePath, + bool RequireStrictOptionMatches = false); /// Returns the suggested contents of the predefines buffer, /// which contains a (typically-empty) subset of the predefines diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -772,7 +772,7 @@ if (ASTReader::isAcceptableASTFile( Dir->path(), FileMgr, CI.getPCHContainerReader(), CI.getLangOpts(), CI.getTargetOpts(), CI.getPreprocessorOpts(), - SpecificModuleCachePath)) { + SpecificModuleCachePath, /*RequireStrictOptionMatches=*/true)) { PPOpts.ImplicitPCHInclude = std::string(Dir->path()); Found = true; break; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -637,7 +637,8 @@ FileManager &FileMgr, std::string &SuggestedPredefines, const LangOptions &LangOpts, - bool Validate = true) { + bool Validate = true, + bool ValidateStrict = false) { // Check macro definitions. MacroDefinitionsMap ASTFileMacros; collectMacroDefinitions(PPOpts, ASTFileMacros); @@ -654,6 +655,14 @@ llvm::StringMap>::iterator Known = ASTFileMacros.find(MacroName); if (!Validate || Known == ASTFileMacros.end()) { + if (ValidateStrict) { + // If strict matches are requested, don't tolerate any extra defines on + // the command line that are missing in the AST file. + if (Diags) { + Diags->Report(diag::err_pch_macro_def_undef) << MacroName << true; + } + return true; + } // FIXME: Check whether this identifier was referenced anywhere in the // AST file. If so, we should reject the AST file. Unfortunately, this // information isn't in the control block. What shall we do about it? @@ -684,8 +693,10 @@ // If the macro was #undef'd in both, or if the macro bodies are identical, // it's fine. - if (Existing.second || Existing.first == Known->second.first) + if (Existing.second || Existing.first == Known->second.first) { + ASTFileMacros.erase(Known); continue; + } // The macro bodies differ; complain. if (Diags) { @@ -694,6 +705,16 @@ } return true; } + if (ValidateStrict) { + // If strict matches are requested, don't tolerate any extra defines in + // the AST file that are missing on the command line. + for (const auto &MacroName : ASTFileMacros.keys()) { + if (Diags) { + Diags->Report(diag::err_pch_macro_def_undef) << MacroName << false; + } + return true; + } + } // Check whether we're using predefines. if (PPOpts.UsePredefines != ExistingPPOpts.UsePredefines && Validate) { @@ -5137,16 +5158,19 @@ const PreprocessorOptions &ExistingPPOpts; std::string ExistingModuleCachePath; FileManager &FileMgr; + bool StrictOptionMatches; public: SimplePCHValidator(const LangOptions &ExistingLangOpts, const TargetOptions &ExistingTargetOpts, const PreprocessorOptions &ExistingPPOpts, - StringRef ExistingModuleCachePath, FileManager &FileMgr) + StringRef ExistingModuleCachePath, FileManager &FileMgr, + bool StrictOptionMatches) : ExistingLangOpts(ExistingLangOpts), ExistingTargetOpts(ExistingTargetOpts), ExistingPPOpts(ExistingPPOpts), - ExistingModuleCachePath(ExistingModuleCachePath), FileMgr(FileMgr) {} + ExistingModuleCachePath(ExistingModuleCachePath), FileMgr(FileMgr), + StrictOptionMatches(StrictOptionMatches) {} bool ReadLanguageOptions(const LangOptions &LangOpts, bool Complain, bool AllowCompatibleDifferences) override { @@ -5173,7 +5197,8 @@ std::string &SuggestedPredefines) override { return checkPreprocessorOptions(PPOpts, ExistingPPOpts, /*Diags=*/nullptr, FileMgr, SuggestedPredefines, - ExistingLangOpts); + ExistingLangOpts, /*Validate=*/true, + /*ValidateStrict=*/StrictOptionMatches); } }; @@ -5450,9 +5475,11 @@ const LangOptions &LangOpts, const TargetOptions &TargetOpts, const PreprocessorOptions &PPOpts, - StringRef ExistingModuleCachePath) { + StringRef ExistingModuleCachePath, + bool RequireStrictOptionMatches) { SimplePCHValidator validator(LangOpts, TargetOpts, PPOpts, - ExistingModuleCachePath, FileMgr); + ExistingModuleCachePath, FileMgr, + RequireStrictOptionMatches); return !readASTFileControlBlock(Filename, FileMgr, PCHContainerRdr, /*FindModuleFileExtensions=*/false, validator, diff --git a/clang/test/PCH/pch-dir.c b/clang/test/PCH/pch-dir.c --- a/clang/test/PCH/pch-dir.c +++ b/clang/test/PCH/pch-dir.c @@ -6,7 +6,7 @@ // RUN: %clang -x c++-header -std=c++98 %S/pch-dir.h -o %t.h.gch/cpp.gch // RUN: %clang -include %t.h -DFOO=foo -fsyntax-only %s -Xclang -print-stats 2> %t.clog // RUN: FileCheck -check-prefix=CHECK-C %s < %t.clog -// RUN: %clang -include %t.h -DFOO=bar -DBAR=bar -fsyntax-only %s -Xclang -ast-print > %t.cbarlog +// RUN: %clang -include %t.h -DFOO=bar -fsyntax-only %s -Xclang -ast-print > %t.cbarlog // RUN: FileCheck -check-prefix=CHECK-CBAR %s < %t.cbarlog // RUN: %clang -x c++ -include %t.h -std=c++98 -fsyntax-only %s -Xclang -print-stats 2> %t.cpplog // RUN: FileCheck -check-prefix=CHECK-CPP %s < %t.cpplog @@ -14,6 +14,11 @@ // RUN: not %clang -x c++ -std=c++11 -include %t.h -fsyntax-only %s 2> %t.cpp11log // RUN: FileCheck -check-prefix=CHECK-NO-SUITABLE %s < %t.cpp11log +// RUN: not %clang -include %t.h -fsyntax-only %s 2> %t.missinglog2 +// RUN: FileCheck -check-prefix=CHECK-NO-SUITABLE %s < %t.missinglog2 +// RUN: not %clang -include %t.h -DFOO=foo -DBAR=bar -fsyntax-only %s 2> %t.missinglog2 +// RUN: FileCheck -check-prefix=CHECK-NO-SUITABLE %s < %t.missinglog2 + // Don't crash if the precompiled header file is missing. // RUN: not %clang_cc1 -include-pch %t.h.gch -DFOO=baz -fsyntax-only %s -print-stats 2> %t.missinglog // RUN: FileCheck -check-prefix=CHECK-NO-SUITABLE %s < %t.missinglog