r267870 - Revert "[MS] Improved implementation of MS stack pragmas (vtordisp, *_seg)"

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 28 04:52:22 PDT 2016


On Thu, Apr 28, 2016 at 7:32 AM, Denis Zobnin via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: dzobnin
> Date: Thu Apr 28 06:32:10 2016
> New Revision: 267870
>
> URL: http://llvm.org/viewvc/llvm-project?rev=267870&view=rev
> Log:
> Revert "[MS] Improved implementation of MS stack pragmas (vtordisp, *_seg)"
>
> This reverts commit r267866.

When reverting code, please state a reason why it is being reverted
(preferably with links to a broken bot if that's the reason).

~Aaron

>
> 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=267870&r1=267869&r2=267870&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Apr 28 06:32:10 2016
> @@ -327,21 +327,40 @@ 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 {
> @@ -358,84 +377,18 @@ public:
>               PragmaMsStackAction Action,
>               llvm::StringRef StackSlotLabel,
>               ValueType 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) {}
> -
> +    explicit PragmaStack(const ValueType &Value)
> +      : CurrentValue(Value) {}
>      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;
> @@ -1058,6 +1011,24 @@ 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:
> @@ -7695,8 +7666,7 @@ public:
>        SourceLocation PragmaLoc);
>
>    /// \brief Called on well formed \#pragma vtordisp().
> -  void ActOnPragmaMSVtorDisp(PragmaMsStackAction Action,
> -                             SourceLocation PragmaLoc,
> +  void ActOnPragmaMSVtorDisp(PragmaVtorDispKind Kind, 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=267870&r1=267869&r2=267870&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParsePragma.cpp (original)
> +++ cfe/trunk/lib/Parse/ParsePragma.cpp Thu Apr 28 06:32:10 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::PragmaMsStackAction Action =
> -      static_cast<Sema::PragmaMsStackAction>((Value >> 16) & 0xFFFF);
> +  Sema::PragmaVtorDispKind Kind =
> +      static_cast<Sema::PragmaVtorDispKind>((Value >> 16) & 0xFFFF);
>    MSVtorDispAttr::Mode Mode = MSVtorDispAttr::Mode(Value & 0xFFFF);
>    SourceLocation PragmaLoc = ConsumeToken(); // The annotation token.
> -  Actions.ActOnPragmaMSVtorDisp(Action, PragmaLoc, Mode);
> +  Actions.ActOnPragmaMSVtorDisp(Kind, PragmaLoc, Mode);
>  }
>
>  void Parser::HandlePragmaMSPragma() {
> @@ -1606,7 +1606,7 @@ void PragmaMSVtorDisp::HandlePragma(Prep
>    }
>    PP.Lex(Tok);
>
> -  Sema::PragmaMsStackAction Action = Sema::PSK_Set;
> +  Sema::PragmaVtorDispKind Kind = Sema::PVDK_Set;
>    const IdentifierInfo *II = Tok.getIdentifierInfo();
>    if (II) {
>      if (II->isStr("push")) {
> @@ -1617,24 +1617,24 @@ void PragmaMSVtorDisp::HandlePragma(Prep
>          return;
>        }
>        PP.Lex(Tok);
> -      Action = Sema::PSK_Push_Set;
> +      Kind = Sema::PVDK_Push;
>        // not push, could be on/off
>      } else if (II->isStr("pop")) {
>        // #pragma vtordisp(pop)
>        PP.Lex(Tok);
> -      Action = Sema::PSK_Pop;
> +      Kind = Sema::PVDK_Pop;
>      }
>      // not push or pop, could be on/off
>    } else {
>      if (Tok.is(tok::r_paren)) {
>        // #pragma vtordisp()
> -      Action = Sema::PSK_Reset;
> +      Kind = Sema::PVDK_Reset;
>      }
>    }
>
>
>    uint64_t Value = 0;
> -  if (Action & Sema::PSK_Push || Action & Sema::PSK_Set) {
> +  if (Kind == Sema::PVDK_Push || Kind == Sema::PVDK_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>((Action << 16) | (Value & 0xFFFF))));
> +      static_cast<uintptr_t>((Kind << 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=267870&r1=267869&r2=267870&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Thu Apr 28 06:32:10 2016
> @@ -1928,8 +1928,7 @@ 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::PragmaStackSentinelRAII
> -    PragmaStackSentinel(Actions, "InternalPragmaState", IsCXXMethod);
> +  Sema::VtorDispStackRAII SavedVtorDispStack(Actions, 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
> @@ -1973,8 +1972,7 @@ 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::PragmaStackSentinelRAII
> -    PragmaStackSentinel(Actions, "InternalPragmaState", IsCXXMethod);
> +  Sema::VtorDispStackRAII SavedVtorDispStack(Actions, 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=267870&r1=267869&r2=267870&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/Sema.cpp (original)
> +++ cfe/trunk/lib/Sema/Sema.cpp Thu Apr 28 06:32:10 2016
> @@ -82,7 +82,7 @@ Sema::Sema(Preprocessor &pp, ASTContext
>      PackContext(nullptr), MSStructPragmaOn(false),
>      MSPointerToMemberRepresentationMethod(
>          LangOpts.getMSPointerToMemberRepresentationMethod()),
> -    VtorDispStack(MSVtorDispAttr::Mode(LangOpts.VtorDispMode)),
> +    VtorDispModeStack(1, 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=267870&r1=267869&r2=267870&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaAttr.cpp Thu Apr 28 06:32:10 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 (VtorDispStack.CurrentValue != getLangOpts().VtorDispMode)
> +  if (VtorDispModeStack.back() != getLangOpts().VtorDispMode)
>      RD->addAttr(
> -        MSVtorDispAttr::CreateImplicit(Context, VtorDispStack.CurrentValue));
> +        MSVtorDispAttr::CreateImplicit(Context, VtorDispModeStack.back()));
>  }
>
>  void Sema::ActOnPragmaOptionsAlign(PragmaOptionsAlignKind Kind,
> @@ -292,13 +292,29 @@ void Sema::ActOnPragmaMSPointersToMember
>    ImplicitMSInheritanceAttrLoc = PragmaLoc;
>  }
>
> -void Sema::ActOnPragmaMSVtorDisp(PragmaMsStackAction Action,
> +void Sema::ActOnPragmaMSVtorDisp(PragmaVtorDispKind Kind,
>                                   SourceLocation PragmaLoc,
>                                   MSVtorDispAttr::Mode Mode) {
> -  if (Action & PSK_Pop && VtorDispStack.Stack.empty())
> -    Diag(PragmaLoc, diag::warn_pragma_pop_failed) << "vtordisp"
> -                                                  << "stack empty";
> -  VtorDispStack.Act(PragmaLoc, Action, StringRef(), 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;
> +  }
>  }
>
>  template<typename ValueType>
> @@ -307,7 +323,7 @@ void Sema::PragmaStack<ValueType>::Act(S
>                                         llvm::StringRef StackSlotLabel,
>                                         ValueType Value) {
>    if (Action == PSK_Reset) {
> -    CurrentValue = DefaultValue;
> +    CurrentValue = nullptr;
>      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=267870&r1=267869&r2=267870&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/sections.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/sections.cpp Thu Apr 28 06:32:10 2016
> @@ -31,31 +31,6 @@ 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.
> @@ -88,9 +63,6 @@ __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"
> @@ -98,4 +70,3 @@ __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=267870&r1=267869&r2=267870&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp (original)
> +++ cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp Thu Apr 28 06:32:10 2016
> @@ -22,8 +22,7 @@ struct B : virtual A { int b; };
>
>  // Test a reset.
>  #pragma vtordisp()
> -#pragma vtordisp(pop) // stack should NOT be affected by reset.
> -                      // Now stack contains '1'.
> +#pragma vtordisp(pop) // expected-warning {{#pragma vtordisp(pop, ...) failed: stack empty}}
>
>  #pragma vtordisp(      // expected-warning {{unknown action for '#pragma vtordisp' - ignored}}
>  #pragma vtordisp(asdf) // expected-warning {{unknown action for '#pragma vtordisp' - ignored}}
> @@ -43,7 +42,6 @@ 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


More information about the cfe-commits mailing list