Fix linkage of _ZTS strings for types involving incomplete classes to match the Itanium ABI rules.

[rsmith]

Summary

Without this patch, type_info objects for pointers to incomplete classes
compare unequal with libc++ when formed in multiple translation units,
because each translation unit has its own copy of the _ZTS symbol, in
violation of the Itanium ABI's uniqueness rule.

[EricWF]

I think this is llvm.org/PR37398

I tried fixing this a while back in r332028 but the fix got reverted for causing llvm.org/PR37545.

[rsmith]

In D62429#1518602, @EricWF wrote:

I think this is llvm.org/PR37398

I tried fixing this a while back in r332028 but the fix got reverted for causing llvm.org/PR37545.

Looks like I had a fix for that in https://reviews.llvm.org/D47092 but it seems we decided to not go that way because the issue is really an ASan instrumentation bug and should be fixed in ASan instead... so I guess this is blocked on a fix to ASan :-/

lib/CodeGen/ItaniumCXXABI.cpp

#include "clang/AST/TypeNodes.def"
 
   Fields.push_back(VTable);
 }
 
 /// 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) {
-  // 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;
-
   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 the class is incomplete, we're forming the pointee of an incomplete
+      // pointer typeinfo. This linkage will only be used for the type name;
+      // give it linkonce_odr linkage.
+      if (!RD->isCompleteDefinition())
+        return llvm::GlobalValue::LinkOnceODRLinkage;
       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() &&
if (RD && RD->hasAttr<DLLExportAttr>())
       DLLStorageClass = llvm::GlobalValue::DLLExportStorageClass;
   }
 
   return BuildTypeInfo(Ty, Linkage, llvmVisibility, DLLStorageClass);
 }
 
 llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
       QualType Ty,
-      llvm::GlobalVariable::LinkageTypes Linkage,
-      llvm::GlobalValue::VisibilityTypes Visibility,
+      llvm::GlobalVariable::LinkageTypes TypeNameLinkage,
+      llvm::GlobalValue::VisibilityTypes TypeNameVisibility,
       llvm::GlobalValue::DLLStorageClassTypes DLLStorageClass) {
   // Add the vtable pointer.
   BuildVTablePointer(cast<Type>(Ty));
 
   // And the name.
-  llvm::GlobalVariable *TypeName = GetAddrOfTypeName(Ty, Linkage);
+  llvm::GlobalVariable *TypeName = GetAddrOfTypeName(Ty, TypeNameLinkage);
   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, TypeNameLinkage);
   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.
     TypeNameField = llvm::ConstantExpr::getPtrToInt(TypeName, CGM.Int64Ty);
     llvm::Constant *flag =
         llvm::ConstantInt::get(CGM.Int64Ty, ((uint64_t)1) << 63);
     TypeNameField = llvm::ConstantExpr::getAdd(TypeNameField, flag);
     TypeNameField =
#include "clang/AST/TypeNodes.def"
 
   case Type::Atomic:
     // No fields, at least for the moment.
     break;
   }
 
   llvm::Constant *Init = llvm::ConstantStruct::getAnon(Fields);
 
+  // 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.
+  //
+  //   Two abi::__pbase_type_info objects can always be compared for equality
+  //   (i.e. of the types represented) or ordering by comparison of their name
+  //   NTBS addresses. In addition, unless either or both have either of the
+  //   incomplete flags set, equality can be tested by comparing the type_info
+  //   addresses.
+  llvm::GlobalVariable::LinkageTypes TypeInfoLinkage = TypeNameLinkage;
+  llvm::GlobalValue::VisibilityTypes TypeInfoVisibility = TypeNameVisibility;
+  if (ContainsIncompleteClassType(Ty)) {
+    TypeInfoLinkage = llvm::GlobalVariable::InternalLinkage;
+    TypeInfoVisibility = llvm::GlobalValue::DefaultVisibility;
+  }
+
   SmallString<256> Name;
   llvm::raw_svector_ostream Out(Name);
   CGM.getCXXABI().getMangleContext().mangleCXXRTTI(Ty, Out);
   llvm::Module &M = CGM.getModule();
   llvm::GlobalVariable *OldGV = M.getNamedGlobal(Name);
   llvm::GlobalVariable *GV =
       new llvm::GlobalVariable(M, Init->getType(),
-                               /*Constant=*/true, Linkage, Init, Name);
+                               /*Constant=*/true, TypeInfoLinkage, Init, Name);
 
   // If there's already an old global variable, replace it with the new one.
   if (OldGV) {
     GV->takeName(OldGV);
     llvm::Constant *NewPtr =
       llvm::ConstantExpr::getBitCast(GV, OldGV->getType());
     OldGV->replaceAllUsesWith(NewPtr);
     OldGV->eraseFromParent();
#include "clang/AST/TypeNodes.def"
   // of RTTI generally require class types to be complete, even when
   // manipulating pointers to those class types.  This allows the
   // implementation of dynamic_cast to rely on address equality tests,
   // which is much faster.
 
   // All of this is to say that it's important that both the type_info
   // object and the type_info name be uniqued when weakly emitted.
 
-  TypeName->setVisibility(Visibility);
+  TypeName->setVisibility(TypeNameVisibility);
   CGM.setDSOLocal(TypeName);
 
-  GV->setVisibility(Visibility);
+  GV->setVisibility(TypeInfoVisibility);
   CGM.setDSOLocal(GV);
 
   TypeName->setDLLStorageClass(DLLStorageClass);
   GV->setDLLStorageClass(DLLStorageClass);
 
   return llvm::ConstantExpr::getBitCast(GV, CGM.Int8PtrTy);
 }
 

