From: Douglas Gregor Date: Wed, 24 Oct 2012 23:41:50 +0000 (+0000) Subject: Teach the PCH validator to check the preprocessor options, especially X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4c0c7e86645dfa1719d17d70e009ab49347aba62;p=clang Teach the PCH validator to check the preprocessor options, especially the macros that are #define'd or #undef'd on the command line. This checking happens much earlier than the current macro-definition checking and is far cleaner, because it does a direct comparison rather than a diff of the predefines buffers. Moreover, it allows us to use the result of this check to skip over PCH files within a directory that have non-matching -D's or -U's on the command line. Finally, it improves the diagnostics a bit for mismatches, fixing . The old predefines-buffer diff'ing will go away in a subsequent commit. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@166641 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSerializationKinds.td b/include/clang/Basic/DiagnosticSerializationKinds.td index 435760bb87..978a35d1d9 100644 --- a/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/include/clang/Basic/DiagnosticSerializationKinds.td @@ -45,6 +45,25 @@ def warn_pch_different_branch : Error< "PCH file built from a different branch (%0) than the compiler (%1)">; def err_pch_with_compiler_errors : Error< "PCH file contains compiler errors">; + + +def err_pch_macro_def_undef : Error< + "macro '%0' was %select{defined|undef'd}1 in the precompiled header but " + "%select{undef'd|defined}1 on the command line">; +def err_pch_macro_def_conflict : Error< + "definition of macro '%0' differs between the precompiled header ('%1') " + "and the command line ('%2')">; +def err_pch_include_opt_missing : Error< + "precompiled header depends on '%select{-include|-imacros}0 %1' option " + "that is missing from the command line">; +def err_pch_include_opt_conflict : Error< + "precompiled header option '%select{-include|-imacros}0 %1' conflicts with " + "corresponding option '%select{-include|-imacros}0 %2' on command line">; +def err_pch_undef : Error< + "%select{command line contains|precompiled header was built with}0 " + "'-undef' but %select{precompiled header was not built with it|" + "it is not present on the command line}0">; + def warn_cmdline_conflicting_macro_def : Error< "definition of the macro '%0' conflicts with the definition used to " "build the precompiled header">; diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index d285ca504a..3b5cf8d396 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/include/clang/Serialization/ASTReader.h @@ -203,6 +203,8 @@ public: bool Complain); virtual bool ReadTargetOptions(const TargetOptions &TargetOpts, bool Complain); + virtual bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, + bool Complain); virtual bool ReadPredefinesBuffer(const PCHPredefinesBlocks &Buffers, StringRef OriginalFileName, std::string &SuggestedPredefines, @@ -1209,7 +1211,8 @@ public: static bool isAcceptableASTFile(StringRef Filename, FileManager &FileMgr, const LangOptions &LangOpts, - const TargetOptions &TargetOpts); + const TargetOptions &TargetOpts, + const PreprocessorOptions &PPOpts); /// \brief Returns the suggested contents of the predefines buffer, /// which contains a (typically-empty) subset of the predefines diff --git a/lib/Frontend/FrontendAction.cpp b/lib/Frontend/FrontendAction.cpp index b7b93a9178..fa1655db79 100644 --- a/lib/Frontend/FrontendAction.cpp +++ b/lib/Frontend/FrontendAction.cpp @@ -243,7 +243,8 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, // Check whether this is an acceptable AST file. if (ASTReader::isAcceptableASTFile(Dir->path(), FileMgr, CI.getLangOpts(), - CI.getTargetOpts())) { + CI.getTargetOpts(), + CI.getPreprocessorOpts())) { for (unsigned I = 0, N = PPOpts.Includes.size(); I != N; ++I) { if (PPOpts.Includes[I] == PPOpts.ImplicitPCHInclude) { PPOpts.Includes[I] = Dir->path(); diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index ccadfbd07a..4d0ad91477 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -187,8 +187,8 @@ static bool checkTargetOptions(const TargetOptions &TargetOpts, bool PCHValidator::ReadLanguageOptions(const LangOptions &LangOpts, bool Complain) { - const LangOptions &PPLangOpts = PP.getLangOpts(); - return checkLanguageOptions(LangOpts, PPLangOpts, + const LangOptions &ExistingLangOpts = PP.getLangOpts(); + return checkLanguageOptions(LangOpts, ExistingLangOpts, Complain? &Reader.Diags : 0); } @@ -199,6 +199,119 @@ bool PCHValidator::ReadTargetOptions(const TargetOptions &TargetOpts, Complain? &Reader.Diags : 0); } +namespace { + typedef llvm::StringMap > + MacroDefinitionsMap; +} + +/// \brief Collect the macro definitions provided by the given preprocessor +/// options. +static void collectMacroDefinitions(const PreprocessorOptions &PPOpts, + MacroDefinitionsMap &Macros, + SmallVectorImpl *MacroNames = 0){ + for (unsigned I = 0, N = PPOpts.Macros.size(); I != N; ++I) { + StringRef Macro = PPOpts.Macros[I].first; + bool IsUndef = PPOpts.Macros[I].second; + + std::pair MacroPair = Macro.split('='); + StringRef MacroName = MacroPair.first; + StringRef MacroBody = MacroPair.second; + + // For an #undef'd macro, we only care about the name. + if (IsUndef) { + if (MacroNames && !Macros.count(MacroName)) + MacroNames->push_back(MacroName); + + Macros[MacroName] = std::make_pair("", true); + continue; + } + + // For a #define'd macro, figure out the actual definition. + if (MacroName.size() == Macro.size()) + MacroBody = "1"; + else { + // Note: GCC drops anything following an end-of-line character. + StringRef::size_type End = MacroBody.find_first_of("\n\r"); + MacroBody = MacroBody.substr(0, End); + } + + if (MacroNames && !Macros.count(MacroName)) + MacroNames->push_back(MacroName); + Macros[MacroName] = std::make_pair(MacroBody, false); + } +} + +/// \brief Check the preprocessor options deserialized from the control block +/// against the preprocessor options in an existing preprocessor. +/// +/// \param Diags If non-null, produce diagnostics for any mismatches incurred. +static bool checkPreprocessorOptions(const PreprocessorOptions &PPOpts, + const PreprocessorOptions &ExistingPPOpts, + DiagnosticsEngine *Diags) { + // Check macro definitions. + MacroDefinitionsMap ASTFileMacros; + collectMacroDefinitions(PPOpts, ASTFileMacros); + MacroDefinitionsMap ExistingMacros; + SmallVector ExistingMacroNames; + collectMacroDefinitions(ExistingPPOpts, ExistingMacros, &ExistingMacroNames); + + for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) { + // Dig out the macro definition in the existing preprocessor options. + StringRef MacroName = ExistingMacroNames[I]; + std::pair Existing = ExistingMacros[MacroName]; + + // Check whether we know anything about this macro name or not. + llvm::StringMap >::iterator Known + = ASTFileMacros.find(MacroName); + if (Known == ASTFileMacros.end()) { + // 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? + continue; + } + + // If the macro was defined in one but undef'd in the other, we have a + // conflict. + if (Existing.second != Known->second.second) { + if (Diags) { + Diags->Report(diag::err_pch_macro_def_undef) + << MacroName << Known->second.second; + } + return true; + } + + // 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) + continue; + + // The macro bodies differ; complain. + if (Diags) { + Diags->Report(diag::err_pch_macro_def_conflict) + << MacroName << Known->second.first << Existing.first; + } + return true; + } + + // Check whether we're using predefines. + if (PPOpts.UsePredefines != ExistingPPOpts.UsePredefines) { + if (Diags) { + Diags->Report(diag::err_pch_undef) << ExistingPPOpts.UsePredefines; + } + return true; + } + + return false; +} + +bool PCHValidator::ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, + bool Complain) { + const PreprocessorOptions &ExistingPPOpts = PP.getPreprocessorOpts(); + + return checkPreprocessorOptions(PPOpts, ExistingPPOpts, + Complain? &Reader.Diags : 0); +} + namespace { struct EmptyStringRef { bool operator ()(StringRef r) const { return r.empty(); } @@ -3389,12 +3502,15 @@ namespace { class SimplePCHValidator : public ASTReaderListener { const LangOptions &ExistingLangOpts; const TargetOptions &ExistingTargetOpts; + const PreprocessorOptions &ExistingPPOpts; public: SimplePCHValidator(const LangOptions &ExistingLangOpts, - const TargetOptions &ExistingTargetOpts) + const TargetOptions &ExistingTargetOpts, + const PreprocessorOptions &ExistingPPOpts) : ExistingLangOpts(ExistingLangOpts), - ExistingTargetOpts(ExistingTargetOpts) + ExistingTargetOpts(ExistingTargetOpts), + ExistingPPOpts(ExistingPPOpts) { } @@ -3406,13 +3522,18 @@ namespace { bool Complain) { return checkTargetOptions(ExistingTargetOpts, TargetOpts, 0); } + virtual bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, + bool Complain) { + return checkPreprocessorOptions(ExistingPPOpts, PPOpts, 0); + } }; } bool ASTReader::isAcceptableASTFile(StringRef Filename, FileManager &FileMgr, const LangOptions &LangOpts, - const TargetOptions &TargetOpts) { + const TargetOptions &TargetOpts, + const PreprocessorOptions &PPOpts) { // Open the AST file. std::string ErrStr; OwningPtr Buffer; @@ -3436,7 +3557,7 @@ bool ASTReader::isAcceptableASTFile(StringRef Filename, return false; } - SimplePCHValidator Validator(LangOpts, TargetOpts); + SimplePCHValidator Validator(LangOpts, TargetOpts, PPOpts); RecordData Record; bool InControlBlock = false; while (!Stream.AtEndOfStream()) { @@ -3952,6 +4073,7 @@ bool ASTReader::ParsePreprocessorOptions(const RecordData &Record, PPOpts.MacroIncludes.push_back(ReadString(Record, Idx)); } + PPOpts.UsePredefines = Record[Idx++]; PPOpts.ImplicitPCHInclude = ReadString(Record, Idx); PPOpts.ImplicitPTHInclude = ReadString(Record, Idx); PPOpts.ObjCXXARCStandardLibrary = diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index 50e6763308..ea100864e3 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -1153,6 +1153,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context, for (unsigned I = 0, N = PPOpts.MacroIncludes.size(); I != N; ++I) AddString(PPOpts.MacroIncludes[I], Record); + Record.push_back(PPOpts.UsePredefines); AddString(PPOpts.ImplicitPCHInclude, Record); AddString(PPOpts.ImplicitPTHInclude, Record); Record.push_back(static_cast(PPOpts.ObjCXXARCStandardLibrary)); diff --git a/test/PCH/fuzzy-pch.c b/test/PCH/fuzzy-pch.c index 567575346c..7296d1dc89 100644 --- a/test/PCH/fuzzy-pch.c +++ b/test/PCH/fuzzy-pch.c @@ -1,8 +1,14 @@ // Test with pch. // RUN: %clang_cc1 -emit-pch -DFOO -o %t %S/variables.h // RUN: %clang_cc1 -DBAR=int -include-pch %t -fsyntax-only -pedantic %s -// RUN: %clang_cc1 -DFOO -DBAR=int -include-pch %t -Werror %s -// RUN: not %clang_cc1 -DFOO -DBAR=int -DX=5 -include-pch %t -Werror %s +// RUN: %clang_cc1 -DFOO -DBAR=int -include-pch %t %s +// RUN: not %clang_cc1 -DFOO=blah -DBAR=int -include-pch %t %s 2> %t.err +// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err +// RUN: not %clang_cc1 -UFOO -include-pch %t %s 2> %t.err +// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err + +// RUN: not %clang_cc1 -DFOO -undef -include-pch %t %s 2> %t.err +// RUN: FileCheck -check-prefix=CHECK-UNDEF %s < %t.err BAR bar = 17; @@ -17,3 +23,9 @@ BAR bar = 17; #ifndef BAR # error BAR was not defined #endif + +// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header ('1') and the command line ('blah') +// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd on the command line + +// CHECK-UNDEF: command line contains '-undef' but precompiled header was not built with it + diff --git a/test/PCH/pch-dir.c b/test/PCH/pch-dir.c index 73a7ba9003..ae841ce0a7 100644 --- a/test/PCH/pch-dir.c +++ b/test/PCH/pch-dir.c @@ -1,14 +1,19 @@ // RUN: mkdir -p %t.h.gch -// RUN: %clang -x c-header %S/pch-dir.h -o %t.h.gch/c.gch +// RUN: %clang -x c-header %S/pch-dir.h -DFOO=foo -o %t.h.gch/c.gch +// RUN: %clang -x c-header %S/pch-dir.h -DFOO=bar -o %t.h.gch/cbar.gch // RUN: %clang -x c++-header -std=c++98 %S/pch-dir.h -o %t.h.gch/cpp.gch -// RUN: %clang -include %t.h -fsyntax-only %s -Xclang -print-stats 2> %t.clog +// RUN: %clang -include %t.h -DFOO=foo -fsyntax-only %s -Xclang -print-stats 2> %t.clog // RUN: FileCheck -check-prefix=C %s < %t.clog +// RUN: %clang -include %t.h -DFOO=bar -DBAR=bar -fsyntax-only %s -Xclang -ast-print > %t.cbarlog +// RUN: FileCheck -check-prefix=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=CPP %s < %t.cpplog // RUN: not %clang -x c++ -std=c++11 -include %t.h -fsyntax-only %s 2> %t.cpp11log // RUN: FileCheck -check-prefix=CPP11 %s < %t.cpp11log +// CHECK-CBAR: int bar +int FOO; int get() { #ifdef __cplusplus