r324695 - [modules] Fix incorrect diagnostic mapping computation when a module changes
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 8 17:15:13 PST 2018
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 at diag_flags.h:14 {{padding struct}}
+// expected-error at diag_flags.h:4 {{padding struct}}
#elif WARNING
-// expected-warning at diag_flags.h:14 {{padding struct}}
-#else
-// expected-no-diagnostics
+// expected-warning at 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 at +4 {{padding struct}}
+#elif LOCAL_WARNING
+// expected-warning at +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
More information about the cfe-commits
mailing list