r332028 - [Itanium] Emit type info names with external linkage.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Thu May 10 12:51:56 PDT 2018


Author: ericwf
Date: Thu May 10 12:51:56 2018
New Revision: 332028

URL: http://llvm.org/viewvc/llvm-project?rev=332028&view=rev
Log:
[Itanium] Emit type info names with external linkage.

Summary:
The Itanium ABI requires that the type info for pointer-to-incomplete types to have internal linkage, so that it doesn't interfere with the type info once completed.  Currently it also marks the type info name as internal as well. However, this causes a bug with the STL implementations, which use the type info name pointer to perform ordering and hashing of type infos.
For example:

```
// header.h
struct T;
extern std::type_info const& Info;

// tu_one.cpp
#include "header.h"
std::type_info const& Info = typeid(T*);

// tu_two.cpp
#include "header.h"
struct T {};
int main() {
  auto &TI1 = Info;
  auto &TI2 = typeid(T*);
  assert(TI1 == TI2); // Fails
  assert(TI1.hash_code() == TI2.hash_code()); // Fails
}
```

This patch fixes the STL bug by emitting the type info name as linkonce_odr when the type-info is for a pointer-to-incomplete type.

Note that libc++ could fix this without a compiler change, but the quality of fix would be poor. The library would either have to:

(A) Always perform strcmp/string hashes.
(B) Determine if we have a pointer-to-incomplete type, and only do strcmp then. This would require an ABI break for libc++.


Reviewers: rsmith, rjmccall, majnemer, vsapsai

Reviewed By: rjmccall

Subscribers: smeenai, cfe-commits

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

Modified:
    cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
    cfe/trunk/test/CodeGenCXX/rtti-linkage.cpp

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=332028&r1=332027&r2=332028&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Thu May 10 12:51:56 2018
@@ -3008,8 +3008,46 @@ void ItaniumRTTIBuilder::BuildVTablePoin
 
 /// Return the linkage that the type info and type info name constants
 /// should have for the given type.
-static llvm::GlobalVariable::LinkageTypes getTypeInfoLinkage(CodeGenModule &CGM,
-                                                             QualType Ty) {
+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;
+
+    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);
+      }
+
+      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
@@ -3020,44 +3058,8 @@ static llvm::GlobalVariable::LinkageType
   //   complete class RTTI (because the latter need not exist), possibly by
   //   making it a local static object.
   if (ContainsIncompleteClassType(Ty))
