[CodeGen] Set construction vtable visibility after creating initializer

[phosek]

Summary

We must only set the construction vtable visibility after we create the
vtable initializer, otherwise the global value will be treated as
declaration rather than definition and the visibility won't be set.

[mcgrathr]

lgtm with a comment (and perhaps an assertion).

clang/lib/CodeGen/CGVTables.cpp

777 ↗

(On Diff #186141)

Give it a comment pointing out the importance of doing this after createVTableInitializer rather than before.
If there's an easy way to add an "assert(is defined)" before setGVProperties to go with the comment, do that too.

cfe/trunk/lib/CodeGen/CGVTables.cpp

CodeGenVTables::GenerateConstructionVTable(const CXXRecordDecl *RD,
   if (Linkage == llvm::GlobalVariable::AvailableExternallyLinkage)
     Linkage = llvm::GlobalVariable::InternalLinkage;
 
   unsigned Align = CGM.getDataLayout().getABITypeAlignment(VTType);
 
   // Create the variable that will hold the construction vtable.
   llvm::GlobalVariable *VTable =
       CGM.CreateOrReplaceCXXRuntimeVariable(Name, VTType, Linkage, Align);
-  CGM.setGVProperties(VTable, RD);
 
   // V-tables are always unnamed_addr.
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
 
   llvm::Constant *RTTI = CGM.GetAddrOfRTTIDescriptor(
       CGM.getContext().getTagDeclType(Base.getBase()));
 
   // Create and set the initializer.
   ConstantInitBuilder builder(CGM);
   auto components = builder.beginStruct();
   createVTableInitializer(components, *VTLayout, RTTI);
   components.finishAndSetAsInitializer(VTable);
 
+  // Set properties only after the initializer has been set to ensure that the
+  // GV is treated as definition and not declaration.
+  assert(!VTable->isDeclaration() && "Shouldn't set properties on declaration");
+  CGM.setGVProperties(VTable, RD);
+
   CGM.EmitVTableTypeMetadata(VTable, *VTLayout.get());
 
   return VTable;
 }
 
 static bool shouldEmitAvailableExternallyVTable(const CodeGenModule &CGM,
                                                 const CXXRecordDecl *RD) {
   return CGM.getCodeGenOpts().OptimizationLevel > 0 &&

cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp

+// RUN: %clang_cc1 -triple x86_64-linux-unknown -fvisibility hidden -emit-llvm %s -o - | FileCheck %s
+
+struct Base {};
+
+class Parent1 : virtual public Base {};
+
+class Parent2 : virtual public Base {};
+
+class Child : public Parent1, public Parent2 {};
+
+void test() {
+  Child x;
+}
+
+// CHECK: @_ZTC5Child0_7Parent1 = linkonce_odr hidden unnamed_addr constant
+// CHECK: @_ZTC5Child8_7Parent2 = linkonce_odr hidden unnamed_addr constant