r347692 - Don't speculatively emit VTTs for classes unless we are able to correctly emit references to all the functions they will (directly or indirectly) reference.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Nov 27 11:33:49 2018
New Revision: 347692

URL: http://llvm.org/viewvc/llvm-project?rev=347692&view=rev
Log:
Don't speculatively emit VTTs for classes unless we are able to correctly emit references to all the functions they will (directly or indirectly) reference.

Summary:
This fixes a miscompile where we'd emit a VTT for a class that ends up
referencing an inline virtual member function that we can't actually
emit a body for (because we never instantiated it in the current TU),
which in a corner case of a corner case can lead to link errors.

Reviewers: rjmccall

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D54768

Added:
    cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp
Modified:
    cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=347692&r1=347691&r2=347692&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Tue Nov 27 11:33:49 2018
@@ -287,6 +287,7 @@ public:
   void emitVirtualInheritanceTables(const CXXRecordDecl *RD) override;
 
   bool canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const override;
+  bool canSpeculativelyEmitVTableAsBaseClass(const CXXRecordDecl *RD) const;
 
   void setThunkLinkage(llvm::Function *Thunk, bool ForVTable, GlobalDecl GD,
                        bool ReturnAdjustment) override {
@@ -1777,7 +1778,8 @@ void ItaniumCXXABI::emitVirtualInheritan
   VTables.EmitVTTDefinition(VTT, CGM.getVTableLinkage(RD), RD);
 }
 
-bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const {
+bool ItaniumCXXABI::canSpeculativelyEmitVTableAsBaseClass(
+    const CXXRecordDecl *RD) const {
   // We don't emit available_externally vtables if we are in -fapple-kext mode
   // because kext mode does not permit devirtualization.
   if (CGM.getLangOpts().AppleKext)
@@ -1795,7 +1797,43 @@ bool ItaniumCXXABI::canSpeculativelyEmit
   // to emit an 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 !hasAnyUnusedVirtualInlineFunction(RD);
+  if (hasAnyUnusedVirtualInlineFunction(RD))
+    return false;
+
+  // For a class with virtual bases, we must also be able to speculatively
+  // emit the VTT, because CodeGen doesn't have separate notions of "can emit
+  // the vtable" and "can emit the VTT". For a base subobject, this means we
+  // need to be able to emit non-virtual base vtables.
+  if (RD->getNumVBases()) {
+    for (const auto &B : RD->bases()) {
+      auto *BRD = B.getType()->getAsCXXRecordDecl();
+      assert(BRD && "no class for base specifier");
+      if (B.isVirtual() || !BRD->isDynamicClass())
+        continue;
+      if (!canSpeculativelyEmitVTableAsBaseClass(BRD))
+        return false;
+    }
+  }
+
+  return true;
+}
+
+bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const {
+  if (!canSpeculativelyEmitVTableAsBaseClass(RD))
+    return false;
+
+  // For a complete-object vtable (or more specifically, for the VTT), we need
+  // to be able to speculatively emit the vtables of all dynamic virtual bases.
+  for (const auto &B : RD->vbases()) {
+    auto *BRD = B.getType()->getAsCXXRecordDecl();
+    assert(BRD && "no class for base specifier");
+    if (!BRD->isDynamicClass())
+      continue;
+    if (!canSpeculativelyEmitVTableAsBaseClass(BRD))
+      return false;
+  }
+
+  return true;
 }
 static llvm::Value *performTypeAdjustment(CodeGenFunction &CGF,
                                           Address InitialPtr,

Added: cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp?rev=347692&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp Tue Nov 27 11:33:49 2018
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -O2 -disable-llvm-passes -emit-llvm -o - | FileCheck %s
+struct A { virtual ~A(); };
+template<typename T> struct B : virtual A {
+  ~B() override {}
+};
+struct C : B<int>, B<float> { C(); ~C() override; };
+struct D : C { ~D() override; };
+
+// We must not create a reference to B<int>::~B() here, because we're not going to emit it.
+// CHECK-NOT: @_ZN1BIiED1Ev
+// CHECK-NOT: @_ZTC1D0_1BIiE =
+// CHECK-NOT: @_ZTT1D = available_externally
+D *p = new D;