Author: rsmith Date: Thu Feb 8 17:15:13 2018 New Revision: 324695 URL: http://llvm.org/viewvc/llvm-project?rev=324695&view=rev Log: [modules] Fix incorrect diagnostic mapping computation when a module changes diagnostic settings using _Pragma within a macro.
The AST writer had previously been assuming that all diagnostic state transitions would occur within a FileID corresponding to a file. When a diagnostic state change occured within a macro, it was unable to form a location for that state change and would instead corrupt the diagnostic state of the "root" node (and thus that of the main compilation). Also introduce a "#pragma clang __debug diag_mapping" debugging utility that I added to track this issue down. Modified: cfe/trunk/include/clang/Basic/Diagnostic.h cfe/trunk/include/clang/Basic/SourceManager.h cfe/trunk/lib/Basic/Diagnostic.cpp cfe/trunk/lib/Lex/Pragma.cpp cfe/trunk/lib/Serialization/ASTReader.cpp cfe/trunk/lib/Serialization/ASTWriter.cpp cfe/trunk/test/Modules/Inputs/diag_flags.h cfe/trunk/test/Modules/diag-flags.cpp Modified: cfe/trunk/include/clang/Basic/Diagnostic.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=324695&r1=324694&r2=324695&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/Diagnostic.h (original) +++ cfe/trunk/include/clang/Basic/Diagnostic.h Thu Feb 8 17:15:13 2018 @@ -262,6 +262,10 @@ private: CurDiagStateLoc = SourceLocation(); } + /// Produce a debugging dump of the diagnostic state. + LLVM_DUMP_METHOD void dump(SourceManager &SrcMgr, + StringRef DiagName = StringRef()) const; + /// Grab the most-recently-added state point. DiagState *getCurDiagState() const { return CurDiagState; } /// Get the location at which a diagnostic state was last added. @@ -409,6 +413,11 @@ public: DiagnosticsEngine &operator=(const DiagnosticsEngine &) = delete; ~DiagnosticsEngine(); + LLVM_DUMP_METHOD void dump() const { DiagStatesByLoc.dump(*SourceMgr); } + LLVM_DUMP_METHOD void dump(StringRef DiagName) const { + DiagStatesByLoc.dump(*SourceMgr, DiagName); + } + const IntrusiveRefCntPtr<DiagnosticIDs> &getDiagnosticIDs() const { return Diags; } Modified: cfe/trunk/include/clang/Basic/SourceManager.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=324695&r1=324694&r2=324695&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/SourceManager.h (original) +++ cfe/trunk/include/clang/Basic/SourceManager.h Thu Feb 8 17:15:13 2018 @@ -1137,6 +1137,18 @@ public: /// be used by clients. SourceLocation getImmediateSpellingLoc(SourceLocation Loc) const; + /// \brief Form a SourceLocation from a FileID and Offset pair. + SourceLocation getComposedLoc(FileID FID, unsigned Offset) const { + bool Invalid = false; + const SrcMgr::SLocEntry &Entry = getSLocEntry(FID, &Invalid); + if (Invalid) + return SourceLocation(); + + unsigned GlobalOffset = Entry.getOffset() + Offset; + return Entry.isFile() ? SourceLocation::getFileLoc(GlobalOffset) + : SourceLocation::getMacroLoc(GlobalOffset); + } + /// \brief Decompose the specified location into a raw FileID + Offset pair. /// /// The first element is the FileID, the second is the offset from the Modified: cfe/trunk/lib/Basic/Diagnostic.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=324695&r1=324694&r2=324695&view=diff ============================================================================== --- cfe/trunk/lib/Basic/Diagnostic.cpp (original) +++ cfe/trunk/lib/Basic/Diagnostic.cpp Thu Feb 8 17:15:13 2018 @@ -236,6 +236,96 @@ DiagnosticsEngine::DiagStateMap::getFile return &F; } +void DiagnosticsEngine::DiagStateMap::dump(SourceManager &SrcMgr, + StringRef DiagName) const { + llvm::errs() << "diagnostic state at "; + CurDiagStateLoc.dump(SrcMgr); + llvm::errs() << ": " << CurDiagState << "\n"; + + for (auto &F : Files) { + FileID ID = F.first; + File &File = F.second; + + bool PrintedOuterHeading = false; + auto PrintOuterHeading = [&] { + if (PrintedOuterHeading) return; + PrintedOuterHeading = true; + + llvm::errs() << "File " << &File << " <FileID " << ID.getHashValue() + << ">: " << SrcMgr.getBuffer(ID)->getBufferIdentifier(); + if (F.second.Parent) { + std::pair<FileID, unsigned> Decomp = + SrcMgr.getDecomposedIncludedLoc(ID); + assert(File.ParentOffset == Decomp.second); + llvm::errs() << " parent " << File.Parent << " <FileID " + << Decomp.first.getHashValue() << "> "; + SrcMgr.getLocForStartOfFile(Decomp.first) + .getLocWithOffset(Decomp.second) + .dump(SrcMgr); + } + if (File.HasLocalTransitions) + llvm::errs() << " has_local_transitions"; + llvm::errs() << "\n"; + }; + + if (DiagName.empty()) + PrintOuterHeading(); + + for (DiagStatePoint &Transition : File.StateTransitions) { + bool PrintedInnerHeading = false; + auto PrintInnerHeading = [&] { + if (PrintedInnerHeading) return; + PrintedInnerHeading = true; + + PrintOuterHeading(); + llvm::errs() << " "; + SrcMgr.getLocForStartOfFile(ID) + .getLocWithOffset(Transition.Offset) + .dump(SrcMgr); + llvm::errs() << ": state " << Transition.State << ":\n"; + }; + + if (DiagName.empty()) + PrintInnerHeading(); + + for (auto &Mapping : *Transition.State) { + StringRef Option = + DiagnosticIDs::getWarningOptionForDiag(Mapping.first); + if (!DiagName.empty() && DiagName != Option) + continue; + + PrintInnerHeading(); + llvm::errs() << " "; + if (Option.empty()) + llvm::errs() << "<unknown " << Mapping.first << ">"; + else + llvm::errs() << Option; + llvm::errs() << ": "; + + switch (Mapping.second.getSeverity()) { + case diag::Severity::Ignored: llvm::errs() << "ignored"; break; + case diag::Severity::Remark: llvm::errs() << "remark"; break; + case diag::Severity::Warning: llvm::errs() << "warning"; break; + case diag::Severity::Error: llvm::errs() << "error"; break; + case diag::Severity::Fatal: llvm::errs() << "fatal"; break; + } + + if (!Mapping.second.isUser()) + llvm::errs() << " default"; + if (Mapping.second.isPragma()) + llvm::errs() << " pragma"; + if (Mapping.second.hasNoWarningAsError()) + llvm::errs() << " no-error"; + if (Mapping.second.hasNoErrorAsFatal()) + llvm::errs() << " no-fatal"; + if (Mapping.second.wasUpgradedFromWarning()) + llvm::errs() << " overruled"; + llvm::errs() << "\n"; + } + } + } +} + void DiagnosticsEngine::PushDiagStatePoint(DiagState *State, SourceLocation Loc) { assert(Loc.isValid() && "Adding invalid loc point"); Modified: cfe/trunk/lib/Lex/Pragma.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Pragma.cpp?rev=324695&r1=324694&r2=324695&view=diff ============================================================================== --- cfe/trunk/lib/Lex/Pragma.cpp (original) +++ cfe/trunk/lib/Lex/Pragma.cpp Thu Feb 8 17:15:13 2018 @@ -1053,6 +1053,20 @@ struct PragmaDebugHandler : public Pragm PP.Diag(Identifier, diag::warn_pragma_debug_missing_argument) << II->getName(); } + } else if (II->isStr("diag_mapping")) { + Token DiagName; + PP.LexUnexpandedToken(DiagName); + if (DiagName.is(tok::eod)) + PP.getDiagnostics().dump(); + else if (DiagName.is(tok::string_literal) && !DiagName.hasUDSuffix()) { + StringLiteralParser Literal(DiagName, PP); + if (Literal.hadError) + return; + PP.getDiagnostics().dump(Literal.GetString()); + } else { + PP.Diag(DiagName, diag::warn_pragma_debug_missing_argument) + << II->getName(); + } } else if (II->isStr("llvm_fatal_error")) { llvm::report_fatal_error("#pragma clang __debug llvm_fatal_error"); } else if (II->isStr("llvm_unreachable")) { Modified: cfe/trunk/lib/Serialization/ASTReader.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=324695&r1=324694&r2=324695&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Feb 8 17:15:13 2018 @@ -5761,6 +5761,8 @@ void ASTReader::ReadPragmaDiagnosticMapp Initial.ExtBehavior = (diag::Severity)Flags; FirstState = ReadDiagState(Initial, SourceLocation(), true); + assert(F.OriginalSourceFileID.isValid()); + // Set up the root buffer of the module to start with the initial // diagnostic state of the module itself, to cover files that contain no // explicit transitions (for which we did not serialize anything). @@ -5781,6 +5783,7 @@ void ASTReader::ReadPragmaDiagnosticMapp "Invalid data, missing pragma diagnostic states"); SourceLocation Loc = ReadSourceLocation(F, Record[Idx++]); auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc); + assert(IDAndOffset.first.isValid() && "invalid FileID for transition"); assert(IDAndOffset.second == 0 && "not a start location for a FileID"); unsigned Transitions = Record[Idx++]; Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=324695&r1=324694&r2=324695&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Thu Feb 8 17:15:13 2018 @@ -3092,8 +3092,11 @@ void ASTWriter::WritePragmaDiagnosticMap !FileIDAndFile.second.HasLocalTransitions) continue; ++NumLocations; - AddSourceLocation(Diag.SourceMgr->getLocForStartOfFile(FileIDAndFile.first), - Record); + + SourceLocation Loc = Diag.SourceMgr->getComposedLoc(FileIDAndFile.first, 0); + assert(!Loc.isInvalid() && "start loc for valid FileID is invalid"); + AddSourceLocation(Loc, Record); + Record.push_back(FileIDAndFile.second.StateTransitions.size()); for (auto &StatePoint : FileIDAndFile.second.StateTransitions) { Record.push_back(StatePoint.Offset); Modified: cfe/trunk/test/Modules/Inputs/diag_flags.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/diag_flags.h?rev=324695&r1=324694&r2=324695&view=diff ============================================================================== --- cfe/trunk/test/Modules/Inputs/diag_flags.h (original) +++ cfe/trunk/test/Modules/Inputs/diag_flags.h Thu Feb 8 17:15:13 2018 @@ -1 +1,4 @@ +#define pragma _Pragma("clang diagnostic push") _Pragma("clang diagnostic error \"-Wpadded\"") _Pragma("clang diagnostic pop") +pragma + struct Padded { char x; int y; }; Modified: cfe/trunk/test/Modules/diag-flags.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/diag-flags.cpp?rev=324695&r1=324694&r2=324695&view=diff ============================================================================== --- cfe/trunk/test/Modules/diag-flags.cpp (original) +++ cfe/trunk/test/Modules/diag-flags.cpp Thu Feb 8 17:15:13 2018 @@ -3,24 +3,24 @@ // For an implicit module, all that matters are the warning flags in the user. // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -emit-module -fmodules-cache-path=%t -fmodule-name=diag_flags -x c++ %S/Inputs/module.map -fmodules-ts // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -Wpadded -// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DERROR -Wpadded -Werror -// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DERROR -Werror=padded +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -DLOCAL_WARNING -Wpadded +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DERROR -DLOCAL_ERROR -Wpadded -Werror +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DERROR -DLOCAL_ERROR -Werror=padded // // For an explicit module, all that matters are the warning flags in the module build. // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ %S/Inputs/module.map -fmodules-ts -o %t/nodiag.pcm -// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -fmodule-file=%t/nodiag.pcm -Wpadded -// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -fmodule-file=%t/nodiag.pcm -Werror -Wpadded +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -fmodule-file=%t/nodiag.pcm -Wpadded -DLOCAL_WARNING +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -fmodule-file=%t/nodiag.pcm -Werror -Wpadded -DLOCAL_ERROR // // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ %S/Inputs/module.map -fmodules-ts -o %t/warning.pcm -Wpadded // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -fmodule-file=%t/warning.pcm -// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -fmodule-file=%t/warning.pcm -Werror=padded +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -fmodule-file=%t/warning.pcm -Werror=padded -DLOCAL_ERROR // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -fmodule-file=%t/warning.pcm -Werror // // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ %S/Inputs/module.map -fmodules-ts -o %t/werror-no-error.pcm -Werror -Wpadded -Wno-error=padded // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -fmodule-file=%t/werror-no-error.pcm // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -fmodule-file=%t/werror-no-error.pcm -Wno-padded -// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -fmodule-file=%t/werror-no-error.pcm -Werror=padded +// RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DWARNING -fmodule-file=%t/werror-no-error.pcm -Werror=padded -DLOCAL_ERROR // // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -emit-module -fmodule-name=diag_flags -x c++ %S/Inputs/module.map -fmodules-ts -o %t/error.pcm -Werror=padded // RUN: %clang_cc1 -triple %itanium_abi_triple -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DERROR -fmodule-file=%t/error.pcm -Wno-padded @@ -35,10 +35,22 @@ import diag_flags; // Diagnostic flags from the module user make no difference to diagnostics // emitted within the module when using an explicitly-loaded module. #if ERROR -// expected-error@diag_flags.h:14 {{padding struct}} +// expected-error@diag_flags.h:4 {{padding struct}} #elif WARNING -// expected-warning@diag_flags.h:14 {{padding struct}} -#else -// expected-no-diagnostics +// expected-warning@diag_flags.h:4 {{padding struct}} #endif int arr[sizeof(Padded)]; + +// Diagnostic flags from the module make no difference to diagnostics emitted +// in the module user. +#if LOCAL_ERROR +// expected-error@+4 {{padding struct}} +#elif LOCAL_WARNING +// expected-warning@+2 {{padding struct}} +#endif +struct Padded2 { char x; int y; }; +int arr2[sizeof(Padded2)]; + +#if !ERROR && !WARNING && !LOCAL_ERROR && !LOCAL_WARNING +// expected-no-diagnostics +#endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits