[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson created this revision.
tejohnson added a reviewer: pcc.
Herald added a subscriber: Prazek.
tejohnson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When WPD is enabled, via WholeProgramVTables, emit type metadata for
available_externally vtables. Additionally, add the vtables to the
llvm.compiler.used global so that they are not prematurely eliminated
(before *LTO analysis).

This is needed to avoid devirtualizing calls to a function overriding a
class defined in a header file but with a strong definition in a shared
library. Without type metadata on the available_externally vtables from
the header, the WPD analysis never sees what a derived class is
overriding. Even if the available_externally base class functions are
pure virtual, because shared library definitions are already treated
conservatively (committed patches D91583 <https://reviews.llvm.org/D91583>, D96721 <https://reviews.llvm.org/D96721>, and D96722 <https://reviews.llvm.org/D96722>) we will
not devirtualize, which would be unsafe since the library might contain
overrides that aren't visible to the LTO unit.

An example is std::error_category, which is overridden in LLVM
and causing failures after a self build with WPD enabled, because
libstdc++ contains hidden overrides of the virtual base class methods.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96919

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/type-metadata.cpp


Index: clang/test/CodeGenCXX/type-metadata.cpp
===================================================================
--- clang/test/CodeGenCXX/type-metadata.cpp
+++ clang/test/CodeGenCXX/type-metadata.cpp
@@ -7,6 +7,7 @@
 // Tests for the whole-program-vtables feature:
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM %s
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM-DEFAULTVIS --check-prefix=TT-ITANIUM %s
+// RUN: %clang_cc1 -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT %s
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=MS --check-prefix=TT-MS %s
 
 // Tests for cfi + whole-program-vtables:
@@ -79,6 +80,13 @@
 // ITANIUM-DIAG-SAME: !type [[ALL16]]
 // ITANIUM-SAME: !type [[FAF16:![0-9]+]]
 
+// ITANIUM: @_ZTVN5test31EE = external unnamed_addr constant
+// ITANIUM-DEFAULTVIS: @_ZTVN5test31EE = external unnamed_addr constant
+// ITANIUM-OPT: @_ZTVN5test31EE = available_externally unnamed_addr constant {{[^!]*}},
+// ITANIUM-OPT-SAME: !type [[E16:![0-9]+]],
+// ITANIUM-OPT-SAME: !type [[EF16:![0-9]+]]
+// ITANIUM-OPT: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast ({ [3 x i8*] }* @_ZTVN5test31EE to i8*)]
+
 // MS: comdat($"??_7A@@6B@"), !type [[A8:![0-9]+]]
 // MS: comdat($"??_7B@@6B0@@"), !type [[B8:![0-9]+]]
 // MS: comdat($"??_7B@@6BA@@@"), !type [[A8]]
@@ -253,6 +261,20 @@
 
 }
 
+namespace test3 {
+// All virtual functions are outline, so we can assume that it will
+// be generated in translation unit where foo is defined.
+struct E {
+  virtual void foo();
+};
+
+void g() {
+  E e;
+  e.foo();
+}
+
+}  // Test9
+
 // ITANIUM: [[A16]] = !{i64 16, !"_ZTS1A"}
 // ITANIUM-DIAG: [[ALL16]] = !{i64 16, !"all-vtables"}
 // ITANIUM: [[AF16]] = !{i64 16, !"_ZTSM1AFvvE.virtual"}
@@ -286,6 +308,9 @@
 // ITANIUM: [[FAF16]] = !{i64 16, [[FAF_ID:![0-9]+]]}
 // ITANIUM: [[FAF_ID]] = distinct !{}
 
+// ITANIUM-OPT: [[E16]] = !{i64 16, !"_ZTSN5test31EE"}
+// ITANIUM-OPT: [[EF16]] = !{i64 16, !"_ZTSMN5test31EEFvvE.virtual"}
+
 // MS: [[A8]] = !{i64 8, !"?AUA@@"}
 // MS: [[B8]] = !{i64 8, !"?AUB@@"}
 // MS: [[D8]] = !{i64 8, [[D_ID:![0-9]+]]}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1767,8 +1767,12 @@
       DC->getParent()->isTranslationUnit())
     EmitFundamentalRTTIDescriptors(RD);
 
-  if (!VTable->isDeclarationForLinker())
+  if (!VTable->isDeclarationForLinker() ||
+      CGM.getCodeGenOpts().WholeProgramVTables) {
     CGM.EmitVTableTypeMetadata(RD, VTable, VTLayout);
+    if (VTable->isDeclarationForLinker())
+      CGM.addCompilerUsedGlobal(VTable);
+  }
 
   if (VTContext.isRelativeLayout() && !VTable->isDSOLocal())
     CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96919.324469.patch
Type: text/x-patch
Size: 3281 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210218/272cb9b3/attachment-0001.bin>