maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[NFC] Renaming PackStack to AlignPackStack 

This patch renames PackStack and related variable names to also contain align across Clang.
As it is right now, Clang already uses one stack to record the information from both #pragma
align and #pragma pack. Leaving it as PackStack is confusing, and could cause people to
ignore #pragma align when developing code that interacts with PackStack.

Differential Revision: https://reviews.llvm.org/D93901
This commit is contained in:
Xiangling Liao 2021-01-07 17:16:33 -05:00
parent 7b9f541c1e
commit e97071d795
9 changed files with 140 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
clang/include/clang/Sema/Sema.h
View File

@ -568,14 +568,14 @@ public:
// #pragma pack.
// Sentinel to represent when the stack is set to mac68k alignment.
static const unsigned kMac68kAlignmentSentinel = ~0U;
PragmaStack<unsigned> PackStack;
// The current #pragma pack values and locations at each #include.
struct PackIncludeState {
PragmaStack<unsigned> AlignPackStack;
// The current #pragma align/pack values and locations at each #include.
struct AlignPackIncludeState {
unsigned CurrentValue;
SourceLocation CurrentPragmaLocation;
bool HasNonDefaultValue, ShouldWarnOnInclude;
};
SmallVector<PackIncludeState, 8> PackIncludeStack;
SmallVector<AlignPackIncludeState, 8> AlignPackIncludeStack;
// Segment #pragmas.
PragmaStack<StringLiteral *> DataSegStack;
PragmaStack<StringLiteral *> BSSSegStack;
@ -9721,14 +9721,14 @@ public:
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);

4
clang/include/clang/Serialization/ASTBitCodes.h
View File

@ -681,8 +681,8 @@ public:
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,

12
clang/include/clang/Serialization/ASTReader.h
View File

@ -869,17 +869,17 @@ private:
llvm::SmallVector<FpPragmaStackEntry, 2> FpPragmaStack;
llvm::SmallVector<std::string, 2> FpPragmaStrings;
/// The pragma pack state.
Optional<unsigned> PragmaPackCurrentValue;
SourceLocation PragmaPackCurrentLocation;
struct PragmaPackStackEntry {
/// The pragma align/pack state.
Optional<unsigned> PragmaAlignPackCurrentValue;
SourceLocation PragmaAlignPackCurrentLocation;
struct PragmaAlignPackStackEntry {
unsigned Value;
SourceLocation Location;
SourceLocation PushLocation;
StringRef SlotLabel;
};
llvm::SmallVector<PragmaPackStackEntry, 2> PragmaPackStack;
llvm::SmallVector<std::string, 2> PragmaPackStrings;
llvm::SmallVector<PragmaAlignPackStackEntry, 2> PragmaAlignPackStack;
llvm::SmallVector<std::string, 2> PragmaAlignPackStrings;
/// The OpenCL extension settings.
OpenCLOptions OpenCLExtensions;

13
clang/lib/Sema/Sema.cpp
View File

@ -120,8 +120,9 @@ public:
}
IncludeStack.push_back(IncludeLoc);
S->DiagnoseNonDefaultPragmaPack(
Sema::PragmaPackDiagnoseKind::NonDefaultStateAtInclude, IncludeLoc);
S->DiagnoseNonDefaultPragmaAlignPack(
Sema::PragmaAlignPackDiagnoseKind::NonDefaultStateAtInclude,
IncludeLoc);
}
break;
}
@ -130,8 +131,8 @@ public:
if (llvm::timeTraceProfilerEnabled())
llvm::timeTraceProfilerEnd();
S->DiagnoseNonDefaultPragmaPack(
Sema::PragmaPackDiagnoseKind::ChangedStateAtExit,
S->DiagnoseNonDefaultPragmaAlignPack(
Sema::PragmaAlignPackDiagnoseKind::ChangedStateAtExit,
IncludeStack.pop_back_val());
}
break;
@ -157,7 +158,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
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 @@ void Sema::ActOnEndOfTranslationUnit() {
}
}
DiagnoseUnterminatedPragmaPack();
DiagnoseUnterminatedPragmaAlignPack();
DiagnoseUnterminatedPragmaAttribute();
// All delayed member exception specs should be checked or we end up accepting

92
clang/lib/Sema/SemaAttr.cpp
View File

@ -49,27 +49,28 @@ Sema::PragmaStackSentinelRAII::~PragmaStackSentinelRAII() {
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 @@ void Sema::ActOnPragmaOptionsAlign(PragmaOptionsAlignKind Kind,
// 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 @@ void Sema::ActOnPragmaOptionsAlign(PragmaOptionsAlignKind Kind,
break;
}
PackStack.Act(PragmaLoc, Action, StringRef(), Alignment);
AlignPackStack.Act(PragmaLoc, Action, StringRef(), Alignment);
}
void Sema::ActOnPragmaClangSection(SourceLocation PragmaLoc, PragmaClangSectionAction Action,
@ -317,7 +318,7 @@ void Sema::ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action,
// 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 @@ void Sema::ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action,
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");
}

50
clang/lib/Serialization/ASTReader.cpp
View File

@ -3742,25 +3742,25 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
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 @@ void ASTReader::UpdateSema() {
}
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) {

16
clang/lib/Serialization/ASTWriter.cpp
View File

@ -4151,24 +4151,24 @@ void ASTWriter::WriteMSPointersToMembersPragmaOptions(Sema &SemaRef) {
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.

11
clang/test/Sema/Inputs/pragma-align-pack1.h Normal file
View File

@ -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

19
clang/test/Sema/misleading-pragma-align-pack-diagnostics.c Normal file
View File

@ -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