Generating available_externally vtables bugfix

[Prazek]

Summary

Bugfix revealed in r245264.

[rsmith]

This looks fine; can you add a testcase?

[Prazek]

Added tests, and changed the VTableComponent code which was wrong on so many levels. Sorry

[rjmccall]

LGTM.

[Prazek]

Have to accept revision to close it

include/clang/AST/VTableBuilder.h

public:
   CharUnits getOffsetToTop() const {
     assert(getKind() == CK_OffsetToTop && "Invalid component kind!");
 
     return getOffset();
   }
 
   const CXXRecordDecl *getRTTIDecl() const {
     assert(getKind() == CK_RTTI && "Invalid component kind!");
-
     return reinterpret_cast<CXXRecordDecl *>(getPointer());
   }
 
   const CXXMethodDecl *getFunctionDecl() const {
-    assert(getKind() == CK_FunctionPointer);
-
+    assert(isFunctionPointerKind() && "Invalid component kind!");
+    if (isDestructorKind())
+      return getDestructorDecl();
     return reinterpret_cast<CXXMethodDecl *>(getPointer());
   }
 
   const CXXDestructorDecl *getDestructorDecl() const {
-    assert((getKind() == CK_CompleteDtorPointer ||
-            getKind() == CK_DeletingDtorPointer) && "Invalid component kind!");
-
+    assert(isDestructorKind() && "Invalid component kind!");
     return reinterpret_cast<CXXDestructorDecl *>(getPointer());
   }
 
   const CXXMethodDecl *getUnusedFunctionDecl() const {
-    assert(getKind() == CK_UnusedFunctionPointer);
-
+    assert(getKind() == CK_UnusedFunctionPointer && "Invalid component kind!");
     return reinterpret_cast<CXXMethodDecl *>(getPointer());
   }
 
+  bool isDestructorKind() const { return isDestructorKind(getKind()); }
+
+  bool isUsedFunctionPointerKind() const {
+    return isUsedFunctionPointerKind(getKind());
+  }
+
+  bool isFunctionPointerKind() const {
+    return isFunctionPointerKind(getKind());
+  }
+
 private:
