-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LLVM] Fix a bug in Intrinsic::getFnAttributes
#161248
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
[LLVM] Fix a bug in Intrinsic::getFnAttributes
#161248
Conversation
When an intrinsic has no function attributes, the current code will hit an assert in `getIntrinsicFnAttributeSet`. Fix this by checking for the sentinel `NoFunctionAttrsID` value and returning an empty attribute set.
@llvm/pr-subscribers-tablegen @llvm/pr-subscribers-llvm-ir Author: Rahul Joshi (jurahul) ChangesWhen an intrinsic has no function attributes, the current code will hit an assert in Full diff: https://github.com/llvm/llvm-project/pull/161248.diff 2 Files Affected:
diff --git a/llvm/unittests/IR/IntrinsicsTest.cpp b/llvm/unittests/IR/IntrinsicsTest.cpp
index 49af83609d98c..cfd99ed542162 100644
--- a/llvm/unittests/IR/IntrinsicsTest.cpp
+++ b/llvm/unittests/IR/IntrinsicsTest.cpp
@@ -189,4 +189,12 @@ TEST_F(IntrinsicsTest, InstrProfInheritance) {
}
}
+// Check that getFnAttributes for intrinsics that do not have any function
+// attributes correcty returns an empty set.
+TEST(IntrinsicAttributes, TestGetFnAttributesBug) {
+ using namespace Intrinsic;
+ LLVMContext Context;
+ AttributeSet AS = getFnAttributes(Context, experimental_guard);
+ EXPECT_FALSE(AS.hasAttributes());
+}
} // end namespace
diff --git a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
index 559868dd54efe..75dffb18fca5a 100644
--- a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
@@ -794,12 +794,15 @@ AttributeSet Intrinsic::getFnAttributes(LLVMContext &C, ID id) {{
if (id == 0)
return AttributeSet();
auto [FnAttrID, _] = unpackID(IntrinsicsToAttributesMap[id - 1]);
+ if (FnAttrID == {})
+ return AttributeSet();
return getIntrinsicFnAttributeSet(C, FnAttrID);
}
#endif // GET_INTRINSIC_ATTRIBUTES
)",
- UniqAttributesBitSize, MaxNumAttrs, NoFunctionAttrsID);
+ UniqAttributesBitSize, MaxNumAttrs, NoFunctionAttrsID,
+ NoFunctionAttrsID);
}
void IntrinsicEmitter::EmitIntrinsicToBuiltinMap(
|
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. Alternatively we could make the "no fn attrs" case less special by getIntrinsicFnAttributeSet() handle that by returning an empty attribute set -- but we'd probably still want an explicit check in getAttributes() to avoid pushing the FunctionIndex, so maybe that's not worthwhile.
Just want to mention that this bug was flagged by an AI assisted code review tool internally. |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/16422 Here is the relevant piece of the build log for the reference
|
When an intrinsic has no function attributes, the current code will hit an assert in
getIntrinsicFnAttributeSet
. Fix this by checking for the sentinelNoFunctionAttrsID
value and returning an empty attribute set.