[MS] Fix for bug 25013 - #pragma vtordisp is unknown inside functions.

[• d.zobnin.bugzilla]

Summary

This patch adds support of #pragma vtordisp inside functions in attempt to improve compatibility. Microsoft compiler appears to save the stack of vtordisp modes on entry of struct methods' bodies and restore it on exit (method-local vtordisp).

[rnk]

I went ahead and ran some of your test cases through MSVC, and I agree with your results. This is pretty interesting behavior. Thanks for looking into it! Looks good.

test/SemaCXX/pragma-vtordisp.cpp

35–37 ↗

(On Diff #39602)

With your changes, can we enable this test?

[• d.zobnin.bugzilla]

Only tests have been updated: enabled a test case for Sema, which pass with these changes, added corresponding test case for CodeGen.

[• d.zobnin.bugzilla]

test/SemaCXX/pragma-vtordisp.cpp

34–35 ↗

(On Diff #40380)

Yes, we can. Enabled this case, it passes with these changes.

cfe/trunk/include/clang/Sema/Sema.h

public:
     ~FPContractStateRAII() {
       S.FPFeatures.fp_contract = OldFPContractState;
     }
   private:
     Sema& S;
     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:
   Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
        TranslationUnitKind TUKind = TU_Complete,
        CodeCompleteConsumer *CompletionConsumer = nullptr);
   ~Sema();
 

cfe/trunk/lib/Parse/ParseDeclCXX.cpp

if (Tok.is(tok::annot_pragma_ms_pointers_to_members)) {
     return DeclGroupPtrTy();
   }
 
   if (Tok.is(tok::annot_pragma_ms_pragma)) {
     HandlePragmaMSPragma();
     return DeclGroupPtrTy();
   }
 
+  if (Tok.is(tok::annot_pragma_ms_vtordisp)) {
+    HandlePragmaMSVtorDisp();
+    return DeclGroupPtrTy();
+  }
+
   // If we see a namespace here, a close brace was missing somewhere.
   if (Tok.is(tok::kw_namespace)) {
     DiagnoseUnexpectedNamespace(cast<NamedDecl>(TagDecl));
     return DeclGroupPtrTy();
   }
 
   AccessSpecifier NewAS = getAccessSpecifierIfPresent();
   if (NewAS != AS_none) {

cfe/trunk/lib/Parse/ParseStmt.cpp

case tok::annot_pragma_ms_pointers_to_members:
     HandlePragmaMSPointersToMembers();
     return StmtEmpty();
 
   case tok::annot_pragma_ms_pragma:
     ProhibitAttributes(Attrs);
     HandlePragmaMSPragma();
     return StmtEmpty();
 
+  case tok::annot_pragma_ms_vtordisp:
+    ProhibitAttributes(Attrs);
+    HandlePragmaMSVtorDisp();
+    return StmtEmpty();
+
   case tok::annot_pragma_loop_hint:
     ProhibitAttributes(Attrs);
     return ParsePragmaLoopHint(Stmts, OnlyStatement, TrailingElseLoc, Attrs);
   }
 
   // If we reached this code, the statement must end in a semicolon.
   if (!TryConsumeToken(tok::semi) && !Res.isInvalid()) {
     // If the result was valid, then we do want to diagnose this.  Use
case tok::annot_pragma_fp_contract:
       HandlePragmaFPContract();
       break;
     case tok::annot_pragma_ms_pointers_to_members:
       HandlePragmaMSPointersToMembers();
       break;
     case tok::annot_pragma_ms_pragma:
       HandlePragmaMSPragma();
       break;
+    case tok::annot_pragma_ms_vtordisp:
+      HandlePragmaMSVtorDisp();
+      break;
     default:
       checkForPragmas = false;
       break;
     }
   }
 
 }
 
if (SkipFunctionBodies && (!Decl || Actions.canSkipFunctionBody(Decl)) &&
       trySkippingFunctionBody()) {
     BodyScope.Exit();
     return Actions.ActOnSkippedFunctionBody(Decl);
   }
 
   PrettyDeclStackTraceEntry CrashInfo(Actions, Decl, LBraceLoc,
                                       "parsing function body");
 
