diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -568,14 +568,14 @@ // #pragma pack. // Sentinel to represent when the stack is set to mac68k alignment. static const unsigned kMac68kAlignmentSentinel = ~0U; - PragmaStack PackStack; - // The current #pragma pack values and locations at each #include. - struct PackIncludeState { + PragmaStack AlignPackStack; + // The current #pragma align/pack values and locations at each #include. + struct AlignPackIncludeState { unsigned CurrentValue; SourceLocation CurrentPragmaLocation; bool HasNonDefaultValue, ShouldWarnOnInclude; }; - SmallVector PackIncludeStack; + SmallVector AlignPackIncludeStack; // Segment #pragmas. PragmaStack DataSegStack; PragmaStack BSSSegStack; @@ -9721,14 +9721,14 @@ void ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action, StringRef SlotLabel, Expr *Alignment); - enum class PragmaPackDiagnoseKind { + enum class PragmaAlignPackDiagnoseKind { NonDefaultStateAtInclude, ChangedStateAtExit }; - void DiagnoseNonDefaultPragmaPack(PragmaPackDiagnoseKind Kind, - SourceLocation IncludeLoc); - void DiagnoseUnterminatedPragmaPack(); + void DiagnoseNonDefaultPragmaAlignPack(PragmaAlignPackDiagnoseKind Kind, + SourceLocation IncludeLoc); + void DiagnoseUnterminatedPragmaAlignPack(); /// ActOnPragmaMSStruct - Called on well formed \#pragma ms_struct [on|off]. void ActOnPragmaMSStruct(PragmaMSStructKind Kind); diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -681,8 +681,8 @@ MODULAR_CODEGEN_DECLS = 60, - /// Record code for \#pragma pack options. - PACK_PRAGMA_OPTIONS = 61, + /// Record code for \#pragma align/pack options. + ALIGN_PACK_PRAGMA_OPTIONS = 61, /// The stack of open #ifs/#ifdefs recorded in a preamble. PP_CONDITIONAL_STACK = 62, 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 @@ -869,17 +869,17 @@ llvm::SmallVector FpPragmaStack; llvm::SmallVector FpPragmaStrings; - /// The pragma pack state. - Optional PragmaPackCurrentValue; - SourceLocation PragmaPackCurrentLocation; - struct PragmaPackStackEntry { + /// The pragma align/pack state. + Optional PragmaAlignPackCurrentValue; + SourceLocation PragmaAlignPackCurrentLocation; + struct PragmaAlignPackStackEntry { unsigned Value; SourceLocation Location; SourceLocation PushLocation; StringRef SlotLabel; }; - llvm::SmallVector PragmaPackStack; - llvm::SmallVector PragmaPackStrings; + llvm::SmallVector PragmaAlignPackStack; + llvm::SmallVector PragmaAlignPackStrings; /// The OpenCL extension settings. OpenCLOptions OpenCLExtensions; diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -120,8 +120,9 @@ } IncludeStack.push_back(IncludeLoc); - S->DiagnoseNonDefaultPragmaPack( - Sema::PragmaPackDiagnoseKind::NonDefaultStateAtInclude, IncludeLoc); + S->DiagnoseNonDefaultPragmaAlignPack( + Sema::PragmaAlignPackDiagnoseKind::NonDefaultStateAtInclude, + IncludeLoc); } break; } @@ -130,8 +131,8 @@ if (llvm::timeTraceProfilerEnabled()) llvm::timeTraceProfilerEnd(); - S->DiagnoseNonDefaultPragmaPack( - Sema::PragmaPackDiagnoseKind::ChangedStateAtExit, + S->DiagnoseNonDefaultPragmaAlignPack( + Sema::PragmaAlignPackDiagnoseKind::ChangedStateAtExit, IncludeStack.pop_back_val()); } break; @@ -157,7 +158,7 @@ OriginalLexicalContext(nullptr), MSStructPragmaOn(false), MSPointerToMemberRepresentationMethod( LangOpts.getMSPointerToMemberRepresentationMethod()), - VtorDispStack(LangOpts.getVtorDispMode()), PackStack(0), + VtorDispStack(LangOpts.getVtorDispMode()), AlignPackStack(0), DataSegStack(nullptr), BSSSegStack(nullptr), ConstSegStack(nullptr), CodeSegStack(nullptr), FpPragmaStack(FPOptionsOverride()), CurInitSeg(nullptr), VisContext(nullptr), @@ -1038,7 +1039,7 @@ } } - DiagnoseUnterminatedPragmaPack(); + DiagnoseUnterminatedPragmaAlignPack(); DiagnoseUnterminatedPragmaAttribute(); // All delayed member exception specs should be checked or we end up accepting diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -49,27 +49,28 @@ void Sema::AddAlignmentAttributesForRecord(RecordDecl *RD) { // If there is no pack value, we don't need any attributes. - if (!PackStack.CurrentValue) + if (!AlignPackStack.CurrentValue) return; // Otherwise, check to see if we need a max field alignment attribute. - if (unsigned Alignment = PackStack.CurrentValue) { + if (unsigned Alignment = AlignPackStack.CurrentValue) { if (Alignment == Sema::kMac68kAlignmentSentinel) RD->addAttr(AlignMac68kAttr::CreateImplicit(Context)); else RD->addAttr(MaxFieldAlignmentAttr::CreateImplicit(Context, Alignment * 8)); } - if (PackIncludeStack.empty()) + if (AlignPackIncludeStack.empty()) return; - // The #pragma pack affected a record in an included file, so Clang should - // warn when that pragma was written in a file that included the included - // file. - for (auto &PackedInclude : llvm::reverse(PackIncludeStack)) { - if (PackedInclude.CurrentPragmaLocation != PackStack.CurrentPragmaLocation) + // The #pragma align/pack affected a record in an included file, so Clang + // should warn when that the pragma was written in a file that included the + // included file. + for (auto &AlignPackedInclude : llvm::reverse(AlignPackIncludeStack)) { + if (AlignPackedInclude.CurrentPragmaLocation != + AlignPackStack.CurrentPragmaLocation) break; - if (PackedInclude.HasNonDefaultValue) - PackedInclude.ShouldWarnOnInclude = true; + if (AlignPackedInclude.HasNonDefaultValue) + AlignPackedInclude.ShouldWarnOnInclude = true; } } @@ -238,8 +239,8 @@ // Reset just pops the top of the stack, or resets the current alignment to // default. Action = Sema::PSK_Pop; - if (PackStack.Stack.empty()) { - if (PackStack.CurrentValue) { + if (AlignPackStack.Stack.empty()) { + if (AlignPackStack.CurrentValue) { Action = Sema::PSK_Reset; } else { Diag(PragmaLoc, diag::warn_pragma_options_align_reset_failed) @@ -250,7 +251,7 @@ break; } - PackStack.Act(PragmaLoc, Action, StringRef(), Alignment); + AlignPackStack.Act(PragmaLoc, Action, StringRef(), Alignment); } void Sema::ActOnPragmaClangSection(SourceLocation PragmaLoc, PragmaClangSectionAction Action, @@ -317,7 +318,7 @@ // Show the current alignment, making sure to show the right value // for the default. // FIXME: This should come from the target. - AlignmentVal = PackStack.CurrentValue; + AlignmentVal = AlignPackStack.CurrentValue; if (AlignmentVal == 0) AlignmentVal = 8; if (AlignmentVal == Sema::kMac68kAlignmentSentinel) @@ -330,61 +331,72 @@ if (Action & Sema::PSK_Pop) { if (Alignment && !SlotLabel.empty()) Diag(PragmaLoc, diag::warn_pragma_pack_pop_identifier_and_alignment); - if (PackStack.Stack.empty()) + if (AlignPackStack.Stack.empty()) Diag(PragmaLoc, diag::warn_pragma_pop_failed) << "pack" << "stack empty"; } - PackStack.Act(PragmaLoc, Action, SlotLabel, AlignmentVal); + AlignPackStack.Act(PragmaLoc, Action, SlotLabel, AlignmentVal); } -void Sema::DiagnoseNonDefaultPragmaPack(PragmaPackDiagnoseKind Kind, - SourceLocation IncludeLoc) { - if (Kind == PragmaPackDiagnoseKind::NonDefaultStateAtInclude) { - SourceLocation PrevLocation = PackStack.CurrentPragmaLocation; +void Sema::DiagnoseNonDefaultPragmaAlignPack(PragmaAlignPackDiagnoseKind Kind, + SourceLocation IncludeLoc) { + if (Kind == PragmaAlignPackDiagnoseKind::NonDefaultStateAtInclude) { + SourceLocation PrevLocation = AlignPackStack.CurrentPragmaLocation; // Warn about non-default alignment at #includes (without redundant // warnings for the same directive in nested includes). // The warning is delayed until the end of the file to avoid warnings // for files that don't have any records that are affected by the modified // alignment. bool HasNonDefaultValue = - PackStack.hasValue() && - (PackIncludeStack.empty() || - PackIncludeStack.back().CurrentPragmaLocation != PrevLocation); - PackIncludeStack.push_back( - {PackStack.CurrentValue, - PackStack.hasValue() ? PrevLocation : SourceLocation(), + AlignPackStack.hasValue() && + (AlignPackIncludeStack.empty() || + AlignPackIncludeStack.back().CurrentPragmaLocation != PrevLocation); + AlignPackIncludeStack.push_back( + {AlignPackStack.CurrentValue, + AlignPackStack.hasValue() ? PrevLocation : SourceLocation(), HasNonDefaultValue, /*ShouldWarnOnInclude*/ false}); return; } - assert(Kind == PragmaPackDiagnoseKind::ChangedStateAtExit && "invalid kind"); - PackIncludeState PrevPackState = PackIncludeStack.pop_back_val(); - if (PrevPackState.ShouldWarnOnInclude) { + assert(Kind == PragmaAlignPackDiagnoseKind::ChangedStateAtExit && + "invalid kind"); + AlignPackIncludeState PrevAlignPackState = + AlignPackIncludeStack.pop_back_val(); + // FIXME: AlignPackStack may contain both #pragma align and #pragma pack + // information, diagnostics below might not be accurate if we have mixed + // pragmas. + if (PrevAlignPackState.ShouldWarnOnInclude) { // Emit the delayed non-default alignment at #include warning. Diag(IncludeLoc, diag::warn_pragma_pack_non_default_at_include); - Diag(PrevPackState.CurrentPragmaLocation, diag::note_pragma_pack_here); + Diag(PrevAlignPackState.CurrentPragmaLocation, diag::note_pragma_pack_here); } // Warn about modified alignment after #includes. - if (PrevPackState.CurrentValue != PackStack.CurrentValue) { + if (PrevAlignPackState.CurrentValue != AlignPackStack.CurrentValue) { Diag(IncludeLoc, diag::warn_pragma_pack_modified_after_include); - Diag(PackStack.CurrentPragmaLocation, diag::note_pragma_pack_here); + Diag(AlignPackStack.CurrentPragmaLocation, diag::note_pragma_pack_here); } } -void Sema::DiagnoseUnterminatedPragmaPack() { - if (PackStack.Stack.empty()) +void Sema::DiagnoseUnterminatedPragmaAlignPack() { + if (AlignPackStack.Stack.empty()) return; bool IsInnermost = true; - for (const auto &StackSlot : llvm::reverse(PackStack.Stack)) { + + // FIXME: AlignPackStack may contain both #pragma align and #pragma pack + // information, diagnostics below might not be accurate if we have mixed + // pragmas. + for (const auto &StackSlot : llvm::reverse(AlignPackStack.Stack)) { Diag(StackSlot.PragmaPushLocation, diag::warn_pragma_pack_no_pop_eof); // The user might have already reset the alignment, so suggest replacing // the reset with a pop. - if (IsInnermost && PackStack.CurrentValue == PackStack.DefaultValue) { - auto DB = Diag(PackStack.CurrentPragmaLocation, + if (IsInnermost && + AlignPackStack.CurrentValue == AlignPackStack.DefaultValue) { + auto DB = Diag(AlignPackStack.CurrentPragmaLocation, diag::note_pragma_pack_pop_instead_reset); - SourceLocation FixItLoc = Lexer::findLocationAfterToken( - PackStack.CurrentPragmaLocation, tok::l_paren, SourceMgr, LangOpts, - /*SkipTrailing=*/false); + SourceLocation FixItLoc = + Lexer::findLocationAfterToken(AlignPackStack.CurrentPragmaLocation, + tok::l_paren, SourceMgr, LangOpts, + /*SkipTrailing=*/false); if (FixItLoc.isValid()) DB << FixItHint::CreateInsertion(FixItLoc, "pop"); } 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 @@ -3742,25 +3742,25 @@ ForceCUDAHostDeviceDepth = Record[0]; break; - case PACK_PRAGMA_OPTIONS: { + case ALIGN_PACK_PRAGMA_OPTIONS: { if (Record.size() < 3) { Error("invalid pragma pack record"); return Failure; } - PragmaPackCurrentValue = Record[0]; - PragmaPackCurrentLocation = ReadSourceLocation(F, Record[1]); + PragmaAlignPackCurrentValue = Record[0]; + PragmaAlignPackCurrentLocation = ReadSourceLocation(F, Record[1]); unsigned NumStackEntries = Record[2]; unsigned Idx = 3; // Reset the stack when importing a new module. - PragmaPackStack.clear(); + PragmaAlignPackStack.clear(); for (unsigned I = 0; I < NumStackEntries; ++I) { - PragmaPackStackEntry Entry; + PragmaAlignPackStackEntry 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); + PragmaAlignPackStrings.push_back(ReadString(Record, Idx)); + Entry.SlotLabel = PragmaAlignPackStrings.back(); + PragmaAlignPackStack.push_back(Entry); } break; } @@ -7875,32 +7875,36 @@ } SemaObj->ForceCUDAHostDeviceDepth = ForceCUDAHostDeviceDepth; - if (PragmaPackCurrentValue) { + if (PragmaAlignPackCurrentValue) { // The bottom of the stack might have a default value. It must be adjusted // to the current value to ensure that the packing state is preserved after // popping entries that were included/imported from a PCH/module. bool DropFirst = false; - if (!PragmaPackStack.empty() && - PragmaPackStack.front().Location.isInvalid()) { - assert(PragmaPackStack.front().Value == SemaObj->PackStack.DefaultValue && + if (!PragmaAlignPackStack.empty() && + PragmaAlignPackStack.front().Location.isInvalid()) { + assert(PragmaAlignPackStack.front().Value == + SemaObj->AlignPackStack.DefaultValue && "Expected a default alignment value"); - SemaObj->PackStack.Stack.emplace_back( - PragmaPackStack.front().SlotLabel, SemaObj->PackStack.CurrentValue, - SemaObj->PackStack.CurrentPragmaLocation, - PragmaPackStack.front().PushLocation); + SemaObj->AlignPackStack.Stack.emplace_back( + PragmaAlignPackStack.front().SlotLabel, + SemaObj->AlignPackStack.CurrentValue, + SemaObj->AlignPackStack.CurrentPragmaLocation, + PragmaAlignPackStack.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.PushLocation); - if (PragmaPackCurrentLocation.isInvalid()) { - assert(*PragmaPackCurrentValue == SemaObj->PackStack.DefaultValue && + llvm::makeArrayRef(PragmaAlignPackStack).drop_front(DropFirst ? 1 : 0)) + SemaObj->AlignPackStack.Stack.emplace_back( + Entry.SlotLabel, Entry.Value, Entry.Location, Entry.PushLocation); + if (PragmaAlignPackCurrentLocation.isInvalid()) { + assert(*PragmaAlignPackCurrentValue == + SemaObj->AlignPackStack.DefaultValue && "Expected a default alignment value"); // Keep the current values. } else { - SemaObj->PackStack.CurrentValue = *PragmaPackCurrentValue; - SemaObj->PackStack.CurrentPragmaLocation = PragmaPackCurrentLocation; + SemaObj->AlignPackStack.CurrentValue = *PragmaAlignPackCurrentValue; + SemaObj->AlignPackStack.CurrentPragmaLocation = + PragmaAlignPackCurrentLocation; } } if (FpPragmaCurrentValue) { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4151,24 +4151,24 @@ Stream.EmitRecord(POINTERS_TO_MEMBERS_PRAGMA_OPTIONS, Record); } -/// Write the state of 'pragma pack' at the end of the module. +/// Write the state of 'pragma align/pack' at the end of the module. void ASTWriter::WritePackPragmaOptions(Sema &SemaRef) { - // Don't serialize pragma pack state for modules, since it should only take - // effect on a per-submodule basis. + // Don't serialize pragma align/pack state for modules, since it should only + // take effect on a per-submodule basis. if (WritingModule) return; RecordData Record; - Record.push_back(SemaRef.PackStack.CurrentValue); - AddSourceLocation(SemaRef.PackStack.CurrentPragmaLocation, Record); - Record.push_back(SemaRef.PackStack.Stack.size()); - for (const auto &StackEntry : SemaRef.PackStack.Stack) { + Record.push_back(SemaRef.AlignPackStack.CurrentValue); + AddSourceLocation(SemaRef.AlignPackStack.CurrentPragmaLocation, Record); + Record.push_back(SemaRef.AlignPackStack.Stack.size()); + for (const auto &StackEntry : SemaRef.AlignPackStack.Stack) { Record.push_back(StackEntry.Value); AddSourceLocation(StackEntry.PragmaLocation, Record); AddSourceLocation(StackEntry.PragmaPushLocation, Record); AddString(StackEntry.StackSlotLabel, Record); } - Stream.EmitRecord(PACK_PRAGMA_OPTIONS, Record); + Stream.EmitRecord(ALIGN_PACK_PRAGMA_OPTIONS, Record); } /// Write the state of 'pragma float_control' at the end of the module. diff --git a/clang/test/Sema/Inputs/pragma-align-pack1.h b/clang/test/Sema/Inputs/pragma-align-pack1.h new file mode 100644 --- /dev/null +++ b/clang/test/Sema/Inputs/pragma-align-pack1.h @@ -0,0 +1,11 @@ +#ifdef ALIGN_SET_HERE +#pragma align = mac68k +// expected-note@-1 {{previous '#pragma pack' directive that modifies alignment is here}} +// expected-warning@-2 {{unterminated '#pragma pack (push, ...)' at end of file}} +#endif + +#ifdef RECORD_ALIGN +struct S { + int x; +}; +#endif diff --git a/clang/test/Sema/misleading-pragma-align-pack-diagnostics.c b/clang/test/Sema/misleading-pragma-align-pack-diagnostics.c new file mode 100644 --- /dev/null +++ b/clang/test/Sema/misleading-pragma-align-pack-diagnostics.c @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -Wpragma-pack-suspicious-include -I %S/Inputs -DALIGN_SET_HERE -verify %s +// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -Wpragma-pack-suspicious-include -I %S/Inputs -DRECORD_ALIGN -verify %s + +#ifdef ALIGN_SET_HERE +#pragma align = natural // expected-warning {{unterminated '#pragma pack (push, ...)' at end of file}} +// expected-warning@+9 {{the current #pragma pack alignment value is modified in the included file}} +#endif + +#ifdef RECORD_ALIGN +#pragma align = mac68k +// expected-note@-1 {{previous '#pragma pack' directive that modifies alignment is here}} +// expected-warning@+3 {{non-default #pragma pack value changes the alignment of struct or union members in the included file}} +#endif + +#include "pragma-align-pack1.h" + +#ifdef RECORD_ALIGN +#pragma align = reset +#endif