[PATCH] D13954: CodeGen: Fix LLVM assertion if Swift and Clang emit Objective-C class reference in same LLVM module
slavapestov created this revision.
slavapestov added a reviewer: cfe-commits.
slavapestov set the repository for this revision to rL LLVM.
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_$_. 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.
Repository:
rL LLVM
http://reviews.llvm.org/D13954
Files:
lib/CodeGen/CGObjCGNU.cpp
lib/CodeGen/CGObjCMac.cpp
lib/CodeGen/CGObjCRuntime.h
lib/CodeGen/CodeGenModule.cpp
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2739,7 +2739,9 @@
std::string str =
StringClass.empty() ? "OBJC_CLASS_$_NSConstantString"
: "OBJC_CLASS_$_" + StringClass;
- GV = getObjCRuntime().GetClassGlobal(str);
+ GV = getObjCRuntime().GetClassGlobal(str,
+ /*ForDefinition=*/false,
+ /*Weak=*/false);
// Make sure the result is of the correct type.
llvm::Type *PTy = llvm::PointerType::getUnqual(Ty);
V = llvm::ConstantExpr::getBitCast(GV, PTy);
Index: lib/CodeGen/CGObjCRuntime.h
===
--- lib/CodeGen/CGObjCRuntime.h
+++ lib/CodeGen/CGObjCRuntime.h
@@ -268,8 +268,9 @@
const CodeGen::CGBlockInfo &blockInfo) = 0;
virtual llvm::Constant *BuildByrefLayout(CodeGen::CodeGenModule &CGM,
QualType T) = 0;
- virtual llvm::GlobalVariable *GetClassGlobal(const std::string &Name,
- bool Weak = false) = 0;
+ virtual llvm::Constant *GetClassGlobal(const std::string &Name,
+ bool ForDefinition,
+ bool Weak) = 0;
struct MessageSendInfo {
const CGFunctionInfo &CallInfo;
Index: lib/CodeGen/CGObjCMac.cpp
===
--- lib/CodeGen/CGObjCMac.cpp
+++ lib/CodeGen/CGObjCMac.cpp
@@ -1260,8 +1260,9 @@
/// GetClassGlobal - Return the global variable for the Objective-C
/// class of the given name.
- llvm::GlobalVariable *GetClassGlobal(const std::string &Name,
- bool Weak = false) override {
+ llvm::Constant *GetClassGlobal(const std::string &Name,
+ bool ForDefinition,
+ bool Weak) override {
llvm_unreachable("CGObjCMac::GetClassGlobal");
}
};
@@ -1362,8 +1363,9 @@
/// GetClassGlobal - Return the global variable for the Objective-C
/// class of the given name.
- llvm::GlobalVariable *GetClassGlobal(const std::string &Name,
- bool Weak = false) override;
+ llvm::Constant *GetClassGlobal(const std::string &Name,
+ bool ForDefinition,
+ bool Weak) override;
/// EmitClassRef - Return a Value*, of type ObjCTypes.ClassPtrTy,
/// for the given class reference.
@@ -5686,7 +5688,12 @@
llvm::PointerType::getUnqual(ObjCTypes.ImpnfABITy));
llvm::Constant *Init = llvm::ConstantStruct::get(ObjCTypes.ClassnfABITy,
Values);
- llvm::GlobalVariable *GV = GetClassGlobal(ClassName, Weak);
+ llvm::GlobalVariable *GV = cast(
+ GetClassGlobal(ClassName,
+ /*ForDefinition=*/true,
+ Weak));
+ if (Init->getType() != GV->getValueType())
+Init = llvm::ConstantExpr::getBitCast(Init, GV->getValueType());
GV->setInitializer(Init);
GV->setSection("__DATA, __objc_data");
GV->setAlignment(
@@ -5755,7 +5762,7 @@
llvm::SmallString<64> ObjCClassName(getClassSymbolPrefix());
llvm::SmallString<64> TClassName;
- llvm::GlobalVariable *SuperClassGV, *IsAGV;
+ llvm::Constant *SuperClassGV, *IsAGV;
// Build the flags for the metaclass.
bool classIsHidden =
@@ -5777,10 +5784,12 @@
TClassName = ObjCClassName;
TClassName += ClassName;
SuperClassGV = GetClassGlobal(TClassName.str(),
+ /*ForDefinition=*/false,
ID->getClassInterface()->isWeakImported());
TClassName = ObjCMetaClassName;
TClassName += ClassName;
IsAGV = GetClassGlobal(TClassName.str(),
+ /*ForDefinition=*/false,
ID
Re: [PATCH] D13954: CodeGen: Fix LLVM assertion if Swift and Clang emit Objective-C class reference in same LLVM module
slavapestov added a subscriber: slavapestov.
slavapestov added a comment.
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
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);
}
Repository:
rL LLVM
http://reviews.llvm.org/D13954
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13954: CodeGen: Fix LLVM assertion if Swift and Clang emit Objective-C class reference in same LLVM module
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
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 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
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
