Reland [clang][dataflow] Fix unsupported types always being equal #131575
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously when the framework encountered unsupported values, they were always treated as equal when comparing with `==`, regardless of their actual values being different. Now the two sides are only equal if there's a Value assigned to them. Added handling for the special case of `nullptr == nullptr`, and preserved untyped `nullptr` having no value.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: Discookie (Discookie) ChangesRelands #129502. Previously when the framework encountered unsupported values (such as enum classes), they were always treated as equal when comparing with Added handling for the special case of Full diff: https://github.com/llvm/llvm-project/pull/131575.diff 2 Files Affected:
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 9c54eb16d2224..6403190723e69 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -60,7 +60,14 @@ static BoolValue &evaluateBooleanEquality(const Expr &LHS, const Expr &RHS,
Value *LHSValue = Env.getValue(LHS);
Value *RHSValue = Env.getValue(RHS);
- if (LHSValue == RHSValue)
+ // When two unsupported values are compared, both are nullptr. Only supported
+ // values should evaluate to equal.
+ if (LHSValue == RHSValue && LHSValue)
+ return Env.getBoolLiteralValue(true);
+
+ // Special case: `NullPtrLiteralExpr == itself`. When both sides are untyped
+ // nullptr, they do not have an assigned Value, but they compare equal.
+ if (LHS.getType()->isNullPtrType() && RHS.getType()->isNullPtrType())
return Env.getBoolLiteralValue(true);
if (auto *LHSBool = dyn_cast_or_null<BoolValue>(LHSValue))
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 0f731f4532535..f52b73dbbdc57 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4974,6 +4974,41 @@ TEST(TransferTest, IntegerLiteralEquality) {
});
}
+TEST(TransferTest, UnsupportedValueEquality) {
+ std::string Code = R"(
+ // An explicitly unsupported type by the framework.
+ enum class EC {
+ A,
+ B
+ };
+
+ void target() {
+ EC ec = EC::A;
+
+ bool unsupported_eq_same = (EC::A == EC::A);
+ bool unsupported_eq_other = (EC::A == EC::B);
+ bool unsupported_eq_var = (ec == EC::B);
+
+ (void)0; // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ // We do not model the values of unsupported types, so this
+ // seemingly-trivial case will not be true either.
+ EXPECT_TRUE(isa<AtomicBoolValue>(
+ getValueForDecl<BoolValue>(ASTCtx, Env, "unsupported_eq_same")));
+ EXPECT_TRUE(isa<AtomicBoolValue>(
+ getValueForDecl<BoolValue>(ASTCtx, Env, "unsupported_eq_other")));
+ EXPECT_TRUE(isa<AtomicBoolValue>(
+ getValueForDecl<BoolValue>(ASTCtx, Env, "unsupported_eq_var")));
+ });
+}
+
TEST(TransferTest, CorrelatedBranches) {
std::string Code = R"(
void target(bool B, bool C) {
|
Contributor
|
Hi, just checking if you wanted to merge this, or if there were any other issues. Thanks! |
Contributor
Author
|
No other issues, thanks for checking! |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/18905 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relands #129502.
Previously when the framework encountered unsupported values (such as enum classes), they were always treated as equal when comparing with
==, regardless of their actual values being different.Now the two sides are only equal if there's a Value assigned to them.
Added handling for the special case of
nullptr == nullptr, to preserve the behavior of untypednullptrhaving no value.