-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[GlobalOpt] Fix unreachable ifunc globalopt crash (#157332) #157593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: Justin Riddell (Arghnews) ChangesAlso fixes (#131488) Unreachable case is triggering Full diff: https://github.com/llvm/llvm-project/pull/157593.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index d7edd1288309b..f88d51f443bcf 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -2551,7 +2551,8 @@ static bool OptimizeNonTrivialIFuncs(
}))
continue;
- assert(!Callees.empty() && "Expecting successful collection of versions");
+ if (Callees.empty())
+ continue;
LLVM_DEBUG(dbgs() << "Statically resolving calls to function "
<< Resolver->getName() << "\n");
diff --git a/llvm/test/Transforms/GlobalOpt/resolve-indirect-ifunc.ll b/llvm/test/Transforms/GlobalOpt/resolve-indirect-ifunc.ll
new file mode 100644
index 0000000000000..1f944a7bde19b
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/resolve-indirect-ifunc.ll
@@ -0,0 +1,11 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt --passes=globalopt -o - -S < %s | FileCheck %s
+
+define ptr @f1() partition "part1" {
+; CHECK-LABEL: define ptr @f1() partition "part1" {
+; CHECK-NEXT: unreachable
+;
+ unreachable
+}
+
+@i1 = ifunc void(), ptr @f1, partition "part2"
|
nikic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
1f439e4 to
05ea023
Compare
|
Fyi @nikic I don't have write access, will need someone to commit for me, thanks |
|
@hstk30-hw Please ensure that you drop any mentions from the commit description when merging changes. We'll be getting these notifications forever now. |
|
Okay, thanks for your reminder. I'll be more mindful next time |
Also fixes (#131488)
Unreachable case is triggering
Callees.empty()assert. Since this was originally acontinueanyway, have applied that as a fix and added a test case. Please let me know if there's a better way.Not sure who/how to get folks to review, tagging a few people (apologies if you're not the right person/this is the wrong way to do it, please let me know what to do in future if so)
@labrinea @dtcxzyw @nikic @fhahn