From 95c5cdeb80aaf0819eba8ddd4421d04be5e271ff Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Tue, 18 Jul 2017 17:23:51 +0000 Subject: Add a warning for missing '#pragma pack (pop)' and suspicious uses of '#pragma pack' in included files This commit adds a new -Wpragma-pack warning. It warns in the following cases: - When a translation unit is missing terminating #pragma pack (pop) directives. - When entering an included file if the current alignment value as determined by '#pragma pack' directives is different from the default alignment value. - When leaving an included file that changed the state of the current alignment value. rdar://10184173 Differential Revision: https://reviews.llvm.org/D35484 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@308327 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Parse/ParsePragma.cpp | 9 +++++-- lib/Sema/Sema.cpp | 54 +++++++++++++++++++++++++++++++++++++++++ lib/Sema/SemaAttr.cpp | 37 +++++++++++++++++++++++++++- lib/Serialization/ASTReader.cpp | 6 +++-- lib/Serialization/ASTWriter.cpp | 1 + 5 files changed, 102 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/Parse/ParsePragma.cpp b/lib/Parse/ParsePragma.cpp index 262743756a..c5215ffcee 100644 --- a/lib/Parse/ParsePragma.cpp +++ b/lib/Parse/ParsePragma.cpp @@ -422,15 +422,20 @@ void Parser::HandlePragmaPack() { assert(Tok.is(tok::annot_pragma_pack)); PragmaPackInfo *Info = static_cast(Tok.getAnnotationValue()); - SourceLocation PragmaLoc = ConsumeAnnotationToken(); + SourceLocation PragmaLoc = Tok.getLocation(); ExprResult Alignment; if (Info->Alignment.is(tok::numeric_constant)) { Alignment = Actions.ActOnNumericConstant(Info->Alignment); - if (Alignment.isInvalid()) + if (Alignment.isInvalid()) { + ConsumeAnnotationToken(); return; + } } Actions.ActOnPragmaPack(PragmaLoc, Info->Action, Info->SlotLabel, Alignment.get()); + // Consume the token after processing the pragma to enable pragma-specific + // #include warnings. + ConsumeAnnotationToken(); } void Parser::HandlePragmaMSStruct() { diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index dc9f977d41..89a9a67133 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -70,6 +70,49 @@ void Sema::ActOnTranslationUnitScope(Scope *S) { PushDeclContext(S, Context.getTranslationUnitDecl()); } +namespace clang { +namespace sema { + +class SemaPPCallbacks : public PPCallbacks { + Sema *S = nullptr; + llvm::SmallVector IncludeStack; + +public: + void set(Sema &S) { this->S = &S; } + + void reset() { S = nullptr; } + + virtual void FileChanged(SourceLocation Loc, FileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID) override { + if (!S) + return; + switch (Reason) { + case EnterFile: { + SourceManager &SM = S->getSourceManager(); + SourceLocation IncludeLoc = SM.getIncludeLoc(SM.getFileID(Loc)); + if (IncludeLoc.isValid()) { + IncludeStack.push_back(IncludeLoc); + S->DiagnoseNonDefaultPragmaPack( + Sema::PragmaPackDiagnoseKind::NonDefaultStateAtInclude, IncludeLoc); + } + break; + } + case ExitFile: + if (!IncludeStack.empty()) + S->DiagnoseNonDefaultPragmaPack( + Sema::PragmaPackDiagnoseKind::ChangedStateAtExit, + IncludeStack.pop_back_val()); + break; + default: + break; + } + } +}; + +} // end namespace sema +} // end namespace clang + Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer, TranslationUnitKind TUKind, CodeCompleteConsumer *CodeCompleter) : ExternalSource(nullptr), isMultiplexExternalSource(false), @@ -122,6 +165,12 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer, // Initilization of data sharing attributes stack for OpenMP InitDataSharingAttributesStack(); + + std::unique_ptr Callbacks = + llvm::make_unique(); + SemaPPCallbackHandler = Callbacks.get(); + PP.addPPCallbacks(std::move(Callbacks)); + SemaPPCallbackHandler->set(*this); } void Sema::addImplicitTypedef(StringRef Name, QualType T) { @@ -306,6 +355,10 @@ Sema::~Sema() { // Destroys data sharing attributes stack for OpenMP DestroyDataSharingAttributesStack(); + // Detach from the PP callback handler which outlives Sema since it's owned + // by the preprocessor. + SemaPPCallbackHandler->reset(); + assert(DelayedTypos.empty() && "Uncorrected typos!"); } @@ -766,6 +819,7 @@ void Sema::ActOnEndOfTranslationUnit() { CheckDelayedMemberExceptionSpecs(); } + DiagnoseUnterminatedPragmaPack(); DiagnoseUnterminatedPragmaAttribute(); // All delayed member exception specs should be checked or we end up accepting diff --git a/lib/Sema/SemaAttr.cpp b/lib/Sema/SemaAttr.cpp index 8c13ead644..7193450220 100644 --- a/lib/Sema/SemaAttr.cpp +++ b/lib/Sema/SemaAttr.cpp @@ -202,6 +202,40 @@ void Sema::ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action, PackStack.Act(PragmaLoc, Action, SlotLabel, AlignmentVal); } +void Sema::DiagnoseNonDefaultPragmaPack(PragmaPackDiagnoseKind Kind, + SourceLocation IncludeLoc) { + if (Kind == PragmaPackDiagnoseKind::NonDefaultStateAtInclude) { + SourceLocation PrevLocation = PackStack.CurrentPragmaLocation; + // Warn about non-default alignment at #includes (without redundant + // warnings for the same directive in nested includes). + if (PackStack.hasValue() && + (PackIncludeStack.empty() || + PackIncludeStack.back().second != PrevLocation)) { + Diag(IncludeLoc, diag::warn_pragma_pack_non_default_at_include); + Diag(PrevLocation, diag::note_pragma_pack_here); + } + PackIncludeStack.push_back( + {PackStack.CurrentValue, + PackStack.hasValue() ? PrevLocation : SourceLocation()}); + return; + } + + assert(Kind == PragmaPackDiagnoseKind::ChangedStateAtExit && "invalid kind"); + unsigned PreviousValue = PackIncludeStack.pop_back_val().first; + // Warn about modified alignment after #includes. + if (PreviousValue != PackStack.CurrentValue) { + Diag(IncludeLoc, diag::warn_pragma_pack_modified_after_include); + Diag(PackStack.CurrentPragmaLocation, diag::note_pragma_pack_here); + } +} + +void Sema::DiagnoseUnterminatedPragmaPack() { + if (PackStack.Stack.empty()) + return; + for (const auto &StackSlot : llvm::reverse(PackStack.Stack)) + Diag(StackSlot.PragmaPushLocation, diag::warn_pragma_pack_no_pop_eof); +} + void Sema::ActOnPragmaMSStruct(PragmaMSStructKind Kind) { MSStructPragmaOn = (Kind == PMSST_ON); } @@ -249,7 +283,8 @@ void Sema::PragmaStack::Act(SourceLocation PragmaLocation, return; } if (Action & PSK_Push) - Stack.push_back(Slot(StackSlotLabel, CurrentValue, CurrentPragmaLocation)); + Stack.emplace_back(StackSlotLabel, CurrentValue, CurrentPragmaLocation, + PragmaLocation); else if (Action & PSK_Pop) { if (!StackSlotLabel.empty()) { // If we've got a label, try to find it and jump there. diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index 50be74f6bf..a384d051d6 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -3384,6 +3384,7 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { PragmaPackStackEntry Entry; Entry.Value = Record[Idx++]; Entry.Location = ReadSourceLocation(F, Record[Idx++]); + Entry.PushLocation = ReadSourceLocation(F, Record[Idx++]); PragmaPackStrings.push_back(ReadString(Record, Idx)); Entry.SlotLabel = PragmaPackStrings.back(); PragmaPackStack.push_back(Entry); @@ -7579,13 +7580,14 @@ void ASTReader::UpdateSema() { "Expected a default alignment value"); SemaObj->PackStack.Stack.emplace_back( PragmaPackStack.front().SlotLabel, SemaObj->PackStack.CurrentValue, - SemaObj->PackStack.CurrentPragmaLocation); + SemaObj->PackStack.CurrentPragmaLocation, + PragmaPackStack.front().PushLocation); DropFirst = true; } for (const auto &Entry : llvm::makeArrayRef(PragmaPackStack).drop_front(DropFirst ? 1 : 0)) SemaObj->PackStack.Stack.emplace_back(Entry.SlotLabel, Entry.Value, - Entry.Location); + Entry.Location, Entry.PushLocation); if (PragmaPackCurrentLocation.isInvalid()) { assert(*PragmaPackCurrentValue == SemaObj->PackStack.DefaultValue && "Expected a default alignment value"); diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index a875e627bd..9a739579b3 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -4295,6 +4295,7 @@ void ASTWriter::WritePackPragmaOptions(Sema &SemaRef) { for (const auto &StackEntry : SemaRef.PackStack.Stack) { Record.push_back(StackEntry.Value); AddSourceLocation(StackEntry.PragmaLocation, Record); + AddSourceLocation(StackEntry.PragmaPushLocation, Record); AddString(StackEntry.StackSlotLabel, Record); } Stream.EmitRecord(PACK_PRAGMA_OPTIONS, Record); -- cgit v1.2.3