[cfe-commits] Vtordisp for MS ABI.

r4start r4start at gmail.com
Wed May 30 04:46:38 PDT 2012


On 30/05/2012 07:06, John McCall wrote:
> Is this the rule used by MSVC?  For example, does a vtordisp still get 
> emitted even if a ctor "obviously" doesn't call any virtual functions, 
> like if it's defined in the class definition and obviously empty? 
>  Please test both an empty ctor and an empty dtor.
>
I think MSVC doesn`t analyze ctor or dtor body. I wrote next test cases:
1. No vtordisp.

    class first {
    public:
       virtual void asdf(){}
    };
    class second : public virtual first{
    public :
       virtual void asdf(){}
    };

2. Vtordisp.

    class first {
    public:
       virtual void asdf(){}
    };
    class second : public virtual first{
    public :
       virtual void asdf(){}
       ~second(){}
    };

3. Vtordisp.

    class first {
    public:
       virtual void asdf(){}
    };
    class second : public virtual first{
    public :
       virtual void asdf(){}
       second(){}
    };

> Please also add testcases verifying that we do the right thing in 
> further-derived classes that provide ctors.  For example:
>   struct A { virtual void foo(); };
>   struct B : virtual A { virtual void foo(); }; // no vtordisp
>   struct Test1 : B { Test1(); };
>   struct Test2 : virtual B { Test2(); };
>   struct Test3 : B { Test3(); virtual void foo(); };
>   struct Test4 : virtual B { Test4(); virtual void foo(); };
>
Test3 and Test4 have vtordisps for virtual base A. At now clang(with my 
patch, I test this on my working copy) add vtordisp only in Test4 for B, 
not for A.
I add new patch. Problem was in undecidedVBases.erase(overriddenBase). 
In Test3 overriddenBase == B but undecidedVBases contains A.
Also I test

       struct A { virtual void foo(); };
       struct B : A { virtual void foo(); };
       struct Test1 : B { Test1(); };
       struct Test2 : virtual B { Test2(); };
       struct Test3 : B { Test3(); virtual void foo(); };
       struct Test4 : virtual B { Test4(); virtual void foo(); };

Only Test4 has vtordisp for B same as MSVC.

  - Dmitry Sokolov.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120530/421bed87/attachment.html>
-------------- next part --------------
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp	(revision 157602)
+++ lib/AST/RecordLayoutBuilder.cpp	(working copy)
@@ -1249,6 +1249,19 @@
   return false;
 }                                             
 
+static const CXXRecordDecl *getFirstDeclarationClass(const CXXMethodDecl *M) {
+  if (!M->size_overridden_methods())
+    return M->getParent();
+
+  for (CXXMethodDecl::method_iterator I = M->begin_overridden_methods(),
+       E = M->end_overridden_methods(); I != E; ++I) {
+    const CXXMethodDecl *MD = *I;
+    if (!MD->size_overridden_methods()) {
+      return MD->getParent();
+    }
+  }
+}
+
 /// In the Microsoft ABI, decide which of the virtual bases require a
 /// vtordisp field.
 void RecordLayoutBuilder::computeVtordisps(const CXXRecordDecl *RD,
@@ -1259,7 +1272,7 @@
   // Build up the set of virtual bases that we haven't decided yet.
   ClassSetTy undecidedVBases;
   for (CXXRecordDecl::base_class_const_iterator
-         I = RD->vbases_begin(), E = RD->vbases_end(); I != E; ++I) {
+       I = RD->vbases_begin(), E = RD->vbases_end(); I != E; ++I) {
     const CXXRecordDecl *vbase = I->getType()->getAsCXXRecordDecl();
     undecidedVBases.insert(vbase);
   }
@@ -1319,7 +1332,6 @@
     for (CXXMethodDecl::method_iterator I = M->begin_overridden_methods(),
           E = M->end_overridden_methods(); I != E; ++I) {
       const CXXMethodDecl *overriddenMethod = (*I);
-
       // Ignore methods that override methods from vbases that require
       // require vtordisps.
       if (overridesMethodRequiringVtorDisp(Context, overriddenMethod))
@@ -1327,7 +1339,15 @@
 
       // As an optimization, check immediately whether we're overriding
       // something from the undecided set.
-      const CXXRecordDecl *overriddenBase = overriddenMethod->getParent();
+      const CXXRecordDecl *overriddenBase = 
+        getFirstDeclarationClass(overriddenMethod);
+      
+      // If in code wasn`t declared ctor or dtor then we don`t need vtordisp.
+      if (!RD->hasUserDeclaredConstructor() && 
+          !RD->hasUserDeclaredDestructor()) {
+        continue;
+      }
+
       if (undecidedVBases.erase(overriddenBase)) {
         vtordispVBases.insert(overriddenBase);
         if (undecidedVBases.empty()) return;
Index: test/Sema/ms_class_layout.cpp
===================================================================
--- test/Sema/ms_class_layout.cpp	(revision 157602)
+++ test/Sema/ms_class_layout.cpp	(working copy)
@@ -505,4 +505,82 @@
 // CHECK-NEXT:  16 |       (A vftable pointer)
 // CHECK-NEXT:  sizeof=20, dsize=20, align=4
 // CHECK-NEXT:  nvsize=4, nvalign=4
+
+
+  struct first {
+    virtual void asdf() {}
+    virtual void g(){}
+	};
+	
+  struct second : virtual first {
+    int q;
+	  virtual void asdf() { q = 90; }
+	  virtual void g(){ q = 12; }
+  };
+	
+  struct third : virtual second {};
+	void test2() { third *t; }
+	
+// CHECK:        0 | struct test1::third
+// CHECK-NEXT:   0 |   (third vbtable pointer)
+// CHECK-NEXT:   4 |   struct test1::first (virtual base)
+// CHECK-NEXT:   4 |     (first vftable pointer)
+// CHECK-NEXT:   8 |   struct test1::second (virtual base)
+// CHECK-NEXT:   8 |     (second vbtable pointer)
+// CHECK-NEXT:  12 |     int q
+// CHECK-NEXT:  sizeof=16, dsize=16, align=4
+// CHECK-NEXT:  nvsize=4, nvalign=4
+
 }
+
+namespace test2 {
+  struct A { virtual void foo(); };
+  struct B : virtual A { virtual void foo(); };
+  struct Test1 : B { Test1(); };
+  struct Test2 : virtual B { Test2(); };
+  struct Test3 : B { Test3(); virtual void foo(); };
+  struct Test4 : virtual B { Test4(); virtual void foo(); };
+  
+  void test() { 
+    Test1 *t;
+    Test2 *t2;
+    Test3 *t3;
+    Test4 *t4;
+  }
+  
+// CHECK:        0 | struct test2::Test1
+// CHECK-NEXT:   0 |   struct test2::B (base)
+// CHECK-NEXT:   0 |     (B vbtable pointer)
+// CHECK-NEXT:   4 |   struct test2::A (virtual base)
+// CHECK-NEXT:   4 |     (A vftable pointer)
+// CHECK-NEXT:  sizeof=8, dsize=8, align=4
+// CHECK-NEXT:  nvsize=4, nvalign=4
+  
+// CHECK:        0 | struct test2::Test2
+// CHECK-NEXT:   0 |   (Test2 vbtable pointer)
+// CHECK-NEXT:   4 |   struct test2::A (virtual base)
+// CHECK-NEXT:   4 |     (A vftable pointer)
+// CHECK-NEXT:   8 |   struct test2::B (virtual base)
+// CHECK-NEXT:   8 |     (B vbtable pointer)
+// CHECK-NEXT:  sizeof=12, dsize=12, align=4
+// CHECK-NEXT:  nvsize=4, nvalign=4
+
+// CHECK:        0 | struct test2::Test3
+// CHECK-NEXT:   0 |   struct test2::B (base)
+// CHECK-NEXT:   0 |     (B vbtable pointer)
+// CHECK-NEXT:   4 |   (vtordisp for vbase A)
+// CHECK-NEXT:   8 |   struct test2::A (virtual base)
+// CHECK-NEXT:   8 |     (A vftable pointer)
+// CHECK-NEXT:  sizeof=12, dsize=12, align=4
+// CHECK-NEXT:  nvsize=4, nvalign=4
+  
+// CHECK:        0 | struct test2::Test4
+// CHECK-NEXT:   0 |   (Test4 vbtable pointer)
+// CHECK-NEXT:   4 |   (vtordisp for vbase A)
+// CHECK-NEXT:   8 |   struct test2::A (virtual base)
+// CHECK-NEXT:   8 |     (A vftable pointer)
+// CHECK-NEXT:  12 |   struct test2::B (virtual base)
+// CHECK-NEXT:  12 |     (B vbtable pointer)
+// CHECK-NEXT:  sizeof=16, dsize=16, align=4
+// CHECK-NEXT:  nvsize=4, nvalign=4
+}


More information about the cfe-commits mailing list