+  // 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);
+
   // 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
   // list and put it into a CompoundStmt for safe keeping.
   StmtResult FnBody(ParseCompoundStatementBody());
 
   // If the function body could not be parsed, make a bogus compoundstmt.
   if (FnBody.isInvalid()) {
     Sema::CompoundScopeRAII CompoundScope(Actions);
else
     Actions.ActOnDefaultCtorInitializers(Decl);
 
   if (SkipFunctionBodies && Actions.canSkipFunctionBody(Decl) &&
       trySkippingFunctionBody()) {
     BodyScope.Exit();
     return Actions.ActOnSkippedFunctionBody(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);
+
   SourceLocation LBraceLoc = Tok.getLocation();
   StmtResult FnBody(ParseCXXTryBlockCommon(TryLoc, /*FnTry*/true));
   // If we failed to parse the try-catch, we just give the function an empty
   // compound statement as the body.
   if (FnBody.isInvalid()) {
     Sema::CompoundScopeRAII CompoundScope(Actions);
     FnBody = Actions.ActOnCompoundStmt(LBraceLoc, LBraceLoc, None, false);
   }

cfe/trunk/test/Layout/ms-vtordisp-local.cpp

+// RUN: %clang_cc1 -fms-extensions -fexceptions -fcxx-exceptions -emit-llvm-only -triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>&1 | FileCheck %s
+
+struct Base {
+  virtual ~Base() {}
+  virtual void BaseFunc() {}
+};
+
+#pragma vtordisp(0)
+
+struct Container {
+  static void f() try {
+    #pragma vtordisp(2)
+    struct HasVtorDisp : virtual Base {
+      virtual ~HasVtorDisp() {}
+      virtual void Func() {}
+    };
+
+    int x[sizeof(HasVtorDisp)];
+
+    // HasVtorDisp: vtordisp because of pragma right before it.
+    //
+    // CHECK: *** Dumping AST Record Layout
+    // CHECK: *** Dumping AST Record Layout
+    // CHECK-NEXT:          0 | struct HasVtorDisp
+    // CHECK-NEXT:          0 |   (HasVtorDisp vftable pointer)
+    // CHECK-NEXT:          8 |   (HasVtorDisp vbtable pointer)
+    // CHECK-NEXT:         20 |   (vtordisp for vbase Base)
+    // CHECK-NEXT:         24 |   struct Base (virtual base)
+    // CHECK-NEXT:         24 |     (Base vftable pointer)
+    // CHECK-NEXT:            | [sizeof=32, align=8,
+    // CHECK-NEXT:            |  nvsize=16, nvalign=8]
+  } catch (...) {
+  }
+};
+
+struct NoVtorDisp1 : virtual Base {
+  virtual ~NoVtorDisp1() {}
+  virtual void Func() {}
+};
+
+int x1[sizeof(NoVtorDisp1)];
+
+// NoVtroDisp1: no vtordisp because of pragma disabling it.
+//
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:          0 | struct NoVtorDisp1
+// CHECK-NEXT:          0 |   (NoVtorDisp1 vftable pointer)
+// CHECK-NEXT:          8 |   (NoVtorDisp1 vbtable pointer)
+// CHECK-NEXT:         16 |   struct Base (virtual base)
+// CHECK-NEXT:         16 |     (Base vftable pointer)
+// CHECK-NEXT:            | [sizeof=24, align=8,
+// CHECK-NEXT:            |  nvsize=16, nvalign=8]
+
+struct Container2 {
+  static void f1() {
+    // Local pragma #1 - must be disabled on exit from f1().
+    #pragma vtordisp(push, 2)
+    struct HasVtorDisp1 : virtual Base {
+      virtual ~HasVtorDisp1() {}
+      virtual void Func() {}
+    };
+
+    int x2[sizeof(HasVtorDisp1)];
+
+    // HasVtorDisp1: vtordisp because of pragma right before it.
+    //
+    // CHECK: *** Dumping AST Record Layout
+    // CHECK-NEXT:          0 | struct HasVtorDisp1
+    // CHECK-NEXT:          0 |   (HasVtorDisp1 vftable pointer)
+    // CHECK-NEXT:          8 |   (HasVtorDisp1 vbtable pointer)
+    // CHECK-NEXT:         20 |   (vtordisp for vbase Base)
+    // CHECK-NEXT:         24 |   struct Base (virtual base)
+    // CHECK-NEXT:         24 |     (Base vftable pointer)
+    // CHECK-NEXT:            | [sizeof=32, align=8,
+    // CHECK-NEXT:            |  nvsize=16, nvalign=8]
+
+    struct InnerContainer {
+      static void g1() {
+        struct HasVtorDisp2 : virtual Base {
+          virtual ~HasVtorDisp2() {}
+          virtual void Func() {}
+        };
+
+        int x3[sizeof(HasVtorDisp2)];
+
+        // HasVtorDisp2: vtordisp because of vtordisp(2) in f1().
+        //
+        // CHECK: *** Dumping AST Record Layout
+        // CHECK-NEXT:          0 | struct HasVtorDisp2
+        // CHECK-NEXT:          0 |   (HasVtorDisp2 vftable pointer)
+        // CHECK-NEXT:          8 |   (HasVtorDisp2 vbtable pointer)
+        // CHECK-NEXT:         20 |   (vtordisp for vbase Base)
+        // CHECK-NEXT:         24 |   struct Base (virtual base)
+        // CHECK-NEXT:         24 |     (Base vftable pointer)
+        // CHECK-NEXT:            | [sizeof=32, align=8,
+        // CHECK-NEXT:            |  nvsize=16, nvalign=8]
+
+        // Local pragma #2 - must be disabled on exit from g1().
+        #pragma vtordisp(push, 0)
+        struct NoVtorDisp2 : virtual Base {
+          virtual ~NoVtorDisp2() {}
+          virtual void Func() {}
+        };
+
+        int x4[sizeof(NoVtorDisp2)];
+
+        // NoVtroDisp2: no vtordisp because of vtordisp(0) in g1().
+        //
+        // CHECK: *** Dumping AST Record Layout
+        // CHECK-NEXT:          0 | struct NoVtorDisp2
+        // CHECK-NEXT:          0 |   (NoVtorDisp2 vftable pointer)
+        // CHECK-NEXT:          8 |   (NoVtorDisp2 vbtable pointer)
+        // CHECK-NEXT:         16 |   struct Base (virtual base)
+        // CHECK-NEXT:         16 |     (Base vftable pointer)
+        // CHECK-NEXT:            | [sizeof=24, align=8,
+        // CHECK-NEXT:            |  nvsize=16, nvalign=8]
+      }
+
+      static void g2() {
+        struct HasVtorDisp3 : virtual Base {
+          virtual ~HasVtorDisp3() {}
+          virtual void Func() {}
+        };
+
+        int x5[sizeof(HasVtorDisp3)];
+
+        // HasVtorDisp3: vtordisp because of vtordisp(2) in f1(),
+        //               local vtordisp(0) in g1() is disabled.
+        //
+        // CHECK: *** Dumping AST Record Layout
+        // CHECK-NEXT:          0 | struct HasVtorDisp3
+        // CHECK-NEXT:          0 |   (HasVtorDisp3 vftable pointer)
+        // CHECK-NEXT:          8 |   (HasVtorDisp3 vbtable pointer)
+        // CHECK-NEXT:         20 |   (vtordisp for vbase Base)
+        // CHECK-NEXT:         24 |   struct Base (virtual base)
+        // CHECK-NEXT:         24 |     (Base vftable pointer)
+        // CHECK-NEXT:            | [sizeof=32, align=8,
+        // CHECK-NEXT:            |  nvsize=16, nvalign=8]
+      }
+    };
+
+    struct HasVtorDisp4 : virtual Base {
+      virtual ~HasVtorDisp4() {}
+      virtual void Func() {}
+    };
+
+    int x6[sizeof(HasVtorDisp4)];
+
+    // HasVtorDisp4: vtordisp because of vtordisp(2) in f1(),
+    //               local vtordisp(0) in g1() is disabled,
+    //               g2() has no pragmas - stack is not affected.
+    //
+    // CHECK: *** Dumping AST Record Layout
+    // CHECK-NEXT:          0 | struct HasVtorDisp4
+    // CHECK-NEXT:          0 |   (HasVtorDisp4 vftable pointer)
+    // CHECK-NEXT:          8 |   (HasVtorDisp4 vbtable pointer)
+    // CHECK-NEXT:         20 |   (vtordisp for vbase Base)
+    // CHECK-NEXT:         24 |   struct Base (virtual base)
+    // CHECK-NEXT:         24 |     (Base vftable pointer)
+    // CHECK-NEXT:            | [sizeof=32, align=8,
+    // CHECK-NEXT:            |  nvsize=16, nvalign=8]
+
+    InnerContainer::g1();
+    InnerContainer::g2();
+  }
+
+  static void f2() {
+    struct NoVtorDisp3 : virtual Base {
+      virtual ~NoVtorDisp3() {}
+      virtual void Func() {}
+    };
+
+    int x7[sizeof(NoVtorDisp3)];
+
+    // NoVtroDisp3: no vtordisp because of global pragma (0),
+    //              local vtordisp(2) is disabled on exit from f1().
+    //
+    // CHECK: *** Dumping AST Record Layout
+    // CHECK-NEXT:          0 | struct NoVtorDisp3
+    // CHECK-NEXT:          0 |   (NoVtorDisp3 vftable pointer)
+    // CHECK-NEXT:          8 |   (NoVtorDisp3 vbtable pointer)
+    // CHECK-NEXT:         16 |   struct Base (virtual base)
+    // CHECK-NEXT:         16 |     (Base vftable pointer)
+    // CHECK-NEXT:            | [sizeof=24, align=8,
+    // CHECK-NEXT:            |  nvsize=16, nvalign=8]
+  }
+};
+
+struct Container3 {
+  #pragma vtordisp(2)
+  struct HasVtorDisp5 : virtual Base {
+    virtual ~HasVtorDisp5() {}
+    virtual void Func() {}
+  };
+
+  int x8[sizeof(HasVtorDisp5)];
+
+  // HasVtorDisp5: vtordisp because of pragma right before it.
+  //
+  // CHECK: *** Dumping AST Record Layout
+  // CHECK-NEXT:          0 | struct Container3::HasVtorDisp5
+  // CHECK-NEXT:          0 |   (HasVtorDisp5 vftable pointer)
+  // CHECK-NEXT:          8 |   (HasVtorDisp5 vbtable pointer)
+  // CHECK-NEXT:         20 |   (vtordisp for vbase Base)
+  // CHECK-NEXT:         24 |   struct Base (virtual base)
+  // CHECK-NEXT:         24 |     (Base vftable pointer)
+  // CHECK-NEXT:            | [sizeof=32, align=8,
+  // CHECK-NEXT:            |  nvsize=16, nvalign=8]
+};
+
+int main() {
+  Container::f();
+  Container2::f1();
+  Container2::f2();
+  Container3 cont3;
+  return 0;
+};

cfe/trunk/test/SemaCXX/pragma-vtordisp.cpp

 #pragma vtordisp(      // expected-warning {{unknown action for '#pragma vtordisp' - ignored}}
 #pragma vtordisp(asdf) // expected-warning {{unknown action for '#pragma vtordisp' - ignored}}
 #pragma vtordisp(,)    // expected-warning {{unknown action for '#pragma vtordisp' - ignored}}
 #pragma vtordisp       // expected-warning {{missing '(' after '#pragma vtordisp' - ignoring}}
 #pragma vtordisp(3)    // expected-warning {{expected integer between 0 and 2 inclusive in '#pragma vtordisp' - ignored}}
 #pragma vtordisp(), stuff // expected-warning {{extra tokens}}
 
 struct C {
-// FIXME: Our implementation based on token insertion makes it impossible for
-// the pragma to appear everywhere we should support it.
-//#pragma vtordisp()
+#pragma vtordisp()
   struct D : virtual A {
   };
 };
+
+struct E {
+  virtual ~E();
+  virtual void f();
+};
+
+#pragma vtordisp(pop) // expected-warning {{#pragma vtordisp(pop, ...) failed: stack empty}}
+
+void g() {
+  #pragma vtordisp(push, 2)
+  struct F : virtual E {
+    virtual ~F();
+    virtual void f();
+  };
+}
+
+#pragma vtordisp(pop) // OK because of local vtordisp(2) in g().
+
+struct G {
+  void f() {
+    #pragma vtordisp(push, 2) // Method-local pragma - stack will be restored on exit.
+  }
+};
+
+// Stack is restored on exit from G::f(), nothing to pop.
+#pragma vtordisp(pop) // expected-warning {{#pragma vtordisp(pop, ...) failed: stack empty}}
+
+int g2()
+// FIXME: Our implementation based on token insertion makes it impossible for
+// the pragma to appear everywhere we should support it.
+// #pragma vtordisp()
+{
+  return 0;
+}