This is an archive of the discontinued LLVM Phabricator instance.

CodeGen: Fix LLVM assertion if Swift and Clang emit Objective-C class reference in same LLVM module
Needs ReviewPublic

Authored by slavapestov on Oct 21 2015, 1:00 PM.

Details

Reviewers
rjmccall
Summary

This patch fixes an issue that can come up when Swift and Clang are emitting declarations into the same LLVM module.

Objective-C class literals result in the emission of a global variable named OBJC_CLASS_$_<ClassName>. Clang and Swift emit this variable with different types, resulting in an LLVM assertion firing. This patch changes Clang to be more resilient in the case where the global has already been emitted, wrapping it in a cast constexpr instead of crashing.

Diff Detail

Event Timeline

slavapestov retitled this revision from to CodeGen: Fix LLVM assertion if Swift and Clang emit Objective-C class reference in same LLVM module.
slavapestov updated this object.
slavapestov added a reviewer: cfe-commits.
slavapestov set the repository for this revision to rL LLVM.
slavapestov edited reviewers, added: rjmccall; removed: cfe-commits.Oct 21 2015, 1:02 PM
slavapestov added a subscriber: cfe-commits.
rjmccall edited edge metadata.Oct 21 2015, 1:18 PM

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.

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@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);

}