=?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/83...@github.com>
llvmbot wrote: <!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (NagyDonat) <details> <summary>Changes</summary> This comment-only change fixes a typo, clarifies some comments and includes some thoughts about the difficulties in resolving a certain FIXME. --- Full diff: https://github.com/llvm/llvm-project/pull/83545.diff 1 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp (+11-5) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index fdcc46e58580b4..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 false + // 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 = `````````` </details> https://github.com/llvm/llvm-project/pull/83545 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