test/CodeGenCXX/rtti-linkage.cpp

 // 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
 
 #include <typeinfo>
 
-// CHECK-BOTH: _ZTSP1C = internal constant
-// CHECK-BOTH: _ZTS1C = internal constant
+// CHECK: _ZTSP1C = linkonce_odr constant
+// CHECK-WITH-HIDDEN: _ZTSP1C = linkonce_odr hidden constant
+// CHECK: _ZTS1C = linkonce_odr constant
+// CHECK-WITH-HIDDEN: _ZTS1C = linkonce_odr hidden constant
 // CHECK-BOTH: _ZTI1C = internal constant
 // CHECK-BOTH: _ZTIP1C = internal constant
-// CHECK-BOTH: _ZTSPP1C = internal constant
+// CHECK: _ZTSPP1C = linkonce_odr constant
+// CHECK-WITH-HIDDEN: _ZTSPP1C = linkonce_odr hidden constant
 // CHECK-BOTH: _ZTIPP1C = internal constant
-// CHECK-BOTH: _ZTSM1Ci = internal constant
+// CHECK: _ZTSM1Ci = linkonce_odr constant
+// CHECK-WITH-HIDDEN: _ZTSM1Ci = linkonce_odr hidden constant
 // CHECK-BOTH: _ZTIM1Ci = internal constant
-// CHECK-BOTH: _ZTSPM1Ci = internal constant
+// CHECK: _ZTSPM1Ci = linkonce_odr constant
+// CHECK-WITH-HIDDEN: _ZTSPM1Ci = linkonce_odr hidden constant
 // CHECK-BOTH: _ZTIPM1Ci = internal constant
-// CHECK-BOTH: _ZTSM1CS_ = internal constant
+// CHECK: _ZTSM1CS_ = linkonce_odr constant
+// CHECK-WITH-HIDDEN: _ZTSM1CS_ = linkonce_odr hidden constant
 // CHECK-BOTH: _ZTIM1CS_ = internal constant
-// CHECK-BOTH: _ZTSM1CPS_ = internal constant
+// CHECK: _ZTSM1CPS_ = linkonce_odr constant
+// CHECK-WITH-HIDDEN: _ZTSM1CPS_ = linkonce_odr hidden constant
 // CHECK-BOTH: _ZTIM1CPS_ = internal constant
-// CHECK-BOTH: _ZTSM1A1C = internal constant
+// CHECK: _ZTSM1A1C = linkonce_odr constant
+// CHECK-WITH-HIDDEN: _ZTSM1A1C = linkonce_odr hidden 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: _ZTSM1AP1C = linkonce_odr constant
+// CHECK-WITH-HIDDEN: _ZTSM1AP1C = linkonce_odr hidden constant
 // CHECK-BOTH: _ZTIM1AP1C = internal constant
 
 // CHECK-WITH-HIDDEN: _ZTSFN12_GLOBAL__N_11DEvE = internal constant
 // CHECK-WITH-HIDDEN: @_ZTSPK2T4 = linkonce_odr hidden constant 
 // CHECK-WITH-HIDDEN: @_ZTS2T4 = linkonce_odr hidden constant 
 // CHECK-WITH-HIDDEN: @_ZTI2T4 = linkonce_odr hidden constant 
 // CHECK-WITH-HIDDEN: @_ZTIPK2T4 = linkonce_odr hidden constant 
 // CHECK-WITH-HIDDEN: @_ZTSZ2t5vE1A = internal constant