rsmith created this revision.
rsmith added reviewers: EricWF, rjmccall, eli.friedman, jgorbe.
Herald added a reviewer: javed.absar.

After https://reviews.llvm.org/D46665 / https://reviews.llvm.org/rC332028, we 
now emit `linkonce_odr` definitions of type info names for incomplete class 
types. This results in formal violations of LLVM's linkage rules (which, for 
example, cause ASan's ODR violation checker to complain) if the type info name 
is declared with `external` linkage in another IR module. To solve this, 
downgrade any type info names that we would give `external` linkage to instead 
have `weak_odr` linkage. Exception: if we would emit a non-unique `type_info` 
name anyway, leave the linkage alone.

This required a bit of rearrangement: the uniqueness of the name can no longer 
depend on the linkage of the name, because the linkage of the name now depends 
on the uniqueness of the name! Fortunately, the uniqueness doesn't *really* 
depend on the linkage of the name, it only depends on the uniqueness of the 
type. I've split out the type uniqueness / name uniqueness / type_info 
uniqueness calculations to try to make this a bit clearer.


Repository:
  rC Clang

https://reviews.llvm.org/D47092

Files:
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/arm64.cpp
  test/CodeGenCXX/rtti-linkage.cpp
  test/CodeGenCXX/type_visibility.cpp
  test/CodeGenCXX/vtable-available-externally.cpp
  test/CodeGenCXX/vtable-key-function-arm.cpp
  test/CodeGenCXX/vtable-linkage.cpp
  test/CodeGenCXX/windows-itanium-type-info.cpp

Index: test/CodeGenCXX/windows-itanium-type-info.cpp
===================================================================
--- test/CodeGenCXX/windows-itanium-type-info.cpp
+++ test/CodeGenCXX/windows-itanium-type-info.cpp
@@ -28,7 +28,7 @@
 // CHECK-DAG: @_ZTSi = dso_local dllexport constant
 
 // CHECK-DAG: @_ZTI7derived = dso_local dllexport constant
-// CHECK-DAG: @_ZTS7derived = dso_local dllexport constant
+// CHECK-DAG: @_ZTS7derived = weak_odr dso_local dllexport constant
 // CHECK-DAG: @_ZTV7derived = dso_local dllexport unnamed_addr constant
 
 // CHECK-DAG: @_ZTI4base = external dllimport constant
Index: test/CodeGenCXX/vtable-linkage.cpp
===================================================================
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -106,14 +106,14 @@
 // D has a key function that is defined in this translation unit so its vtable is
 // defined in the translation unit.
 // CHECK-DAG: @_ZTV1D = unnamed_addr constant
-// CHECK-DAG: @_ZTS1D = constant
+// CHECK-DAG: @_ZTS1D = weak_odr constant
 // CHECK-DAG: @_ZTI1D = constant
 
 // E<char> is an explicit specialization with a key function defined
 // in this translation unit, so its vtable should have external
 // linkage.
 // CHECK-DAG: @_ZTV1EIcE = unnamed_addr constant
-// CHECK-DAG: @_ZTS1EIcE = constant
+// CHECK-DAG: @_ZTS1EIcE = weak_odr constant
 // CHECK-DAG: @_ZTI1EIcE = constant
 
 // E<short> is an explicit template instantiation with a key function
@@ -201,6 +201,18 @@
   H<int> h;
 }
 
+// local_class()::J is local to this translation unit, so is given internal
+// linkage.
+// CHECK-DAG: @_ZTVZ11local_classvE1J = internal unnamed_addr constant
+// CHECK-DAG: @_ZTSZ11local_classvE1J = internal constant
+// CHECK-DAG: @_ZTIZ11local_classvE1J = internal constant
+void local_class() {
+  struct J {
+    virtual void f() {}
+  };
+  J().f();
+}
+
 // I<int> has an explicit instantiation declaration and needs a VTT and
 // construction vtables.
 
