Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
rnk added a comment. Right, I understand the behavior change, I'm just wondering why it results in link failures. There isn't a ton of public info about how ObjC++ EH interacts with C++ EH. Repository: rL LLVM http://reviews.llvm.org/D12743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
vsk added a comment.
I think 'optimization' is a bit of a misnomer. There's a comment in this code
that reads: "Can't do the optimization if it has non-C++ uses", so that's why I
picked up the word. Without SimplifyPersonality(), some objective c++ code can
no longer link against c++ libraries.
Consider the following test:
$ echo "#include " "\n" 'void foo() { throw
std::runtime_error("foo"); }' | clang -x objective-cxx -S -emit-llvm - -o - |
grep personality
After we moved personality from landingpadinst to functions, the output of the
test changes (from __gxx_personality to __objc_personality). This commit just
takes us back to the original behavior.
The original comment, "Otherwise, it has to be a landingpad instruction.", is
wrong after David's patch.
http://reviews.llvm.org/D12743
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
rnk added a subscriber: rnk.
rnk added a comment.
I'm confused. I thought SimplifyPersonalityFunction was an optimization, but
somehow it caused link failures? Why do you think this was an ABI break?
Comment at: test/CodeGenObjCXX/exception-cxx.mm:8
@@ +7,3 @@
+ throw 0;
+} catch (...) {
+ return;
Don't you want to test the 'catch (int e)' case? That introduces interesting
uses of the selector that catch-all doesn't have.
http://reviews.llvm.org/D12743
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
vsk marked an inline comment as done. vsk added a comment. Addressed Reid's comment w.r.t exception object type. Repository: rL LLVM http://reviews.llvm.org/D12743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
rjmccall added a comment. In http://reviews.llvm.org/D12743#244375, @vsk wrote: > Ah, ok. We have some objective-c++ code which calls into a boost routine > which throws an exception. That results in an undefined reference to > ___objc_personality_v0, because the boost library we linked against doesn't > have ___objc_personality_v0. > > Should the compiler have found the ___objc_personality_v0 symbol in this case > regardless? It's pretty straightforward. Sometimes people write code in ObjC++ files that's really pure C++. Such code is generally compiled with -fexceptions because that's the default, and so it has cleanups, and those cleanups require us to pick a personality function. When they do so, they generally don't link against the ObjC runtime, and so __objc_personality_v0 isn't found. The workaround here is to recognize that they're not actually catching ObjC exception types and just quietly degrade to use the C++ personality. It is probably technically an optimization, because it removes some overhead from double-parsing the exception tables (because the ObjC personality delegates to the C++ personality, instead of being tightly integrated with it), but that's not the real reason we do it. Repository: rL LLVM http://reviews.llvm.org/D12743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
This revision was automatically updated to reflect the committed changes.
Closed by commit rL247437: [test] Specify exception object type in two tests
(authored by vedantk).
Changed prior to commit:
http://reviews.llvm.org/D12743?vs=34408=34560#toc
Repository:
rL LLVM
http://reviews.llvm.org/D12743
Files:
cfe/trunk/test/CodeGenObjCXX/exception-cxx.mm
cfe/trunk/test/CodeGenObjCXX/personality-abuse.mm
Index: cfe/trunk/test/CodeGenObjCXX/personality-abuse.mm
===
--- cfe/trunk/test/CodeGenObjCXX/personality-abuse.mm
+++ cfe/trunk/test/CodeGenObjCXX/personality-abuse.mm
@@ -11,7 +11,7 @@
void foo() {
try {
throw 0;
- } catch (...) {
+ } catch (int e) {
return;
}
}
Index: cfe/trunk/test/CodeGenObjCXX/exception-cxx.mm
===
--- cfe/trunk/test/CodeGenObjCXX/exception-cxx.mm
+++ cfe/trunk/test/CodeGenObjCXX/exception-cxx.mm
@@ -5,7 +5,7 @@
void foo() {
try {
throw 0;
-} catch (...) {
+} catch (int e) {
return;
}
}
Index: cfe/trunk/test/CodeGenObjCXX/personality-abuse.mm
===
--- cfe/trunk/test/CodeGenObjCXX/personality-abuse.mm
+++ cfe/trunk/test/CodeGenObjCXX/personality-abuse.mm
@@ -11,7 +11,7 @@
void foo() {
try {
throw 0;
- } catch (...) {
+ } catch (int e) {
return;
}
}
Index: cfe/trunk/test/CodeGenObjCXX/exception-cxx.mm
===
--- cfe/trunk/test/CodeGenObjCXX/exception-cxx.mm
+++ cfe/trunk/test/CodeGenObjCXX/exception-cxx.mm
@@ -5,7 +5,7 @@
void foo() {
try {
throw 0;
-} catch (...) {
+} catch (int e) {
return;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
vsk accepted this revision. vsk added a reviewer: vsk. vsk marked an inline comment as done. vsk added a comment. This revision is now accepted and ready to land. Committed r247421 http://reviews.llvm.org/D12743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
rjmccall added a comment. LGTM, thanks. http://reviews.llvm.org/D12743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
vsk created this revision.
vsk added reviewers: majnemer, rjmccall.
vsk added a subscriber: cfe-commits.
When personality function references were moved from LandingPadInst to
Function, we forgot to update SimplifyPersonality(). This is an
optimization which replaces personality functions with the default C++
personality if it has only C++-style uses. As a result, some ObjC++
projects could no longer link against C++ libraries (seeing as the
exception ABI had effectively changed).
http://reviews.llvm.org/D12743
Files:
lib/CodeGen/CGException.cpp
test/CodeGenObjCXX/exception-cxx.mm
Index: test/CodeGenObjCXX/exception-cxx.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/exception-cxx.mm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin10 -emit-llvm -fcxx-exceptions -fexceptions -fobjc-exceptions -o - %s | FileCheck %s
+
+// rdar://problem/22155434
+namespace test0 {
+ void foo() {
+try {
+ throw 0;
+} catch (...) {
+ return;
+}
+ }
+// CHECK: define void @_ZN5test03fooEv() #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
+}
Index: lib/CodeGen/CGException.cpp
===
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -229,6 +229,36 @@
return llvm::ConstantExpr::getBitCast(Fn, CGM.Int8PtrTy);
}
+/// Check whether a landingpad instruction uses any non-C++ features.
+static bool LandingPadHasOnlyCXXUses(llvm::LandingPadInst *LPI) {
+ for (unsigned I = 0, E = LPI->getNumClauses(); I != E; ++I) {
+// Look for something that would've been returned by the ObjC
+// runtime's GetEHType() method.
+llvm::Value *Val = LPI->getClause(I)->stripPointerCasts();
+if (LPI->isCatch(I)) {
+ // Check if the catch value has the ObjC prefix.
+ if (llvm::GlobalVariable *GV = dyn_cast(Val))
+// ObjC EH selector entries are always global variables with
+// names starting like this.
+if (GV->getName().startswith("OBJC_EHTYPE"))
+ return false;
+} else {
+ // Check if any of the filter values have the ObjC prefix.
+ llvm::Constant *CVal = cast(Val);
+ for (llvm::User::op_iterator
+ II = CVal->op_begin(), IE = CVal->op_end(); II != IE; ++II) {
+if (llvm::GlobalVariable *GV =
+cast((*II)->stripPointerCasts()))
+ // ObjC EH selector entries are always global variables with
+ // names starting like this.
+ if (GV->getName().startswith("OBJC_EHTYPE"))
+return false;
+ }
+}
+ }
+ return true;
+}
+
/// Check whether a personality function could reasonably be swapped
/// for a C++ personality function.
static bool PersonalityHasOnlyCXXUses(llvm::Constant *Fn) {
@@ -241,34 +271,14 @@
continue;
}
-// Otherwise, it has to be a landingpad instruction.
-llvm::LandingPadInst *LPI = dyn_cast(U);
-if (!LPI) return false;
-
-for (unsigned I = 0, E = LPI->getNumClauses(); I != E; ++I) {
- // Look for something that would've been returned by the ObjC
- // runtime's GetEHType() method.
- llvm::Value *Val = LPI->getClause(I)->stripPointerCasts();
- if (LPI->isCatch(I)) {
-// Check if the catch value has the ObjC prefix.
-if (llvm::GlobalVariable *GV = dyn_cast(Val))
- // ObjC EH selector entries are always global variables with
- // names starting like this.
- if (GV->getName().startswith("OBJC_EHTYPE"))
-return false;
- } else {
-// Check if any of the filter values have the ObjC prefix.
-llvm::Constant *CVal = cast(Val);
-for (llvm::User::op_iterator
- II = CVal->op_begin(), IE = CVal->op_end(); II != IE; ++II) {
- if (llvm::GlobalVariable *GV =
- cast((*II)->stripPointerCasts()))
-// ObjC EH selector entries are always global variables with
-// names starting like this.
-if (GV->getName().startswith("OBJC_EHTYPE"))
- return false;
-}
- }
+// Otherwise it must be a function.
+llvm::Function *F = dyn_cast(U);
+if (!U) return false;
+
+for (auto BB = F->begin(), E = F->end(); BB != E; ++BB) {
+ if (BB->isLandingPad())
+if (!LandingPadHasOnlyCXXUses(BB->getLandingPadInst()))
+ return false;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst
vsk updated the summary for this revision.
vsk updated this revision to Diff 34408.
vsk added a comment.
Thanks for the review.
- Addressed if (!U) bug.
- Added test which loads a personality function, confirmed that we crash
without the proper `if (!F)' check.
http://reviews.llvm.org/D12743
Files:
lib/CodeGen/CGException.cpp
test/CodeGenObjCXX/exception-cxx.mm
test/CodeGenObjCXX/personality-abuse.mm
Index: test/CodeGenObjCXX/personality-abuse.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/personality-abuse.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin10 -emit-llvm -fcxx-exceptions -fexceptions -fobjc-exceptions -o - %s | FileCheck %s
+
+extern "C" {
+ int __objc_personality_v0();
+}
+
+void *abuse_personality_func() {
+ return (void *)&__objc_personality_v0;
+}
+
+void foo() {
+ try {
+throw 0;
+ } catch (...) {
+return;
+ }
+}
+
+// CHECK: define void @_Z3foov() #1 personality i8* bitcast (i32 ()* @__objc_personality_v0 to i8*)
Index: test/CodeGenObjCXX/exception-cxx.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/exception-cxx.mm
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin10 -emit-llvm -fcxx-exceptions -fexceptions -fobjc-exceptions -o - %s | FileCheck %s
+
+// rdar://problem/22155434
+namespace test0 {
+ void foo() {
+try {
+ throw 0;
+} catch (...) {
+ return;
+}
+ }
+// CHECK: define void @_ZN5test03fooEv() #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
+}
Index: lib/CodeGen/CGException.cpp
===
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -229,6 +229,36 @@
return llvm::ConstantExpr::getBitCast(Fn, CGM.Int8PtrTy);
}
+/// Check whether a landingpad instruction only uses C++ features.
+static bool LandingPadHasOnlyCXXUses(llvm::LandingPadInst *LPI) {
+ for (unsigned I = 0, E = LPI->getNumClauses(); I != E; ++I) {
+// Look for something that would've been returned by the ObjC
+// runtime's GetEHType() method.
+llvm::Value *Val = LPI->getClause(I)->stripPointerCasts();
+if (LPI->isCatch(I)) {
+ // Check if the catch value has the ObjC prefix.
+ if (llvm::GlobalVariable *GV = dyn_cast(Val))
+// ObjC EH selector entries are always global variables with
+// names starting like this.
+if (GV->getName().startswith("OBJC_EHTYPE"))
+ return false;
+} else {
+ // Check if any of the filter values have the ObjC prefix.
+ llvm::Constant *CVal = cast(Val);
+ for (llvm::User::op_iterator
+ II = CVal->op_begin(), IE = CVal->op_end(); II != IE; ++II) {
+if (llvm::GlobalVariable *GV =
+cast((*II)->stripPointerCasts()))
+ // ObjC EH selector entries are always global variables with
+ // names starting like this.
+ if (GV->getName().startswith("OBJC_EHTYPE"))
+return false;
+ }
+}
+ }
+ return true;
+}
+
/// Check whether a personality function could reasonably be swapped
/// for a C++ personality function.
static bool PersonalityHasOnlyCXXUses(llvm::Constant *Fn) {
@@ -241,34 +271,14 @@
continue;
}
-// Otherwise, it has to be a landingpad instruction.
-llvm::LandingPadInst *LPI = dyn_cast(U);
-if (!LPI) return false;
-
-for (unsigned I = 0, E = LPI->getNumClauses(); I != E; ++I) {
- // Look for something that would've been returned by the ObjC
- // runtime's GetEHType() method.
- llvm::Value *Val = LPI->getClause(I)->stripPointerCasts();
- if (LPI->isCatch(I)) {
-// Check if the catch value has the ObjC prefix.
-if (llvm::GlobalVariable *GV = dyn_cast(Val))
- // ObjC EH selector entries are always global variables with
- // names starting like this.
- if (GV->getName().startswith("OBJC_EHTYPE"))
-return false;
- } else {
-// Check if any of the filter values have the ObjC prefix.
-llvm::Constant *CVal = cast(Val);
-for (llvm::User::op_iterator
- II = CVal->op_begin(), IE = CVal->op_end(); II != IE; ++II) {
- if (llvm::GlobalVariable *GV =
- cast((*II)->stripPointerCasts()))
-// ObjC EH selector entries are always global variables with
-// names starting like this.
-if (GV->getName().startswith("OBJC_EHTYPE"))
- return false;
-}
- }
+// Otherwise it must be a function.
+llvm::Function *F = dyn_cast(U);
+if (!F) return false;
+
+for (auto BB = F->begin(), E = F->end(); BB != E; ++BB) {
+ if (BB->isLandingPad())
+if (!LandingPadHasOnlyCXXUses(BB->getLandingPadInst()))
+ return false;
}
}
