Skip to content

Commit 7a41ed5

Browse files
committed
[clang][dataflow] Fix bug in Value comparison.
Makes value equivalence require that the values have no properties, except in the case of equivalence by pointer equality (if the pointers are equal, nothing else is checked). Fixes issue #76459.
1 parent fc64a73 commit 7a41ed5

File tree

2 files changed

+38
-6
lines changed

2 files changed

+38
-6
lines changed

clang/lib/Analysis/FlowSensitive/Value.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,17 @@ static bool areEquivalentIndirectionValues(const Value &Val1,
2727
}
2828

2929
bool areEquivalentValues(const Value &Val1, const Value &Val2) {
30-
return &Val1 == &Val2 || (Val1.getKind() == Val2.getKind() &&
31-
(isa<TopBoolValue>(&Val1) ||
32-
areEquivalentIndirectionValues(Val1, Val2)));
30+
if (&Val1 == &Val2)
31+
return true;
32+
if (Val1.getKind() != Val2.getKind())
33+
return false;
34+
// If values are distinct and have properties, we don't consider them equal,
35+
// leaving equality up to the user model.
36+
if (!Val1.properties().empty() || !Val2.properties().empty())
37+
return false;
38+
if (isa<TopBoolValue>(&Val1))
39+
return true;
40+
return areEquivalentIndirectionValues(Val1, Val2);
3341
}
3442

3543
raw_ostream &operator<<(raw_ostream &OS, const Value &Val) {

clang/unittests/Analysis/FlowSensitive/ValueTest.cpp

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,40 @@ TEST(ValueTest, TopsEquivalent) {
4545
EXPECT_TRUE(areEquivalentValues(V2, V1));
4646
}
4747

48-
TEST(ValueTest, EquivalentValuesWithDifferentPropsEquivalent) {
48+
// The framework does not (currently) consider equivalence for values with
49+
// properties, leaving such to individual analyses.
50+
TEST(ValueTest, ValuesWithSamePropsDifferent) {
51+
Arena A;
52+
TopBoolValue Prop(A.makeAtomRef(Atom(0)));
53+
TopBoolValue V1(A.makeAtomRef(Atom(2)));
54+
TopBoolValue V2(A.makeAtomRef(Atom(3)));
55+
V1.setProperty("foo", Prop);
56+
V2.setProperty("foo", Prop);
57+
EXPECT_FALSE(areEquivalentValues(V1, V2));
58+
EXPECT_FALSE(areEquivalentValues(V2, V1));
59+
}
60+
61+
TEST(ValueTest, ValuesWithDifferentPropsDifferent) {
4962
Arena A;
5063
TopBoolValue Prop1(A.makeAtomRef(Atom(0)));
5164
TopBoolValue Prop2(A.makeAtomRef(Atom(1)));
5265
TopBoolValue V1(A.makeAtomRef(Atom(2)));
5366
TopBoolValue V2(A.makeAtomRef(Atom(3)));
5467
V1.setProperty("foo", Prop1);
5568
V2.setProperty("bar", Prop2);
56-
EXPECT_TRUE(areEquivalentValues(V1, V2));
57-
EXPECT_TRUE(areEquivalentValues(V2, V1));
69+
EXPECT_FALSE(areEquivalentValues(V1, V2));
70+
EXPECT_FALSE(areEquivalentValues(V2, V1));
71+
}
72+
73+
TEST(ValueTest, ValuesWithDifferentNumberPropsDifferent) {
74+
Arena A;
75+
TopBoolValue Prop(A.makeAtomRef(Atom(0)));
76+
TopBoolValue V1(A.makeAtomRef(Atom(2)));
77+
TopBoolValue V2(A.makeAtomRef(Atom(3)));
78+
// Only set a property on `V1`.
79+
V1.setProperty("foo", Prop);
80+
EXPECT_FALSE(areEquivalentValues(V1, V2));
81+
EXPECT_FALSE(areEquivalentValues(V2, V1));
5882
}
5983

6084
TEST(ValueTest, DifferentKindsNotEquivalent) {

0 commit comments

Comments
 (0)