[PATCH] D13954: CodeGen: Fix LLVM assertion if Swift and Clang emit Objective-C class reference in same LLVM module

[Slava Pestov via cfe-commits]

Unfortunately the asm label attribute doesn’t seem to work in this case, at least on Darwin. I get two symbols in the IR, with the asm label having the \01 prefix from MangleContext::mangleName():

@"\01OBJC_CLASS_$_NSNumber" = common global %struct.art_class* null, align 8                          
@"OBJC_CLASS_$_NSNumber" = external global %struct._class_t 

As for the diagnostic, is this what you had in mind?

commit 4d4f15ffc4d6a72357fa4cee8e2197a03afe6b51
Author: Slava Pestov <spestov at apple.com>
Date:   Fri Oct 23 23:57:56 2015 -0700

    CodeGen: More robust handling of type conflicts in GetClassGlobal()

diff --git a/lib/CodeGen/CGObjCMac.cpp b/lib/CodeGen/CGObjCMac.cpp
index 31e98b9..e5ef767 100644
--- a/lib/CodeGen/CGObjCMac.cpp
+++ b/lib/CodeGen/CGObjCMac.cpp
@@ -6692,16 +6692,31 @@ CGObjCNonFragileABIMac::GetClassGlobal(const std::string &Name,
 
   llvm::GlobalVariable *GV = CGM.getModule().getGlobalVariable(Name);
 
-  if (!GV)
-    GV = new llvm::GlobalVariable(CGM.getModule(), ObjCTypes.ClassnfABITy,
-                                  false, L, nullptr, Name);
+  // If it doesn't exist, create it with the right type.
+  if (!GV) {
+    return new llvm::GlobalVariable(CGM.getModule(), ObjCTypes.ClassnfABITy,
+                                    false, L, nullptr, Name);
+  }
 
   assert(GV->getLinkage() == L);
 
-  if (ForDefinition ||
-      GV->getValueType() == ObjCTypes.ClassnfABITy)
+  // If it already exists, we might need to bitcast.
+  if (GV->getValueType() == ObjCTypes.ClassnfABITy)
     return GV;
 
+  if (ForDefinition) {
+    DiagnosticsEngine &Diags = CGM.getDiags();
+    unsigned DiagID = Diags.getCustomDiagID(
+        DiagnosticsEngine::Error,
+        "global variable %0 already defined with wrong type");
+    Diags.Report(SourceLocation(), DiagID) << Name;
+
+    // Return a new global in this case, with the right type, since the caller
+    // doesn't expect a constexpr
+    return new llvm::GlobalVariable(CGM.getModule(), ObjCTypes.ClassnfABITy,
+                                    false, L, nullptr, Name);
+  }
+
   return llvm::ConstantExpr::getBitCast(GV, ObjCTypes.ClassnfABIPtrTy);
 }
 
> On Oct 21, 2015, at 1:18 PM, John McCall <rjmccall at gmail.com> wrote:
> 
> rjmccall added a comment.
> 
> It just occurred to me that there is a way to test this in Clang with the asm-label extension:
> 
>  int Foo_class asm("OBJC_CLASS_$_Foo");
> 
> Of course, you'll have to actually use it from somewhere, or define it, in order for it to actually show up in the IR and cause a conflict.
> 
> You should probably also test that we do something sane if the caller is making a definition but one already exists.  An error counts as "sane"; just make sure you return a GlobalVariable of the right type.  There's model code for this in GetOrCreateLLVMFunction, although it's okay for the diagnostic here to be worse than that one.
> 
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D13954
> 
> 
>