Skip to content

Commit

Permalink
[clang][dataflow] Fix bug in Value comparison.
Browse files Browse the repository at this point in the history
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 llvm#76459.
  • Loading branch information
ymand committed Jan 16, 2024
1 parent fc64a73 commit 7a41ed5
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 6 deletions.
14 changes: 11 additions & 3 deletions clang/lib/Analysis/FlowSensitive/Value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,17 @@ static bool areEquivalentIndirectionValues(const Value &Val1,
}

bool areEquivalentValues(const Value &Val1, const Value &Val2) {
return &Val1 == &Val2 || (Val1.getKind() == Val2.getKind() &&
(isa<TopBoolValue>(&Val1) ||
areEquivalentIndirectionValues(Val1, Val2)));
if (&Val1 == &Val2)
return true;
if (Val1.getKind() != Val2.getKind())
return false;
// If values are distinct and have properties, we don't consider them equal,
// leaving equality up to the user model.
if (!Val1.properties().empty() || !Val2.properties().empty())
return false;
if (isa<TopBoolValue>(&Val1))
return true;
return areEquivalentIndirectionValues(Val1, Val2);
}

raw_ostream &operator<<(raw_ostream &OS, const Value &Val) {
Expand Down
30 changes: 27 additions & 3 deletions clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,40 @@ TEST(ValueTest, TopsEquivalent) {
EXPECT_TRUE(areEquivalentValues(V2, V1));
}

TEST(ValueTest, EquivalentValuesWithDifferentPropsEquivalent) {
// The framework does not (currently) consider equivalence for values with
// properties, leaving such to individual analyses.
TEST(ValueTest, ValuesWithSamePropsDifferent) {
Arena A;
TopBoolValue Prop(A.makeAtomRef(Atom(0)));
TopBoolValue V1(A.makeAtomRef(Atom(2)));
TopBoolValue V2(A.makeAtomRef(Atom(3)));
V1.setProperty("foo", Prop);
V2.setProperty("foo", Prop);
EXPECT_FALSE(areEquivalentValues(V1, V2));
EXPECT_FALSE(areEquivalentValues(V2, V1));
}

TEST(ValueTest, ValuesWithDifferentPropsDifferent) {
Arena A;
TopBoolValue Prop1(A.makeAtomRef(Atom(0)));
TopBoolValue Prop2(A.makeAtomRef(Atom(1)));
TopBoolValue V1(A.makeAtomRef(Atom(2)));
TopBoolValue V2(A.makeAtomRef(Atom(3)));
V1.setProperty("foo", Prop1);
V2.setProperty("bar", Prop2);
EXPECT_TRUE(areEquivalentValues(V1, V2));
EXPECT_TRUE(areEquivalentValues(V2, V1));
EXPECT_FALSE(areEquivalentValues(V1, V2));
EXPECT_FALSE(areEquivalentValues(V2, V1));
}

TEST(ValueTest, ValuesWithDifferentNumberPropsDifferent) {
Arena A;
TopBoolValue Prop(A.makeAtomRef(Atom(0)));
TopBoolValue V1(A.makeAtomRef(Atom(2)));
TopBoolValue V2(A.makeAtomRef(Atom(3)));
// Only set a property on `V1`.
V1.setProperty("foo", Prop);
EXPECT_FALSE(areEquivalentValues(V1, V2));
EXPECT_FALSE(areEquivalentValues(V2, V1));
}

TEST(ValueTest, DifferentKindsNotEquivalent) {
Expand Down

0 comments on commit 7a41ed5

Please # to comment.