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

Denis Zobnin via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 29 04:27:01 PDT 2016


Author: dzobnin
Date: Fri Apr 29 06:27:00 2016
New Revision: 268029

URL: http://llvm.org/viewvc/llvm-project?rev=268029&view=rev
Log:
Recommit "[MS] Improved implementation of stack pragmas (vtordisp, *_seg)"

Slightly updated version, double-checked build and tests.
Improve implementation of MS pragmas that use stack + compatibility fixes.
This patch:
  1. Changes implementation of #pragma vtordisp to use PragmaStack class
     that other stack pragmas use;
  2. Fixes "#pragma vtordisp()" behavior - it shouldn't affect the stack;
  3. Supports "save-restore" of pragma stacks on enter / exit a C++ method
     body, as MSVC does.

TODO:
  1. Change implementation of #pragma pack to use the same approach;
  2. Introduce diagnostics on popping named stack slots, as MSVC does.

Reviewers:
  rnk, thakis

Differential revision: http://reviews.llvm.org/D19361

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=268029&r1=268028&r2=268029&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Apr 29 06:27:00 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,66 @@ 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 push / 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);
+    ~PragmaStackSentinelRAII();
+
+  private:
+    Sema &S;
+    StringRef SlotLabel;
+    bool ShouldAct;
+  };
 
   /// A mapping that describes the nullability we've seen in each header file.
   FileNullabilityMap NullabilityMap;
@@ -1011,24 +1040,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:
@@ -7677,7 +7688,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=268029&r1=268028&r2=268029&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParsePragma.cpp (original)
+++ cfe/trunk/lib/Parse/ParsePragma.cpp Fri Apr 29 06:27:00 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=268029&r1=268028&r2=268029&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Fri Apr 29 06:27:00 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=268029&r1=268028&r2=268029&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Fri Apr 29 06:27:00 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=268029&r1=268028&r2=268029&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaAttr.cpp Fri Apr 29 06:27:00 2016
@@ -105,6 +105,28 @@ bool PragmaPackStack::pop(IdentifierInfo
   return false;
 }
 
+Sema::PragmaStackSentinelRAII::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);
+  }
+}
+
+Sema::PragmaStackSentinelRAII::~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);
+  }
+}
 
 /// FreePackedContext - Deallocate and null out PackContext.
 void Sema::FreePackedContext() {
@@ -136,9 +158,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 +314,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 +329,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=268029&r1=268028&r2=268029&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/sections.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/sections.cpp Fri Apr 29 06:27:00 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=268029&r1=268028&r2=268029&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp (original)
+++ cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp Fri Apr 29 06:27:00 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() {




More information about the cfe-commits mailing list