=?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com> Message-ID: <llvm.org/llvm/llvm-project/pull/83...@github.com> In-Reply-To:
https://github.com/NagyDonat created https://github.com/llvm/llvm-project/pull/83545 This comment-only change fixes a typo, clarifies some comments and includes some thoughts about the difficulties in resolving a certain FIXME. >From b15b88d1449d81e23629d79c914e80a4ee9e19eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com> Date: Fri, 1 Mar 2024 09:57:26 +0100 Subject: [PATCH 1/2] [analyzer] Fix a typo in a comment in ArrayBoundCheckerV2 (NFC) ...which was introduced by one of my recent commits. --- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index fdcc46e58580b4..65aad35315976c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -304,7 +304,7 @@ compareValueToThreshold(ProgramStateRef State, NonLoc Value, NonLoc Threshold, // negative_value == unsigned_value is always false return {nullptr, State}; } - // negative_value < unsigned_value is always false + // negative_value < unsigned_value is always true return {State, nullptr}; } if (isUnsigned(SVB, Value) && isNegative(SVB, State, Threshold)) { >From 467b9c6094018efead1981d607b67dfa174ca969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com> Date: Fri, 1 Mar 2024 10:17:20 +0100 Subject: [PATCH 2/2] [analyzer] Improve some comments in ArrayBoundCheckerV2 (NFC) To clarify them and include some thoughts about the difficulties in resolving a certain FIXME. --- .../Checkers/ArrayBoundCheckerV2.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index 65aad35315976c..29eb932584027d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -301,21 +301,27 @@ compareValueToThreshold(ProgramStateRef State, NonLoc Value, NonLoc Threshold, // calling `evalBinOpNN`: if (isNegative(SVB, State, Value) && isUnsigned(SVB, Threshold)) { if (CheckEquality) { - // negative_value == unsigned_value is always false + // negative_value == unsigned_threshold is always false return {nullptr, State}; } - // negative_value < unsigned_value is always true + // negative_value < unsigned_threshold is always true return {State, nullptr}; } if (isUnsigned(SVB, Value) && isNegative(SVB, State, Threshold)) { - // unsigned_value == negative_value and unsigned_value < negative_value are - // both always false + // unsigned_value == negative_threshold and + // unsigned_value < negative_threshold are both always false return {nullptr, State}; } - // FIXME: these special cases are sufficient for handling real-world + // FIXME: These special cases are sufficient for handling real-world // comparisons, but in theory there could be contrived situations where // automatic conversion of a symbolic value (which can be negative and can be // positive) leads to incorrect results. + // NOTE: We NEED to use the `evalBinOpNN` call in the "common" case, because + // we want to ensure that assumptions coming from this precondition and + // assumptions coming from regular C/C++ operator calls are represented by + // constraints on the same symbolic expression. A solution that would + // evaluate these "mathematical" compariosns through a separate pathway would + // be a step backwards in this sense. const BinaryOperatorKind OpKind = CheckEquality ? BO_EQ : BO_LT; auto BelowThreshold = _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
