[clang][RelativeVTablesABI] Place non-local vtables in comdat groups

[leonardchan]

Summary

Currently, it's possible for relative vtables (RV) that fit precisely in a mergable constants group to not be collected by --gc-sections even when -fdata-sections is enabled. This results in some vtables that would normally be garbage collected to persist after linking, causing extra bloat in rodata.

This happens because constants that do not need relocations according to Constant::needsRelocation have the option of being placed into these .rodata.cstN sections. That is, a vtable can get placed into .section .rodata.cst16 rather than something like .section .rodata.{vtable} under -fdata-sections, and --gc-sections would not collect the vtable in .section .rodata.cst16.

Note that this doesn't affect vtables under the default Itanium ABI that *could* fit into one of these mergeable sections but *would* get placed only into .rodata. Example:

class A {
 public:
  virtual void func();
  virtual void func2();
};

void A::func() {}
void A::func2() {}

compiled with ./bin/clang++ -c /tmp/test.cc -target x86_64-linux-gnu -fno-pic -fno-rtti -fdata-sections -S -o - generates

	.type	_ZTV1A,@object                  # @_ZTV1A
	.section	.rodata._ZTV1A,"a",@progbits
	.globl	_ZTV1A
	.p2align	3
_ZTV1A:
	.quad	0
	.quad	0
	.quad	_ZN1A4funcEv
	.quad	_ZN1A5func2Ev
	.size	_ZTV1A, 32

The only reason this isn't placed into .rodata.cst32 is because they need relocations according to Constant::needsRelocation (so they take a different branch from what relative vtables would), but because this is made with a static relocation model (non-PIC mode), the vtable can be safely placed in .rodata. This is a unique scenario that is only surfacing now from the RV work.

To keep this contained under the realm of RV changes, a simple fix would be to place these kinds of vtables in comdat groups when appropriate, then --gc-sections can collect them appropriately.

clang/lib/CodeGen/CGVTables.h

public:
   llvm::Type *getVTableType(const VTableLayout &layout);
 
   /// Generate a public facing alias for the vtable and make the vtable either
   /// hidden or private. The alias will have the original linkage and visibility
   /// of the vtable. This is used for cases under the relative vtables ABI
   /// when a vtable may not be dso_local.
   void GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
                                    llvm::StringRef AliasNameRef);
+
+  /// Check if we can place the vtable in a comdat group under the relative
+  /// layout.
+  bool CanPlaceRelativeVTableInComdat(const llvm::GlobalVariable *VTable);
 };
 
 } // end namespace CodeGen
 } // end namespace clang
 #endif

clang/lib/CodeGen/CGVTables.cpp

llvm::GlobalVariable *CodeGenVTables::GenerateConstructionVTable(
   assert(!VTable->isDeclaration() && "Shouldn't set properties on declaration");
   CGM.setGVProperties(VTable, RD);
 
   CGM.EmitVTableTypeMetadata(RD, VTable, *VTLayout.get());
 
   if (UsingRelativeLayout && !VTable->isDSOLocal())
     GenerateRelativeVTableAlias(VTable, OutName);
 
+  if (CanPlaceRelativeVTableInComdat(VTable))
+    VTable->setComdat(CGM.getModule().getOrInsertComdat(VTable->getName()));
+
   return VTable;
 }
 
+bool CodeGenVTables::CanPlaceRelativeVTableInComdat(
+    const llvm::GlobalVariable *VTable) {
+  return CGM.supportsCOMDAT() && getItaniumVTableContext().isRelativeLayout() &&
+         !VTable->hasLocalLinkage() && !VTable->isDeclarationForLinker() &&
+         !VTable->hasComdat();
+}
+
 // If the VTable is not dso_local, then we will not be able to indicate that
 // the VTable does not need a relocation and move into rodata. A frequent
 // time this can occur is for classes that should be made public from a DSO
 // (like in libc++). For cases like these, we can make the vtable hidden or
 // private and create a public alias with the same visibility and linkage as
 // the original vtable type.
 void CodeGenVTables::GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
                                                  llvm::StringRef AliasNameRef) {

clang/lib/CodeGen/ItaniumCXXABI.cpp

if (RD->getIdentifier() &&
       DC->getParent()->isTranslationUnit())
     EmitFundamentalRTTIDescriptors(RD);
 
   if (!VTable->isDeclarationForLinker())
     CGM.EmitVTableTypeMetadata(RD, VTable, VTLayout);
 
   if (VTContext.isRelativeLayout() && !VTable->isDSOLocal())
     CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());
+
+  if (CGVT.CanPlaceRelativeVTableInComdat(VTable))
+    VTable->setComdat(CGM.getModule().getOrInsertComdat(VTable->getName()));
 }
 
 bool ItaniumCXXABI::isVirtualOffsetNeededForVTableField(
     CodeGenFunction &CGF, CodeGenFunction::VPtr Vptr) {
   if (Vptr.NearestVBase == nullptr)
     return false;
   return NeedsVTTParameter(CGF.CurGD);
 }

clang/test/CodeGenCXX/RelativeVTablesABI/vtable-in-comdat.cpp

+// Check that all vtables under the relative ABI are placed in a comdat group.
+// This is necessary because if the vtable happens to fit in a mergable
+// constants section, then the LLVM backend will not place the symbol in its own
+// unique section even if -fdata-sections is enabled. (That is, in the object
+// file the vtable is defined, it will be in section .rodata.cst{N} instead of
+// something like .rodata.{vtable}). This can cause vtables to be retained that
+// normally would be collected by --gc-sections. Placing them in a comdat group
+// can guarantee that they will be placed in a unique section group and garbage
+// collected.
+
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -S -o - -emit-llvm -fexperimental-relative-c++-abi-vtables -fno-rtti | FileCheck %s
+
+// This will result in a vtable of size 16, which would normally be placed in
+// .rodata.cst16.
+class A {
+public:
+  virtual void func();
+  virtual void func2();
+};
+
+void A::func() {}
+void A::func2() {}
+
+// CHECK: @_ZTV1A = {{.*}}, comdat, align 4