Index: test/CodeGenCXX/vtable-key-function-arm.cpp
===================================================================
--- test/CodeGenCXX/vtable-key-function-arm.cpp
+++ test/CodeGenCXX/vtable-key-function-arm.cpp
@@ -90,7 +90,7 @@
 // V-table should be defined with strong linkage.
 Test2a::Test2a() { use(typeid(Test2a)); }
 // CHECK:      @_ZTV6Test2a = unnamed_addr constant
-// CHECK-LATE: @_ZTS6Test2a = constant
+// CHECK-LATE: @_ZTS6Test2a = weak_odr constant
 // CHECK-LATE: @_ZTI6Test2a = constant
 
 // 'bar' becomes the key function when 'foo' is defined inline.
@@ -111,7 +111,7 @@
 // V-table should be defined with strong linkage.
 Test2b::Test2b() { use(typeid(Test2b)); }
 // CHECK:      @_ZTV6Test2b = unnamed_addr constant
-// CHECK-LATE: @_ZTS6Test2b = constant
+// CHECK-LATE: @_ZTS6Test2b = weak_odr constant
 // CHECK-LATE: @_ZTI6Test2b = constant
 
 inline void Test2b::foo() {}
@@ -131,7 +131,7 @@
 // V-table should be defined with strong linkage.
 Test2c::Test2c() { use(typeid(Test2c)); }
 // CHECK: @_ZTV6Test2c = unnamed_addr constant
-// CHECK: @_ZTS6Test2c = constant
+// CHECK: @_ZTS6Test2c = weak_odr constant
 // CHECK: @_ZTI6Test2c = constant
 
 /*** Test3a ******************************************************************/
Index: test/CodeGenCXX/vtable-available-externally.cpp
===================================================================
--- test/CodeGenCXX/vtable-available-externally.cpp
+++ test/CodeGenCXX/vtable-available-externally.cpp
@@ -45,7 +45,7 @@
 // This tests mainly that the typeinfo and typename constants have their linkage
 // updated correctly.
 
-// CHECK-TEST2: @_ZTSN5Test21AE = constant
+// CHECK-TEST2: @_ZTSN5Test21AE = weak_odr constant
 // CHECK-TEST2: @_ZTIN5Test21AE = constant
 // CHECK-TEST2: @_ZTVN5Test21AE = unnamed_addr constant
 namespace Test2 {
Index: test/CodeGenCXX/type_visibility.cpp
===================================================================
--- test/CodeGenCXX/type_visibility.cpp
+++ test/CodeGenCXX/type_visibility.cpp
@@ -112,11 +112,11 @@
   void A::foo() {}
   // FUNS-LABEL:        define void @_ZN5type01A3fooEv(
   // VARS:        @_ZTVN5type01AE = unnamed_addr constant
-  // VARS:        @_ZTSN5type01AE = constant
+  // VARS:        @_ZTSN5type01AE = weak_odr constant
   // VARS:        @_ZTIN5type01AE = constant
   // FUNS-HIDDEN-LABEL: define hidden void @_ZN5type01A3fooEv(
   // VARS-HIDDEN: @_ZTVN5type01AE = unnamed_addr constant
-  // VARS-HIDDEN: @_ZTSN5type01AE = constant
+  // VARS-HIDDEN: @_ZTSN5type01AE = weak_odr constant
   // VARS-HIDDEN: @_ZTIN5type01AE = constant
 }
 
@@ -128,11 +128,11 @@
   void A::foo() {}
   // FUNS-LABEL:        define hidden void @_ZN5type11A3fooEv(
   // VARS:        @_ZTVN5type11AE = unnamed_addr constant
-  // VARS:        @_ZTSN5type11AE = constant
+  // VARS:        @_ZTSN5type11AE = weak_odr constant
   // VARS:        @_ZTIN5type11AE = constant
   // FUNS-HIDDEN-LABEL: define hidden void @_ZN5type11A3fooEv(
   // VARS-HIDDEN: @_ZTVN5type11AE = unnamed_addr constant
-  // VARS-HIDDEN: @_ZTSN5type11AE = constant
+  // VARS-HIDDEN: @_ZTSN5type11AE = weak_odr constant
   // VARS-HIDDEN: @_ZTIN5type11AE = constant
 }
 