-    return llvm::GlobalValue::InternalLinkage;
-
-  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;
-
-    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->isDynamicClass() &&
-          !CGM.getContext()
-               .getTargetInfo()
-               .getTriple()
-               .isWindowsGNUEnvironment())
-        return CGM.getVTableLinkage(RD);
-    }
-
-    return llvm::GlobalValue::LinkOnceODRLinkage;
-  }
-
-  llvm_unreachable("Invalid linkage!");
+    return {llvm::GlobalValue::InternalLinkage, TypeLinkage};
+  return {TypeLinkage, TypeLinkage};
 }
 
 llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(QualType Ty, bool Force,
@@ -3084,23 +3086,25 @@ llvm::Constant *ItaniumRTTIBuilder::Buil
     return GetAddrOfExternalRTTIDescriptor(Ty);
 
   // Emit the standard library with external linkage.
-  llvm::GlobalVariable::LinkageTypes Linkage;
+  llvm::GlobalVariable::LinkageTypes InfoLinkage, NameLinkage;
   if (IsStdLib)
-    Linkage = llvm::GlobalValue::ExternalLinkage;
-  else
-    Linkage = getTypeInfoLinkage(CGM, Ty);
-
+    InfoLinkage = NameLinkage = llvm::GlobalValue::ExternalLinkage;
+  else {
+    auto LinkagePair = getTypeInfoLinkage(CGM, Ty);
+    InfoLinkage = LinkagePair.first;
+    NameLinkage = LinkagePair.second;
+  }
   // Add the vtable pointer.
   BuildVTablePointer(cast<Type>(Ty));
 
   // And the name.
-  llvm::GlobalVariable *TypeName = GetAddrOfTypeName(Ty, Linkage);
+  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, Linkage);
+      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.
@@ -3206,7 +3210,7 @@ llvm::Constant *ItaniumRTTIBuilder::Buil
   llvm::Module &M = CGM.getModule();
   llvm::GlobalVariable *GV =
       new llvm::GlobalVariable(M, Init->getType(),
-                               /*Constant=*/true, Linkage, Init, Name);
+                               /*Constant=*/true, InfoLinkage, Init, Name);
 
   // If there's already an old global variable, replace it with the new one.
   if (OldGV) {
@@ -3237,19 +3241,20 @@ llvm::Constant *ItaniumRTTIBuilder::Buil
 
   // Give the type_info object and name the formal visibility of the
   // type itself.
-  llvm::GlobalValue::VisibilityTypes llvmVisibility;
-  if (llvm::GlobalValue::isLocalLinkage(Linkage))
-    // If the linkage is local, only default visibility makes sense.
-    llvmVisibility = llvm::GlobalValue::DefaultVisibility;
-  else if (RTTIUniqueness == ItaniumCXXABI::RUK_NonUniqueHidden)
-    llvmVisibility = llvm::GlobalValue::HiddenVisibility;
-  else
-    llvmVisibility = CodeGenModule::GetLLVMVisibility(Ty->getVisibility());
+  auto computeVisibility = [&](llvm::GlobalValue::LinkageTypes Linkage) {
+    if (llvm::GlobalValue::isLocalLinkage(Linkage))
+      // If the linkage is local, only default visibility makes sense.
+      return llvm::GlobalValue::DefaultVisibility;
+    else if (RTTIUniqueness == ItaniumCXXABI::RUK_NonUniqueHidden)
+      return llvm::GlobalValue::HiddenVisibility;
+    else
+      return CodeGenModule::GetLLVMVisibility(Ty->getVisibility());
+  };
 
-  TypeName->setVisibility(llvmVisibility);
+  TypeName->setVisibility(computeVisibility(NameLinkage));
   CGM.setDSOLocal(TypeName);
 
-  GV->setVisibility(llvmVisibility);
+  GV->setVisibility(computeVisibility(InfoLinkage));
   CGM.setDSOLocal(GV);
 
   if (CGM.getTriple().isWindowsItaniumEnvironment()) {

Modified: cfe/trunk/test/CodeGenCXX/rtti-linkage.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/rtti-linkage.cpp?rev=332028&r1=332027&r2=332028&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/rtti-linkage.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/rtti-linkage.cpp Thu May 10 12:51:56 2018
@@ -1,29 +1,31 @@
-// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-BOTH
-// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -fvisibility hidden -emit-llvm -o - | FileCheck -check-prefix=CHECK-WITH-HIDDEN -check-prefix=CHECK-BOTH %s
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm -o - | \
+// RUN:    FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-BOTH \
+// RUN:   -DLINKONCE_VIS_CONSTANT='linkonce_odr constant'
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -fvisibility hidden -emit-llvm -o - | \
+// RUN:    FileCheck %s -check-prefix=CHECK-WITH-HIDDEN -check-prefix=CHECK-BOTH \
+// RUN:   -DLINKONCE_VIS_CONSTANT='linkonce_odr hidden constant'
 
 #include <typeinfo>
 
-// CHECK-BOTH: _ZTSP1C = internal constant
-// CHECK-BOTH: _ZTS1C = internal constant
+// CHECK-BOTH: _ZTSP1C = [[LINKONCE_VIS_CONSTANT]]
+// CHECK-BOTH: _ZTS1C = [[LINKONCE_VIS_CONSTANT]]
 // CHECK-BOTH: _ZTI1C = internal constant
 // CHECK-BOTH: _ZTIP1C = internal constant
-// CHECK-BOTH: _ZTSPP1C = internal constant
+// CHECK-BOTH: _ZTSPP1C = [[LINKONCE_VIS_CONSTANT]]
 // CHECK-BOTH: _ZTIPP1C = internal constant
-// CHECK-BOTH: _ZTSM1Ci = internal constant
+// CHECK-BOTH: _ZTSM1Ci = [[LINKONCE_VIS_CONSTANT]]
 // CHECK-BOTH: _ZTIM1Ci = internal constant
-// CHECK-BOTH: _ZTSPM1Ci = internal constant
+// CHECK-BOTH: _ZTSPM1Ci = [[LINKONCE_VIS_CONSTANT]]
 // CHECK-BOTH: _ZTIPM1Ci = internal constant
-// CHECK-BOTH: _ZTSM1CS_ = internal constant
+// CHECK-BOTH: _ZTSM1CS_ = [[LINKONCE_VIS_CONSTANT]]
 // CHECK-BOTH: _ZTIM1CS_ = internal constant
-// CHECK-BOTH: _ZTSM1CPS_ = internal constant
+// CHECK-BOTH: _ZTSM1CPS_ = [[LINKONCE_VIS_CONSTANT]]
 // CHECK-BOTH: _ZTIM1CPS_ = internal constant
-// CHECK-BOTH: _ZTSM1A1C = internal constant
-// CHECK: _ZTS1A = linkonce_odr constant
-// CHECK-WITH-HIDDEN: _ZTS1A = linkonce_odr hidden constant
-// CHECK: _ZTI1A = linkonce_odr constant
-// CHECK-WITH-HIDDEN: _ZTI1A = linkonce_odr hidden constant
+// CHECK-BOTH: _ZTSM1A1C = [[LINKONCE_VIS_CONSTANT]]
+// CHECK-BOTH: _ZTS1A = [[LINKONCE_VIS_CONSTANT]]
+// CHECK-BOTH: _ZTI1A = [[LINKONCE_VIS_CONSTANT]]
 // CHECK-BOTH: _ZTIM1A1C = internal constant
-// CHECK-BOTH: _ZTSM1AP1C = internal constant
+// CHECK-BOTH: _ZTSM1AP1C = [[LINKONCE_VIS_CONSTANT]]
 // CHECK-BOTH: _ZTIM1AP1C = internal constant
 
 // CHECK-WITH-HIDDEN: _ZTSFN12_GLOBAL__N_11DEvE = internal constant
@@ -52,6 +54,12 @@
 // CHECK: _ZTSFvvE = linkonce_odr constant
 // CHECK: _ZTIFvvE = linkonce_odr constant
 // CHECK: _ZTIPFvvE = linkonce_odr constant
+// CHECK: _ZTSPN12_GLOBAL__N_12DIE = internal constant
+// CHECK: _ZTSN12_GLOBAL__N_12DIE = internal constant
+// CHECK: _ZTIN12_GLOBAL__N_12DIE = internal constant
+// CHECK: _ZTIPN12_GLOBAL__N_12DIE = internal constant
+// CHECK: _ZTSMN12_GLOBAL__N_12DIEFvvE = internal constant
+// CHECK: _ZTIMN12_GLOBAL__N_12DIEFvvE = internal constant
 // CHECK: _ZTSN12_GLOBAL__N_11EE = internal constant
 // CHECK: _ZTIN12_GLOBAL__N_11EE = internal constant
 // CHECK: _ZTSA10_i = linkonce_odr constant
@@ -99,12 +107,13 @@ void t1() {
 }
 
 namespace {
-  // D is inside an anonymous namespace, so all type information related to D should have
-  // internal linkage.
-  struct D { };
-  
-  // E is also inside an anonymous namespace.
-  enum E { };
+// D and DI are inside an anonymous namespace, so all type information related
+// to both should have internal linkage.
+struct D {};
+struct DI;
+
+// E is also inside an anonymous namespace.
+enum E {};
   
 };
 
@@ -126,7 +135,10 @@ const std::type_info &t2() {
   // The exception specification is not part of the RTTI descriptor, so it should not have
   // internal linkage.
   (void)typeid(void (*)() throw (D));
-  
+
+  (void)typeid(DI *);
+  (void)typeid(void (DI::*)());
+
   (void)typeid(E);
   
   return typeid(getD());  




More information about the cfe-commits mailing list