r267866 - [MS] Improved implementation of MS stack pragmas (vtordisp, *_seg)
Diana Picus via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 28 04:20:52 PDT 2016
Hi,
I don't know if you've noticed, but this commit broke a lot of builds -
e.g. http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15/builds/11865.
Regards,
Diana
On 28 April 2016 at 13:13, Denis Zobnin via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: dzobnin
> Date: Thu Apr 28 05:13:18 2016
> New Revision: 267866
>
> URL: http://llvm.org/viewvc/llvm-project?rev=267866&view=rev
> Log:
> [MS] Improved implementation of MS stack pragmas (vtordisp, *_seg)
>
> Rework implementation of several MS pragmas that use internal stack:
> vtordisp, {bss|code|const|data}_seg.
> This patch:
> 1. Makes #pragma vtordisp use PragmaStack class as *_seg pragmas do;
> 2. Fixes "#pragma vtordisp()" behavior: it shouldn't affect stack;
> 3. Saves/restores the stacks on enter/exit a C++ method body.
>
>
> Modified:
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Parse/ParsePragma.cpp
> cfe/trunk/lib/Parse/ParseStmt.cpp
> cfe/trunk/lib/Sema/Sema.cpp
> cfe/trunk/lib/Sema/SemaAttr.cpp
> cfe/trunk/test/CodeGenCXX/sections.cpp
> cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=267866&r1=267865&r2=267866&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Apr 28 05:13:18 2016
> @@ -327,40 +327,21 @@ public:
> LangOptions::PragmaMSPointersToMembersKind
> MSPointerToMemberRepresentationMethod;
>
> - enum PragmaVtorDispKind {
> - PVDK_Push, ///< #pragma vtordisp(push, mode)
> - PVDK_Set, ///< #pragma vtordisp(mode)
> - PVDK_Pop, ///< #pragma vtordisp(pop)
> - PVDK_Reset ///< #pragma vtordisp()
> - };
> -
> - enum PragmaMsStackAction {
> - PSK_Reset, // #pragma ()
> - PSK_Set, // #pragma ("name")
> - PSK_Push, // #pragma (push[, id])
> - PSK_Push_Set, // #pragma (push[, id], "name")
> - PSK_Pop, // #pragma (pop[, id])
> - PSK_Pop_Set, // #pragma (pop[, id], "name")
> - };
> -
> - /// \brief Whether to insert vtordisps prior to virtual bases in the
> Microsoft
> - /// C++ ABI. Possible values are 0, 1, and 2, which mean:
> - ///
> - /// 0: Suppress all vtordisps
> - /// 1: Insert vtordisps in the presence of vbase overrides and
> non-trivial
> - /// structors
> - /// 2: Always insert vtordisps to support RTTI on partially constructed
> - /// objects
> - ///
> - /// The stack always has at least one element in it.
> - SmallVector<MSVtorDispAttr::Mode, 2> VtorDispModeStack;
> -
> /// Stack of active SEH __finally scopes. Can be empty.
> SmallVector<Scope*, 2> CurrentSEHFinally;
>
> /// \brief Source location for newly created implicit MSInheritanceAttrs
> SourceLocation ImplicitMSInheritanceAttrLoc;
>
> + enum PragmaMsStackAction {
> + PSK_Reset = 0x0, // #pragma ()
> + PSK_Set = 0x1, // #pragma (value)
> + PSK_Push = 0x2, // #pragma (push[, id])
> + PSK_Pop = 0x4, // #pragma (pop[, id])
> + PSK_Push_Set = PSK_Push | PSK_Set, // #pragma (push[, id], value)
> + PSK_Pop_Set = PSK_Pop | PSK_Set, // #pragma (pop[, id], value)
> + };
> +
> template<typename ValueType>
> struct PragmaStack {
> struct Slot {
> @@ -377,18 +358,84 @@ public:
> PragmaMsStackAction Action,
> llvm::StringRef StackSlotLabel,
> ValueType Value);
> - explicit PragmaStack(const ValueType &Value)
> - : CurrentValue(Value) {}
> +
> + // MSVC seems to add artificial slots to #pragma stacks on entering a
> C++
> + // method body to restore the stacks on exit, so it works like this:
> + //
> + // struct S {
> + // #pragma <name>(push, InternalPragmaSlot,
> <current_pragma_value>)
> + // void Method {}
> + // #pragma <name>(pop, InternalPragmaSlot)
> + // };
> + //
> + // It works even with #pragma vtordisp, although MSVC doesn't support
> + // #pragma vtordisp(push [, id], n)
> + // syntax.
> + //
> + // Push / pop a named sentinel slot.
> + void SentinelAction(PragmaMsStackAction Action, StringRef Label) {
> + assert((Action == PSK_Push || Action == PSK_Pop) &&
> + "Can only push / pop #pragma stack sentinels!");
> + Act(CurrentPragmaLocation, Action, Label, CurrentValue);
> + }
> +
> + // Constructors.
> + explicit PragmaStack(const ValueType &Default)
> + : DefaultValue(Default), CurrentValue(Default) {}
> +
> SmallVector<Slot, 2> Stack;
> + ValueType DefaultValue; // Value used for PSK_Reset action.
> ValueType CurrentValue;
> SourceLocation CurrentPragmaLocation;
> };
> // FIXME: We should serialize / deserialize these if they occur in a
> PCH (but
> // we shouldn't do so if they're in a module).
> +
> + /// \brief Whether to insert vtordisps prior to virtual bases in the
> Microsoft
> + /// C++ ABI. Possible values are 0, 1, and 2, which mean:
> + ///
> + /// 0: Suppress all vtordisps
> + /// 1: Insert vtordisps in the presence of vbase overrides and
> non-trivial
> + /// structors
> + /// 2: Always insert vtordisps to support RTTI on partially constructed
> + /// objects
> + PragmaStack<MSVtorDispAttr::Mode> VtorDispStack;
> PragmaStack<StringLiteral *> DataSegStack;
> PragmaStack<StringLiteral *> BSSSegStack;
> PragmaStack<StringLiteral *> ConstSegStack;
> PragmaStack<StringLiteral *> CodeSegStack;
> + // TODO: Change implementation of #pragma pack to use PragmaStack<>
> approach.
> +
> + // RAII object to psuh / pop sentinel slots for all MS #pragma stacks.
> + // Actions should be performed only if we enter / exit a C++ method
> body.
> + class PragmaStackSentinelRAII {
> + public:
> + PragmaStackSentinelRAII(Sema &S, StringRef SlotLabel, bool ShouldAct)
> + : S(S), SlotLabel(SlotLabel), ShouldAct(ShouldAct) {
> + if (ShouldAct) {
> + S.VtorDispStack.SentinelAction(PSK_Push, SlotLabel);
> + S.DataSegStack.SentinelAction(PSK_Push, SlotLabel);
> + S.BSSSegStack.SentinelAction(PSK_Push, SlotLabel);
> + S.ConstSegStack.SentinelAction(PSK_Push, SlotLabel);
> + S.CodeSegStack.SentinelAction(PSK_Push, SlotLabel);
> + }
> + }
> +
> + ~PragmaStackSentinelRAII() {
> + if (ShouldAct) {
> + S.VtorDispStack.SentinelAction(PSK_Pop, SlotLabel);
> + S.DataSegStack.SentinelAction(PSK_Pop, SlotLabel);
> + S.BSSSegStack.SentinelAction(PSK_Pop, SlotLabel);
> + S.ConstSegStack.SentinelAction(PSK_Pop, SlotLabel);
> + S.CodeSegStack.SentinelAction(PSK_Pop, SlotLabel);
> + }
> + }
> +
> + private:
> + Sema &S;
> + StringRef SlotLabel;
> + bool ShouldAct;
> + };
>
> /// A mapping that describes the nullability we've seen in each header
> file.
> FileNullabilityMap NullabilityMap;
> @@ -1011,24 +1058,6 @@ public:
> bool OldFPContractState : 1;
> };
>
> - /// Records and restores the vtordisp state on entry/exit of C++ method
> body.
> - class VtorDispStackRAII {
> - public:
> - VtorDispStackRAII(Sema &S, bool ShouldSaveAndRestore)
> - : S(S), ShouldSaveAndRestore(ShouldSaveAndRestore),
> OldVtorDispStack() {
> - if (ShouldSaveAndRestore)
> - OldVtorDispStack = S.VtorDispModeStack;
> - }
> - ~VtorDispStackRAII() {
> - if (ShouldSaveAndRestore)
> - S.VtorDispModeStack = OldVtorDispStack;
> - }
> - private:
> - Sema &S;
> - bool ShouldSaveAndRestore;
> - SmallVector<MSVtorDispAttr::Mode, 2> OldVtorDispStack;
> - };
> -
> void addImplicitTypedef(StringRef Name, QualType T);
>
> public:
> @@ -7666,7 +7695,8 @@ public:
> SourceLocation PragmaLoc);
>
> /// \brief Called on well formed \#pragma vtordisp().
> - void ActOnPragmaMSVtorDisp(PragmaVtorDispKind Kind, SourceLocation
> PragmaLoc,
> + void ActOnPragmaMSVtorDisp(PragmaMsStackAction Action,
> + SourceLocation PragmaLoc,
> MSVtorDispAttr::Mode Value);
>
> enum PragmaSectionKind {
>
> Modified: cfe/trunk/lib/Parse/ParsePragma.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParsePragma.cpp?rev=267866&r1=267865&r2=267866&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParsePragma.cpp (original)
> +++ cfe/trunk/lib/Parse/ParsePragma.cpp Thu Apr 28 05:13:18 2016
> @@ -497,11 +497,11 @@ void Parser::HandlePragmaMSPointersToMem
> void Parser::HandlePragmaMSVtorDisp() {
> assert(Tok.is(tok::annot_pragma_ms_vtordisp));
> uintptr_t Value = reinterpret_cast<uintptr_t>(Tok.getAnnotationValue());
> - Sema::PragmaVtorDispKind Kind =
> - static_cast<Sema::PragmaVtorDispKind>((Value >> 16) & 0xFFFF);
> + Sema::PragmaMsStackAction Action =
> + static_cast<Sema::PragmaMsStackAction>((Value >> 16) & 0xFFFF);
> MSVtorDispAttr::Mode Mode = MSVtorDispAttr::Mode(Value & 0xFFFF);
> SourceLocation PragmaLoc = ConsumeToken(); // The annotation token.
> - Actions.ActOnPragmaMSVtorDisp(Kind, PragmaLoc, Mode);
> + Actions.ActOnPragmaMSVtorDisp(Action, PragmaLoc, Mode);
> }
>
> void Parser::HandlePragmaMSPragma() {
> @@ -1606,7 +1606,7 @@ void PragmaMSVtorDisp::HandlePragma(Prep
> }
> PP.Lex(Tok);
>
> - Sema::PragmaVtorDispKind Kind = Sema::PVDK_Set;
> + Sema::PragmaMsStackAction Action = Sema::PSK_Set;
> const IdentifierInfo *II = Tok.getIdentifierInfo();
> if (II) {
> if (II->isStr("push")) {
> @@ -1617,24 +1617,24 @@ void PragmaMSVtorDisp::HandlePragma(Prep
> return;
> }
> PP.Lex(Tok);
> - Kind = Sema::PVDK_Push;
> + Action = Sema::PSK_Push_Set;
> // not push, could be on/off
> } else if (II->isStr("pop")) {
> // #pragma vtordisp(pop)
> PP.Lex(Tok);
> - Kind = Sema::PVDK_Pop;
> + Action = Sema::PSK_Pop;
> }
> // not push or pop, could be on/off
> } else {
> if (Tok.is(tok::r_paren)) {
> // #pragma vtordisp()
> - Kind = Sema::PVDK_Reset;
> + Action = Sema::PSK_Reset;
> }
> }
>
>
> uint64_t Value = 0;
> - if (Kind == Sema::PVDK_Push || Kind == Sema::PVDK_Set) {
> + if (Action & Sema::PSK_Push || Action & Sema::PSK_Set) {
> const IdentifierInfo *II = Tok.getIdentifierInfo();
> if (II && II->isStr("off")) {
> PP.Lex(Tok);
> @@ -1676,7 +1676,7 @@ void PragmaMSVtorDisp::HandlePragma(Prep
> AnnotTok.setLocation(VtorDispLoc);
> AnnotTok.setAnnotationEndLoc(EndLoc);
> AnnotTok.setAnnotationValue(reinterpret_cast<void *>(
> - static_cast<uintptr_t>((Kind << 16) | (Value & 0xFFFF))));
> + static_cast<uintptr_t>((Action << 16) | (Value & 0xFFFF))));
> PP.EnterToken(AnnotTok);
> }
>
>
> Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=267866&r1=267865&r2=267866&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Thu Apr 28 05:13:18 2016
> @@ -1928,7 +1928,8 @@ Decl *Parser::ParseFunctionStatementBody
> // Save and reset current vtordisp stack if we have entered a C++
> method body.
> bool IsCXXMethod =
> getLangOpts().CPlusPlus && Decl && isa<CXXMethodDecl>(Decl);
> - Sema::VtorDispStackRAII SavedVtorDispStack(Actions, IsCXXMethod);
> + Sema::PragmaStackSentinelRAII
> + PragmaStackSentinel(Actions, "InternalPragmaState", IsCXXMethod);
>
> // Do not enter a scope for the brace, as the arguments are in the same
> scope
> // (the function body) as the body itself. Instead, just read the
> statement
> @@ -1972,7 +1973,8 @@ Decl *Parser::ParseFunctionTryBlock(Decl
> // Save and reset current vtordisp stack if we have entered a C++
> method body.
> bool IsCXXMethod =
> getLangOpts().CPlusPlus && Decl && isa<CXXMethodDecl>(Decl);
> - Sema::VtorDispStackRAII SavedVtorDispStack(Actions, IsCXXMethod);
> + Sema::PragmaStackSentinelRAII
> + PragmaStackSentinel(Actions, "InternalPragmaState", IsCXXMethod);
>
> SourceLocation LBraceLoc = Tok.getLocation();
> StmtResult FnBody(ParseCXXTryBlockCommon(TryLoc, /*FnTry*/true));
>
> Modified: cfe/trunk/lib/Sema/Sema.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=267866&r1=267865&r2=267866&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/Sema.cpp (original)
> +++ cfe/trunk/lib/Sema/Sema.cpp Thu Apr 28 05:13:18 2016
> @@ -82,7 +82,7 @@ Sema::Sema(Preprocessor &pp, ASTContext
> PackContext(nullptr), MSStructPragmaOn(false),
> MSPointerToMemberRepresentationMethod(
> LangOpts.getMSPointerToMemberRepresentationMethod()),
> - VtorDispModeStack(1, MSVtorDispAttr::Mode(LangOpts.VtorDispMode)),
> + VtorDispStack(MSVtorDispAttr::Mode(LangOpts.VtorDispMode)),
> DataSegStack(nullptr), BSSSegStack(nullptr), ConstSegStack(nullptr),
> CodeSegStack(nullptr), CurInitSeg(nullptr), VisContext(nullptr),
> IsBuildingRecoveryCallExpr(false),
>
> Modified: cfe/trunk/lib/Sema/SemaAttr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAttr.cpp?rev=267866&r1=267865&r2=267866&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaAttr.cpp Thu Apr 28 05:13:18 2016
> @@ -136,9 +136,9 @@ void Sema::AddMsStructLayoutForRecord(Re
> // FIXME: We should merge AddAlignmentAttributesForRecord with
> // AddMsStructLayoutForRecord into AddPragmaAttributesForRecord, which
> takes
> // all active pragmas and applies them as attributes to class
> definitions.
> - if (VtorDispModeStack.back() != getLangOpts().VtorDispMode)
> + if (VtorDispStack.CurrentValue != getLangOpts().VtorDispMode)
> RD->addAttr(
> - MSVtorDispAttr::CreateImplicit(Context,
> VtorDispModeStack.back()));
> + MSVtorDispAttr::CreateImplicit(Context,
> VtorDispStack.CurrentValue));
> }
>
> void Sema::ActOnPragmaOptionsAlign(PragmaOptionsAlignKind Kind,
> @@ -292,29 +292,13 @@ void Sema::ActOnPragmaMSPointersToMember
> ImplicitMSInheritanceAttrLoc = PragmaLoc;
> }
>
> -void Sema::ActOnPragmaMSVtorDisp(PragmaVtorDispKind Kind,
> +void Sema::ActOnPragmaMSVtorDisp(PragmaMsStackAction Action,
> SourceLocation PragmaLoc,
> MSVtorDispAttr::Mode Mode) {
> - switch (Kind) {
> - case PVDK_Set:
> - VtorDispModeStack.back() = Mode;
> - break;
> - case PVDK_Push:
> - VtorDispModeStack.push_back(Mode);
> - break;
> - case PVDK_Reset:
> - VtorDispModeStack.clear();
> -
> VtorDispModeStack.push_back(MSVtorDispAttr::Mode(LangOpts.VtorDispMode));
> - break;
> - case PVDK_Pop:
> - VtorDispModeStack.pop_back();
> - if (VtorDispModeStack.empty()) {
> - Diag(PragmaLoc, diag::warn_pragma_pop_failed) << "vtordisp"
> - << "stack empty";
> -
> VtorDispModeStack.push_back(MSVtorDispAttr::Mode(LangOpts.VtorDispMode));
> - }
> - break;
> - }
> + if (Action & PSK_Pop && VtorDispStack.Stack.empty())
> + Diag(PragmaLoc, diag::warn_pragma_pop_failed) << "vtordisp"
> + << "stack empty";
> + VtorDispStack.Act(PragmaLoc, Action, StringRef(), Mode);
> }
>
> template<typename ValueType>
> @@ -323,7 +307,7 @@ void Sema::PragmaStack<ValueType>::Act(S
> llvm::StringRef StackSlotLabel,
> ValueType Value) {
> if (Action == PSK_Reset) {
> - CurrentValue = nullptr;
> + CurrentValue = DefaultValue;
> return;
> }
> if (Action & PSK_Push)
>
> Modified: cfe/trunk/test/CodeGenCXX/sections.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/sections.cpp?rev=267866&r1=267865&r2=267866&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/sections.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/sections.cpp Thu Apr 28 05:13:18 2016
> @@ -31,6 +31,31 @@ int TEST1;
> #pragma bss_seg(pop)
> int TEST2;
>
> +
> +// Check "save-restore" of pragma stacks.
> +struct Outer {
> + void f() {
> + #pragma bss_seg(push, ".bss3")
> + #pragma code_seg(push, ".my_code1")
> + #pragma const_seg(push, ".my_const1")
> + #pragma data_seg(push, ".data3")
> + struct Inner {
> + void g() {
> + #pragma bss_seg(push, ".bss4")
> + #pragma code_seg(push, ".my_code2")
> + #pragma const_seg(push, ".my_const2")
> + #pragma data_seg(push, ".data4")
> + }
> + };
> + }
> +};
> +
> +void h2(void) {} // should be in ".my_code"
> +int TEST3; // should be in ".bss1"
> +int d2 = 1; // should be in ".data"
> +extern const int b2; // should be in ".my_const"
> +const int b2 = 1;
> +
> #pragma section("read_flag_section", read)
> // Even though they are not declared const, these become constant since
> they are
> // in a read-only section.
> @@ -63,6 +88,9 @@ __declspec(allocate("short_section")) sh
> //CHECK: @i = global i32 0
> //CHECK: @TEST1 = global i32 0
> //CHECK: @TEST2 = global i32 0, section ".bss1"
> +//CHECK: @TEST3 = global i32 0, section ".bss1"
> +//CHECK: @d2 = global i32 1, section ".data"
> +//CHECK: @b2 = constant i32 1, section ".my_const"
> //CHECK: @unreferenced = constant i32 0, section "read_flag_section"
> //CHECK: @referenced = constant i32 42, section "read_flag_section"
> //CHECK: @implicitly_read_write = global i32 42, section
> "no_section_attributes"
> @@ -70,3 +98,4 @@ __declspec(allocate("short_section")) sh
> //CHECK: @short_var = global i16 42, section "short_section"
> //CHECK: define void @g()
> //CHECK: define void @h() {{.*}} section ".my_code"
> +//CHECK: define void @h2() {{.*}} section ".my_code"
>
> Modified: cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp?rev=267866&r1=267865&r2=267866&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp (original)
> +++ cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp Thu Apr 28 05:13:18 2016
> @@ -22,7 +22,8 @@ struct B : virtual A { int b; };
>
> // Test a reset.
> #pragma vtordisp()
> -#pragma vtordisp(pop) // expected-warning {{#pragma vtordisp(pop, ...)
> failed: stack empty}}
> +#pragma vtordisp(pop) // stack should NOT be affected by reset.
> + // Now stack contains '1'.
>
> #pragma vtordisp( // expected-warning {{unknown action for '#pragma
> vtordisp' - ignored}}
> #pragma vtordisp(asdf) // expected-warning {{unknown action for '#pragma
> vtordisp' - ignored}}
> @@ -42,6 +43,7 @@ struct E {
> virtual void f();
> };
>
> +#pragma vtordisp(pop) // After this stack should be empty.
> #pragma vtordisp(pop) // expected-warning {{#pragma vtordisp(pop, ...)
> failed: stack empty}}
>
> void g() {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160428/94ebd87c/attachment-0001.html>
More information about the cfe-commits
mailing list