@@ -144,11 +144,11 @@
   void A::foo() {}
   // FUNS-LABEL:        define void @_ZN5type21A3fooEv(
   // VARS:        @_ZTVN5type21AE = hidden unnamed_addr constant
-  // VARS:        @_ZTSN5type21AE = hidden constant
+  // VARS:        @_ZTSN5type21AE = weak_odr hidden constant
   // VARS:        @_ZTIN5type21AE = hidden constant
   // FUNS-HIDDEN-LABEL: define hidden void @_ZN5type21A3fooEv(
   // VARS-HIDDEN: @_ZTVN5type21AE = hidden unnamed_addr constant
-  // VARS-HIDDEN: @_ZTSN5type21AE = hidden constant
+  // VARS-HIDDEN: @_ZTSN5type21AE = weak_odr hidden constant
   // VARS-HIDDEN: @_ZTIN5type21AE = hidden constant
 }
 
@@ -160,11 +160,11 @@
   void A::foo() {}
   // FUNS-LABEL:        define void @_ZN5type31A3fooEv(
   // VARS:        @_ZTVN5type31AE = hidden unnamed_addr constant
-  // VARS:        @_ZTSN5type31AE = hidden constant
+  // VARS:        @_ZTSN5type31AE = weak_odr hidden constant
   // VARS:        @_ZTIN5type31AE = hidden constant
   // FUNS-HIDDEN-LABEL: define void @_ZN5type31A3fooEv(
   // VARS-HIDDEN: @_ZTVN5type31AE = hidden unnamed_addr constant
-  // VARS-HIDDEN: @_ZTSN5type31AE = hidden constant
+  // VARS-HIDDEN: @_ZTSN5type31AE = weak_odr hidden constant
   // VARS-HIDDEN: @_ZTIN5type31AE = hidden constant
 }
 
Index: test/CodeGenCXX/rtti-linkage.cpp
===================================================================
--- test/CodeGenCXX/rtti-linkage.cpp
+++ test/CodeGenCXX/rtti-linkage.cpp
@@ -69,7 +69,7 @@
 // CHECK: _ZTI1TILj2EE = external constant
 // CHECK: _ZTSZ2t5vE1A = internal constant
 // CHECK: _ZTIZ2t5vE1A = internal constant
-// CHECK: _ZTS1B = constant
+// CHECK: _ZTS1B = weak_odr constant
 // CHECK: _ZTI1B = constant
 // CHECK: _ZTS1F = linkonce_odr constant
 // CHECK: _ZTSZ2t6vE1A = linkonce_odr constant
Index: test/CodeGenCXX/arm64.cpp
===================================================================
--- test/CodeGenCXX/arm64.cpp
+++ test/CodeGenCXX/arm64.cpp
@@ -45,7 +45,7 @@
     virtual void foo();
   };
   void A::foo() {}
-  // CHECK-GLOBALS-DAG: @_ZTSN5test21AE = constant [11 x i8]
+  // CHECK-GLOBALS-DAG: @_ZTSN5test21AE = weak_odr constant [11 x i8]
   // CHECK-GLOBALS-DAG: @_ZTIN5test21AE = constant { {{.*}}, i8* getelementptr inbounds ([11 x i8], [11 x i8]* @_ZTSN5test21AE, i32 0, i32 0) }
 
   struct __attribute__((visibility("hidden"))) B {};
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -3004,62 +3004,66 @@
   Fields.push_back(VTable);
 }
 
