[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG60bd8cbc0c84: [analyzer][solver][NFC] Refactor how we detect
(dis)equalities (authored by vsavchenko).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105693/new/
https://reviews.llvm.org/D105693
Files:
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
clang/test/Analysis/equality_tracking.c
Index: clang/test/Analysis/equality_tracking.c
===
--- clang/test/Analysis/equality_tracking.c
+++ clang/test/Analysis/equality_tracking.c
@@ -219,3 +219,17 @@
if (c < 0)
;
}
+
+void implyDisequalityFromGT(int a, int b) {
+ if (a > b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+ }
+}
+
+void implyDisequalityFromLT(int a, int b) {
+ if (a < b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+ }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -684,58 +684,30 @@
// Equality/diseqiality abstraction
//===--===//
-/// A small helper structure representing symbolic equality.
+/// A small helper function for detecting symbolic (dis)equality.
///
/// Equality check can have different forms (like a == b or a - b) and this
/// class encapsulates those away if the only thing the user wants to check -
-/// whether it's equality/diseqiality or not and have an easy access to the
-/// compared symbols.
-struct EqualityInfo {
-public:
- SymbolRef Left, Right;
- // true for equality and false for disequality.
- bool IsEquality = true;
-
- void invert() { IsEquality = !IsEquality; }
- /// Extract equality information from the given symbol and the constants.
- ///
- /// This function assumes the following expression Sym + Adjustment != Int.
- /// It is a default because the most widespread case of the equality check
- /// is (A == B) + 0 != 0.
- static Optional extract(SymbolRef Sym, const llvm::APSInt &Int,
-const llvm::APSInt &Adjustment) {
-// As of now, the only equality form supported is Sym + 0 != 0.
-if (!Int.isNullValue() || !Adjustment.isNullValue())
- return llvm::None;
-
-return extract(Sym);
- }
- /// Extract equality information from the given symbol.
- static Optional extract(SymbolRef Sym) {
-return EqualityExtractor().Visit(Sym);
+/// whether it's equality/diseqiality or not.
+///
+/// \returns true if assuming this Sym to be true means equality of operands
+/// false if it means disequality of operands
+/// None otherwise
+Optional meansEquality(const SymSymExpr *Sym) {
+ switch (Sym->getOpcode()) {
+ case BO_Sub:
+// This case is: A - B != 0 -> disequality check.
+return false;
+ case BO_EQ:
+// This case is: A == B != 0 -> equality check.
+return true;
+ case BO_NE:
+// This case is: A != B != 0 -> diseqiality check.
+return false;
+ default:
+return llvm::None;
}
-
-private:
- class EqualityExtractor
- : public SymExprVisitor> {
- public:
-Optional VisitSymSymExpr(const SymSymExpr *Sym) const {
- switch (Sym->getOpcode()) {
- case BO_Sub:
-// This case is: A - B != 0 -> disequality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), false};
- case BO_EQ:
-// This case is: A == B != 0 -> equality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), true};
- case BO_NE:
-// This case is: A != B != 0 -> diseqiality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), false};
- default:
-return llvm::None;
- }
-}
- };
-};
+}
//===--===//
//Intersection functions
@@ -866,7 +838,13 @@
}
RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
-return VisitBinaryOperator(Sym);
+return intersect(
+RangeFactory,
+// If Sym is (dis)equality, we might have some information
+// on that in our equality classes data structure.
+getRangeForEqualities(Sym),
+// And we should always check what we can get from the operands.
+VisitBinaryOperator(Sym));
}
private:
@@ -907,9 +885,6 @@
// calculate the effective range set by intersecting the range set
// for A - B and the negated range set of B - A.
getRangeForNegatedSub(Sym),
-// If Sym is (dis)equality, we mi
[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:687 /// A small helper structure representing symbolic equality. /// martong wrote: > This is no longer a `structure`. Good catch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105693/new/ https://reviews.llvm.org/D105693 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities
vsavchenko updated this revision to Diff 358341.
vsavchenko marked an inline comment as done.
vsavchenko added a comment.
Fix comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105693/new/
https://reviews.llvm.org/D105693
Files:
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
clang/test/Analysis/equality_tracking.c
Index: clang/test/Analysis/equality_tracking.c
===
--- clang/test/Analysis/equality_tracking.c
+++ clang/test/Analysis/equality_tracking.c
@@ -219,3 +219,17 @@
if (c < 0)
;
}
+
+void implyDisequalityFromGT(int a, int b) {
+ if (a > b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+ }
+}
+
+void implyDisequalityFromLT(int a, int b) {
+ if (a < b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+ }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -684,58 +684,30 @@
// Equality/diseqiality abstraction
//===--===//
-/// A small helper structure representing symbolic equality.
+/// A small helper function for detecting symbolic (dis)equality.
///
/// Equality check can have different forms (like a == b or a - b) and this
/// class encapsulates those away if the only thing the user wants to check -
-/// whether it's equality/diseqiality or not and have an easy access to the
-/// compared symbols.
-struct EqualityInfo {
-public:
- SymbolRef Left, Right;
- // true for equality and false for disequality.
- bool IsEquality = true;
-
- void invert() { IsEquality = !IsEquality; }
- /// Extract equality information from the given symbol and the constants.
- ///
- /// This function assumes the following expression Sym + Adjustment != Int.
- /// It is a default because the most widespread case of the equality check
- /// is (A == B) + 0 != 0.
- static Optional extract(SymbolRef Sym, const llvm::APSInt &Int,
-const llvm::APSInt &Adjustment) {
-// As of now, the only equality form supported is Sym + 0 != 0.
-if (!Int.isNullValue() || !Adjustment.isNullValue())
- return llvm::None;
-
-return extract(Sym);
- }
- /// Extract equality information from the given symbol.
- static Optional extract(SymbolRef Sym) {
-return EqualityExtractor().Visit(Sym);
+/// whether it's equality/diseqiality or not.
+///
+/// \returns true if assuming this Sym to be true means equality of operands
+/// false if it means disequality of operands
+/// None otherwise
+Optional meansEquality(const SymSymExpr *Sym) {
+ switch (Sym->getOpcode()) {
+ case BO_Sub:
+// This case is: A - B != 0 -> disequality check.
+return false;
+ case BO_EQ:
+// This case is: A == B != 0 -> equality check.
+return true;
+ case BO_NE:
+// This case is: A != B != 0 -> diseqiality check.
+return false;
+ default:
+return llvm::None;
}
-
-private:
- class EqualityExtractor
- : public SymExprVisitor> {
- public:
-Optional VisitSymSymExpr(const SymSymExpr *Sym) const {
- switch (Sym->getOpcode()) {
- case BO_Sub:
-// This case is: A - B != 0 -> disequality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), false};
- case BO_EQ:
-// This case is: A == B != 0 -> equality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), true};
- case BO_NE:
-// This case is: A != B != 0 -> diseqiality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), false};
- default:
-return llvm::None;
- }
-}
- };
-};
+}
//===--===//
//Intersection functions
@@ -866,7 +838,13 @@
}
RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
-return VisitBinaryOperator(Sym);
+return intersect(
+RangeFactory,
+// If Sym is (dis)equality, we might have some information
+// on that in our equality classes data structure.
+getRangeForEqualities(Sym),
+// And we should always check what we can get from the operands.
+VisitBinaryOperator(Sym));
}
private:
@@ -907,9 +885,6 @@
// calculate the effective range set by intersecting the range set
// for A - B and the negated range set of B - A.
getRangeForNegatedSub(Sym),
-// If Sym is (dis)equality, we might have some information on that
-// in our equality classes data structure.
-getRangeForEqualities
[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
LGTM!
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:687
/// A small helper structure representing symbolic equality.
///
This is no longer a `structure`.
Comment at: clang/test/Analysis/equality_tracking.c:223-235
+void implyDisequalityFromGT(int a, int b) {
+ if (a > b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+ }
+}
+
Thanks, for the new test cases!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105693/new/
https://reviews.llvm.org/D105693
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities
vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus,
ASDenysPetrov, manas, RedDocMD.
Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin,
rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This patch simplifies the way we deal with (dis)equalities.
Due to the symmetry between constraint handler and range inferrer,
we can have very similar implementations of logic handling
questions about (dis)equality and assumptions involving (dis)equality.
It also helps us to remove one more visitor, and removes uncertainty
that we got all the right places to put `trackNE` and `trackEQ`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D105693
Files:
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
clang/test/Analysis/equality_tracking.c
Index: clang/test/Analysis/equality_tracking.c
===
--- clang/test/Analysis/equality_tracking.c
+++ clang/test/Analysis/equality_tracking.c
@@ -219,3 +219,17 @@
if (c < 0)
;
}
+
+void implyDisequalityFromGT(int a, int b) {
+ if (a > b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+ }
+}
+
+void implyDisequalityFromLT(int a, int b) {
+ if (a < b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+ }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -688,54 +688,26 @@
///
/// Equality check can have different forms (like a == b or a - b) and this
/// class encapsulates those away if the only thing the user wants to check -
-/// whether it's equality/diseqiality or not and have an easy access to the
-/// compared symbols.
-struct EqualityInfo {
-public:
- SymbolRef Left, Right;
- // true for equality and false for disequality.
- bool IsEquality = true;
-
- void invert() { IsEquality = !IsEquality; }
- /// Extract equality information from the given symbol and the constants.
- ///
- /// This function assumes the following expression Sym + Adjustment != Int.
- /// It is a default because the most widespread case of the equality check
- /// is (A == B) + 0 != 0.
- static Optional extract(SymbolRef Sym, const llvm::APSInt &Int,
-const llvm::APSInt &Adjustment) {
-// As of now, the only equality form supported is Sym + 0 != 0.
-if (!Int.isNullValue() || !Adjustment.isNullValue())
- return llvm::None;
-
-return extract(Sym);
- }
- /// Extract equality information from the given symbol.
- static Optional extract(SymbolRef Sym) {
-return EqualityExtractor().Visit(Sym);
+/// whether it's equality/diseqiality or not.
+///
+/// \returns true if assuming this Sym to be true means equality of operands
+/// false if it means disequality of operands
+/// None otherwise
+Optional meansEquality(const SymSymExpr *Sym) {
+ switch (Sym->getOpcode()) {
+ case BO_Sub:
+// This case is: A - B != 0 -> disequality check.
+return false;
+ case BO_EQ:
+// This case is: A == B != 0 -> equality check.
+return true;
+ case BO_NE:
+// This case is: A != B != 0 -> diseqiality check.
+return false;
+ default:
+return llvm::None;
}
-
-private:
- class EqualityExtractor
- : public SymExprVisitor> {
- public:
-Optional VisitSymSymExpr(const SymSymExpr *Sym) const {
- switch (Sym->getOpcode()) {
- case BO_Sub:
-// This case is: A - B != 0 -> disequality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), false};
- case BO_EQ:
-// This case is: A == B != 0 -> equality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), true};
- case BO_NE:
-// This case is: A != B != 0 -> diseqiality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), false};
- default:
-return llvm::None;
- }
-}
- };
-};
+}
//===--===//
//Intersection functions
@@ -866,7 +838,13 @@
}
RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
-return VisitBinaryOperator(Sym);
+return intersect(
+RangeFactory,
+// If Sym is (dis)equality, we might have some information
+// on that in our equality classes data structure.
+getRangeForEqualities(Sym),
+// And we should always check what we can get from the operands.
+VisitBinaryOperator(Sym));
}
private:
@@ -907,9 +885,6 @@
// calculate the effective rang
