EricWF created this revision.
EricWF added reviewers: rsmith, rjmccall, majnemer, vsapsai.

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++.


https://reviews.llvm.org/D46665

Files:
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/rtti-linkage.cpp

Index: test/CodeGenCXX/rtti-linkage.cpp
===================================================================
--- test/CodeGenCXX/rtti-linkage.cpp
+++ test/CodeGenCXX/rtti-linkage.cpp
@@ -3,27 +3,27 @@
 
 #include <typeinfo>
 
-// CHECK-BOTH: _ZTSP1C = internal constant
+// CHECK-BOTH: _ZTSP1C = linkonce_odr constant
 // CHECK-BOTH: _ZTS1C = internal constant
 // CHECK-BOTH: _ZTI1C = internal constant
 // CHECK-BOTH: _ZTIP1C = internal constant
-// CHECK-BOTH: _ZTSPP1C = internal constant
+// CHECK-BOTH: _ZTSPP1C = linkonce_odr constant
 // CHECK-BOTH: _ZTIPP1C = internal constant
-// CHECK-BOTH: _ZTSM1Ci = internal constant
+// CHECK-BOTH: _ZTSM1Ci = linkonce_odr constant
 // CHECK-BOTH: _ZTIM1Ci = internal constant
-// CHECK-BOTH: _ZTSPM1Ci = internal constant
+// CHECK-BOTH: _ZTSPM1Ci = linkonce_odr constant
 // CHECK-BOTH: _ZTIPM1Ci = internal constant
-// CHECK-BOTH: _ZTSM1CS_ = internal constant
+// CHECK-BOTH: _ZTSM1CS_ = linkonce_odr constant
 // CHECK-BOTH: _ZTIM1CS_ = internal constant
-// CHECK-BOTH: _ZTSM1CPS_ = internal constant
+// CHECK-BOTH: _ZTSM1CPS_ = linkonce_odr constant
 // CHECK-BOTH: _ZTIM1CPS_ = internal constant
-// CHECK-BOTH: _ZTSM1A1C = internal constant
+// CHECK-BOTH: _ZTSM1A1C = linkonce_odr 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: _ZTIM1A1C = internal constant
-// CHECK-BOTH: _ZTSM1AP1C = internal constant
+// CHECK-BOTH: _ZTSM1AP1C = linkonce_odr constant
 // CHECK-BOTH: _ZTIM1AP1C = internal constant
 
 // CHECK-WITH-HIDDEN: _ZTSFN12_GLOBAL__N_11DEvE = internal constant
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -3008,8 +3008,9 @@
 
 /// \brief 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) {
   // 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
@@ -3019,9 +3020,22 @@
   //   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;
+  if (ContainsIncompleteClassType(Ty)) {
+    // Itanium C++ ABI 2.9.3:
+    //  The name() member function returns the address of an NTBS, unique to the
+    //  type, ...
+    //
+    // This means we should emit the type info name visibly, so that the type
+    // info objects for the complete and incomplete types share the same string.
+    // See llvm.org/PR37398
+    if (Ty->isPointerType() || Ty->isMemberPointerType())
+      return {llvm::GlobalValue::InternalLinkage,
+              llvm::GlobalValue::LinkOnceODRLinkage};
+    return {llvm::GlobalValue::InternalLinkage,
+            llvm::GlobalValue::InternalLinkage};
+  }
 
+  llvm::GlobalValue::LinkageTypes LinkageForType = [&]() {
     switch (Ty->getLinkage()) {
     case NoLinkage:
     case InternalLinkage:
@@ -3046,18 +3060,18 @@
               ShouldUseExternalRTTIDescriptor(CGM, Ty))
             return llvm::GlobalValue::ExternalLinkage;
         // MinGW always uses LinkOnceODRLinkage for type info.
-      if (RD->isDynamicClass() &&
-          !CGM.getContext()
+        if (RD->isDynamicClass() && !CGM.getContext()
                                          .getTargetInfo()
                                          .getTriple()
                                          .isWindowsGNUEnvironment())
           return CGM.getVTableLinkage(RD);
       }
 
       return llvm::GlobalValue::LinkOnceODRLinkage;
     }
-
     llvm_unreachable("Invalid linkage!");
+  }();
+  return {LinkageForType, LinkageForType};
 }
 
 llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(QualType Ty, bool Force,
@@ -3084,23 +3098,25 @@
     return GetAddrOfExternalRTTIDescriptor(Ty);
 
   // Emit the standard library with external linkage.
-  llvm::GlobalVariable::LinkageTypes Linkage;
+  llvm::GlobalVariable::LinkageTypes TInfoLinkage, NameLinkage;
   if (IsStdLib)
-    Linkage = llvm::GlobalValue::ExternalLinkage;
-  else
-    Linkage = getTypeInfoLinkage(CGM, Ty);
-
+    TInfoLinkage = NameLinkage = llvm::GlobalValue::ExternalLinkage;
+  else {
+    auto TInfoNameLinkage = getTypeInfoLinkage(CGM, Ty);
+    TInfoLinkage = TInfoNameLinkage.first;
+    NameLinkage = TInfoNameLinkage.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, TInfoLinkage);
   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 +3222,7 @@
   llvm::Module &M = CGM.getModule();
   llvm::GlobalVariable *GV =
       new llvm::GlobalVariable(M, Init->getType(),
-                               /*Constant=*/true, Linkage, Init, Name);
+                               /*Constant=*/true, TInfoLinkage, Init, Name);
 
   // If there's already an old global variable, replace it with the new one.
   if (OldGV) {
@@ -3237,19 +3253,20 @@
 
   // Give the type_info object and name the formal visibility of the
   // type itself.
-  llvm::GlobalValue::VisibilityTypes llvmVisibility;
-  if (llvm::GlobalValue::isLocalLinkage(Linkage))
+  auto getLLVMVis = [&](llvm::GlobalVariable::LinkageTypes Linkage) {
+    if (llvm::GlobalValue::isLocalLinkage(TInfoLinkage))
       // If the linkage is local, only default visibility makes sense.
-    llvmVisibility = llvm::GlobalValue::DefaultVisibility;
+      return llvm::GlobalValue::DefaultVisibility;
     else if (RTTIUniqueness == ItaniumCXXABI::RUK_NonUniqueHidden)
-    llvmVisibility = llvm::GlobalValue::HiddenVisibility;
+      return llvm::GlobalValue::HiddenVisibility;
     else
-    llvmVisibility = CodeGenModule::GetLLVMVisibility(Ty->getVisibility());
+      return CodeGenModule::GetLLVMVisibility(Ty->getVisibility());
+  };
 
-  TypeName->setVisibility(llvmVisibility);
+  TypeName->setVisibility(getLLVMVis(TInfoLinkage));
   CGM.setDSOLocal(TypeName);
 
-  GV->setVisibility(llvmVisibility);
+  GV->setVisibility(getLLVMVis(NameLinkage));
   CGM.setDSOLocal(GV);
 
   if (CGM.getTriple().isWindowsItaniumEnvironment()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to