+  static bool isFunctionPointerKind(Kind ComponentKind) {
+    return isUsedFunctionPointerKind(ComponentKind) ||
+           ComponentKind == CK_UnusedFunctionPointer;
+  }
+  static bool isUsedFunctionPointerKind(Kind ComponentKind) {
+    return ComponentKind == CK_FunctionPointer ||
+           isDestructorKind(ComponentKind);
+  }
+  static bool isDestructorKind(Kind ComponentKind) {
+    return ComponentKind == CK_CompleteDtorPointer ||
+           ComponentKind == CK_DeletingDtorPointer;
+  }
+
   VTableComponent(Kind ComponentKind, CharUnits Offset) {
     assert((ComponentKind == CK_VCallOffset ||
             ComponentKind == CK_VBaseOffset ||
             ComponentKind == CK_OffsetToTop) && "Invalid component kind!");
     assert(Offset.getQuantity() < (1LL << 56) && "Offset is too big!");
     assert(Offset.getQuantity() >= -(1LL << 56) && "Offset is too small!");
 
     Value = (uint64_t(Offset.getQuantity()) << 3) | ComponentKind;
   }
 
   VTableComponent(Kind ComponentKind, uintptr_t Ptr) {
-    assert((ComponentKind == CK_RTTI ||
-            ComponentKind == CK_FunctionPointer ||
-            ComponentKind == CK_CompleteDtorPointer ||
-            ComponentKind == CK_DeletingDtorPointer ||
-            ComponentKind == CK_UnusedFunctionPointer) &&
-            "Invalid component kind!");
+    assert((ComponentKind == CK_RTTI || isFunctionPointerKind(ComponentKind)) &&
 
     assert((Ptr & 7) == 0 && "Pointer not sufficiently aligned!");
 
     Value = Ptr | ComponentKind;
   }
 
   CharUnits getOffset() const {
     assert((getKind() == CK_VCallOffset || getKind() == CK_VBaseOffset ||
             getKind() == CK_OffsetToTop) && "Invalid component kind!");
 
     return CharUnits::fromQuantity(Value >> 3);
   }
 
   uintptr_t getPointer() const {
-    assert((getKind() == CK_RTTI ||
-            getKind() == CK_FunctionPointer ||
-            getKind() == CK_CompleteDtorPointer ||
-            getKind() == CK_DeletingDtorPointer ||
-            getKind() == CK_UnusedFunctionPointer) &&
+    assert((getKind() == CK_RTTI || isFunctionPointerKind()) &&
            "Invalid component kind!");
 
     return static_cast<uintptr_t>(Value & ~7ULL);
   }
 
   explicit VTableComponent(uint64_t Value)
     : Value(Value) { }
 

lib/CodeGen/ItaniumCXXABI.cpp

public:
   RTTIUniquenessKind
   classifyRTTIUniqueness(QualType CanTy,
                          llvm::GlobalValue::LinkageTypes Linkage) const;
   friend class ItaniumRTTIBuilder;
 
   void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override;
 
  private:
-  /// Checks if function has any virtual inline function.
-  bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const {
+   bool hasAnyUsedVirtualInlineFunction(const CXXRecordDecl *RD) const {
     const auto &VtableLayout =
         CGM.getItaniumVTableContext().getVTableLayout(RD);
 
     for (const auto &VtableComponent : VtableLayout.vtable_components()) {
-      if (VtableComponent.getKind() !=
-          VTableComponent::Kind::CK_FunctionPointer)
+      if (!VtableComponent.isUsedFunctionPointerKind())
         continue;
 
-      const auto &Method = VtableComponent.getFunctionDecl();
+      const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
       if (Method->getCanonicalDecl()->isInlined())
         return true;
     }
     return false;
   }
 };
 
 class ARMCXXABI : public ItaniumCXXABI {
bool ItaniumCXXABI::canEmitAvailableExternallyVTable(
   // because kext mode does not permit devirtualization.
   if (CGM.getLangOpts().AppleKext)
     return false;
 
   // If we don't have any inline virtual functions,
   // then we are safe to emit available_externally copy of vtable.
   // FIXME we can still emit a copy of the vtable if we
   // can emit definition of the inline functions.
-  return !hasAnyVirtualInlineFunction(RD);
+  return !hasAnyUsedVirtualInlineFunction(RD);
 }
 static llvm::Value *performTypeAdjustment(CodeGenFunction &CGF,
                                           llvm::Value *Ptr,
                                           int64_t NonVirtualAdjustment,
                                           int64_t VirtualAdjustment,
                                           bool IsReturnAdjustment) {
   if (!NonVirtualAdjustment && !VirtualAdjustment)
     return Ptr;

test/CodeGenCXX/vtable-available-externally.cpp

 // RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm -o %t
 // RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -O2 -disable-llvm-optzns -emit-llvm -o %t.opt
 // RUN: FileCheck --check-prefix=CHECK-TEST1 %s < %t
 // RUN: FileCheck --check-prefix=CHECK-TEST2 %s < %t
 // RUN: FileCheck --check-prefix=CHECK-TEST5 %s < %t
 // RUN: FileCheck --check-prefix=CHECK-TEST8 %s < %t.opt
 // RUN: FileCheck --check-prefix=CHECK-TEST9 %s < %t.opt
 // RUN: FileCheck --check-prefix=CHECK-TEST10 %s < %t.opt
 // RUN: FileCheck --check-prefix=CHECK-TEST11 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST12 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST13 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST14 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt
 
 #include <typeinfo>
 
 // CHECK-TEST1: @_ZTVN5Test11AE = external unnamed_addr constant
 namespace Test1 {
 
 struct A {
   A();
void g(D& a) {
   a.key();
 }
 void g() {
   D d;
   d = f();
   g(d);
 }
 }  // Test 11
+
+namespace Test12 {
+
+// CHECK-TEST12: @_ZTVN6Test121AE = external unnamed_addr constant
+struct A {
+  virtual void foo();
+  virtual ~A() {}
+};
+// CHECK-TEST12: @_ZTVN6Test121BE = external unnamed_addr constant
+struct B : A {
+  void foo();
+};
+
+void g() {
+  A a;
+  a.foo();
+  B b;
+  b.foo();
+}
+}
+
+namespace Test13 {
+
+// CHECK-TEST13-DAG: @_ZTVN6Test131AE = available_externally unnamed_addr constant
+// CHECK-TEST13-DAG: @_ZTVN6Test131BE = external unnamed_addr constant
+struct A {
+  virtual ~A();
+};
+struct B : A {
+  virtual void f();
+  void operator delete(void *);
+  ~B() {}
+};
+
+void g() {
+  A *b = new B;
+}
+}
+
+namespace Test14 {
+
+// CHECK-TEST14: @_ZTVN6Test141AE = available_externally unnamed_addr constant
+struct A {
+  virtual void f();
+  void operator delete(void *);
+  ~A();
+};
+
+void g() {
+  A *b = new A;
+  delete b;
+}
+}
+
+namespace Test15 {
+// In this test D's vtable has two slots for function f(), but uses only one,
+// so the second slot is set to null.
+// CHECK-TEST15: @_ZTVN6Test151DE = available_externally unnamed_addr constant
+struct A { virtual void f() {} };
+struct B : virtual A {};
+struct C : virtual A {};
+struct D : B, C {
+  virtual void g();
+  void f();
+};
+
+void test() {
+  D * d = new D;
+  d->f();
+}
+}
+
+