-/// Return the linkage that the type info and type info name constants
-/// should have for the given type.
-static std::pair<llvm::GlobalVariable::LinkageTypes,
-                 llvm::GlobalVariable::LinkageTypes>
-getTypeInfoLinkage(CodeGenModule &CGM, QualType Ty) {
-  llvm::GlobalValue::LinkageTypes TypeLinkage = [&]() {
-    switch (Ty->getLinkage()) {
-    case NoLinkage:
-    case InternalLinkage:
-    case UniqueExternalLinkage:
-      return llvm::GlobalValue::InternalLinkage;
+/// Return the linkage that the type info constant should have for the given
+/// type. This does not take into account the demotion of incomplete type info 
+/// to internal linkage required by the Itanium ABI.
+static llvm::GlobalVariable::LinkageTypes getTypeInfoLinkage(CodeGenModule &CGM,
+                                                             QualType Ty) {
+  switch (Ty->getLinkage()) {
+  case NoLinkage:
+  case InternalLinkage:
+  case UniqueExternalLinkage:
+    return llvm::GlobalValue::InternalLinkage;
 
-    case VisibleNoLinkage:
-    case ModuleInternalLinkage:
-    case ModuleLinkage:
-    case ExternalLinkage:
-      // RTTI is not enabled, which means that this type info struct is going
-      // to be used for exception handling. Give it linkonce_odr linkage.
-      if (!CGM.getLangOpts().RTTI)
-        return llvm::GlobalValue::LinkOnceODRLinkage;
+  case VisibleNoLinkage:
+  case ModuleInternalLinkage:
+  case ModuleLinkage:
+  case ExternalLinkage:
+    // RTTI is not enabled, which means that this type info struct is going
+    // to be used for exception handling. Give it linkonce_odr linkage.
+    if (!CGM.getLangOpts().RTTI)
+      return llvm::GlobalValue::LinkOnceODRLinkage;
 
-      if (const RecordType *Record = dyn_cast<RecordType>(Ty)) {
-        const CXXRecordDecl *RD = cast<CXXRecordDecl>(Record->getDecl());
-        if (RD->hasAttr<WeakAttr>())
-          return llvm::GlobalValue::WeakODRLinkage;
-        if (CGM.getTriple().isWindowsItaniumEnvironment())
-          if (RD->hasAttr<DLLImportAttr>() &&
-              ShouldUseExternalRTTIDescriptor(CGM, Ty))
-            return llvm::GlobalValue::ExternalLinkage;
-        // MinGW always uses LinkOnceODRLinkage for type info.
-        if (RD->isCompleteDefinition() && RD->isDynamicClass() &&
-            !CGM.getContext()
-                 .getTargetInfo()
-                 .getTriple()
-                 .isWindowsGNUEnvironment())
-          return CGM.getVTableLinkage(RD);
-      }
+    if (const RecordType *Record = dyn_cast<RecordType>(Ty)) {
+      const CXXRecordDecl *RD = cast<CXXRecordDecl>(Record->getDecl());
+      if (RD->hasAttr<WeakAttr>())
+        return llvm::GlobalValue::WeakODRLinkage;
+      if (CGM.getTriple().isWindowsItaniumEnvironment())
+        if (RD->hasAttr<DLLImportAttr>() &&
+            ShouldUseExternalRTTIDescriptor(CGM, Ty))
+          return llvm::GlobalValue::ExternalLinkage;
+      // MinGW always uses LinkOnceODRLinkage for type info.
+      if (RD->isCompleteDefinition() && RD->isDynamicClass() &&
+          !CGM.getContext()
+               .getTargetInfo()
+               .getTriple()
+               .isWindowsGNUEnvironment())
+        return CGM.getVTableLinkage(RD);
+    }
 
-      return llvm::GlobalValue::LinkOnceODRLinkage;
-    }
-    llvm_unreachable("Invalid linkage!");
-  }();
-  // Itanium C++ ABI 2.9.5p7:
-  //   In addition, it and all of the intermediate abi::__pointer_type_info
-  //   structs in the chain down to the abi::__class_type_info for the
-  //   incomplete class type must be prevented from resolving to the
-  //   corresponding type_info structs for the complete class type, possibly
-  //   by making them local static objects. Finally, a dummy class RTTI is
-  //   generated for the incomplete type that will not resolve to the final
-  //   complete class RTTI (because the latter need not exist), possibly by
-  //   making it a local static object.
-  if (ContainsIncompleteClassType(Ty))
-    return {llvm::GlobalValue::InternalLinkage, TypeLinkage};
-  return {TypeLinkage, TypeLinkage};
+    return llvm::GlobalValue::LinkOnceODRLinkage;
+  }
+  llvm_unreachable("Invalid linkage!");
 }
 
+/// Determine the linkage that the type info name constant should have for
+/// the given type in the current translation unit.
+static llvm::GlobalVariable::LinkageTypes
+getTypeInfoNameLinkage(CodeGenModule &CGM, QualType Ty,
+                       llvm::GlobalVariable::LinkageTypes TypeLinkage,
+                       ItaniumCXXABI::RTTIUniquenessKind NameUniqueness) {
+  // If a class could be forward-declared in another translation unit, and
+  // we want a globally unique type info name constant, we cannot emit its type
+  // info name with external linkage even if this TU contains the type's key
+  // function, because we might emit a linkonce_odr type info name for the type
+  // in the other TU.
+  if (TypeLinkage == llvm::GlobalValue::ExternalLinkage &&
+      NameUniqueness == ItaniumCXXABI::RUK_Unique)
+    return llvm::GlobalValue::WeakODRLinkage;
+
+  return TypeLinkage;
+}
+
 llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(QualType Ty, bool Force,
                                                   bool DLLExport) {
   // We want to operate on the canonical type.
@@ -3084,25 +3088,42 @@
     return GetAddrOfExternalRTTIDescriptor(Ty);
 
   // Emit the standard library with external linkage.
-  llvm::GlobalVariable::LinkageTypes InfoLinkage, NameLinkage;
+  llvm::GlobalVariable::LinkageTypes TypeLinkage;
   if (IsStdLib)
-    InfoLinkage = NameLinkage = llvm::GlobalValue::ExternalLinkage;
-  else {
-    auto LinkagePair = getTypeInfoLinkage(CGM, Ty);
-    InfoLinkage = LinkagePair.first;
-    NameLinkage = LinkagePair.second;
-  }
+    TypeLinkage = llvm::GlobalValue::ExternalLinkage;
+  else
+    TypeLinkage = getTypeInfoLinkage(CGM, Ty);
+
+  // Determine the uniqueness and linkage to use for the name.
+  ItaniumCXXABI::RTTIUniquenessKind RTTIUniqueness =
+      CXXABI.classifyRTTIUniqueness(Ty, TypeLinkage);
+  llvm::GlobalVariable::LinkageTypes NameLinkage =
+      getTypeInfoNameLinkage(CGM, Ty, TypeLinkage, RTTIUniqueness);
+
+  // Determine the linkage to use for this type_info object.
+  //
+  // Itanium C++ ABI 2.9.5p7:
+  //   In addition, it and all of the intermediate abi::__pointer_type_info
+  //   structs in the chain down to the abi::__class_type_info for the
+  //   incomplete class type must be prevented from resolving to the
+  //   corresponding type_info structs for the complete class type, possibly
+  //   by making them local static objects. Finally, a dummy class RTTI is
+  //   generated for the incomplete type that will not resolve to the final
+  //   complete class RTTI (because the latter need not exist), possibly by
+  //   making it a local static object.
+  llvm::GlobalVariable::LinkageTypes InfoLinkage =
+      ContainsIncompleteClassType(Ty) ? llvm::GlobalValue::InternalLinkage
+                                      : TypeLinkage;
+
   // Add the vtable pointer.
   BuildVTablePointer(cast<Type>(Ty));
 
-  // And the name.
+  // Add the name.
   llvm::GlobalVariable *TypeName = GetAddrOfTypeName(Ty, NameLinkage);
   llvm::Constant *TypeNameField;
 
   // If we're supposed to demote the visibility, be sure to set a flag
   // to use a string comparison for type_info comparisons.
-  ItaniumCXXABI::RTTIUniquenessKind RTTIUniqueness =
-      CXXABI.classifyRTTIUniqueness(Ty, NameLinkage);
   if (RTTIUniqueness != ItaniumCXXABI::RUK_Unique) {
     // The flag is the sign bit, which on ARM64 is defined to be clear
     // for global pointers.  This is very ARM64-specific.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D47092: d... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to