Skip to content

Commit 5dc9e87

Browse files
authored
[analyzer] Improve some comments in ArrayBoundCheckerV2 (NFC) (#83545)
This comment-only change fixes a typo, clarifies some comments and includes some thoughts about the difficulties in resolving a certain FIXME.
1 parent c6565f2 commit 5dc9e87

File tree

1 file changed

+11
-5
lines changed

1 file changed

+11
-5
lines changed

clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,21 +301,27 @@ compareValueToThreshold(ProgramStateRef State, NonLoc Value, NonLoc Threshold,
301301
// calling `evalBinOpNN`:
302302
if (isNegative(SVB, State, Value) && isUnsigned(SVB, Threshold)) {
303303
if (CheckEquality) {
304-
// negative_value == unsigned_value is always false
304+
// negative_value == unsigned_threshold is always false
305305
return {nullptr, State};
306306
}
307-
// negative_value < unsigned_value is always false
307+
// negative_value < unsigned_threshold is always true
308308
return {State, nullptr};
309309
}
310310
if (isUnsigned(SVB, Value) && isNegative(SVB, State, Threshold)) {
311-
// unsigned_value == negative_value and unsigned_value < negative_value are
312-
// both always false
311+
// unsigned_value == negative_threshold and
312+
// unsigned_value < negative_threshold are both always false
313313
return {nullptr, State};
314314
}
315-
// FIXME: these special cases are sufficient for handling real-world
315+
// FIXME: These special cases are sufficient for handling real-world
316316
// comparisons, but in theory there could be contrived situations where
317317
// automatic conversion of a symbolic value (which can be negative and can be
318318
// positive) leads to incorrect results.
319+
// NOTE: We NEED to use the `evalBinOpNN` call in the "common" case, because
320+
// we want to ensure that assumptions coming from this precondition and
321+
// assumptions coming from regular C/C++ operator calls are represented by
322+
// constraints on the same symbolic expression. A solution that would
323+
// evaluate these "mathematical" compariosns through a separate pathway would
324+
// be a step backwards in this sense.
319325

320326
const BinaryOperatorKind OpKind = CheckEquality ? BO_EQ : BO_LT;
321327
auto BelowThreshold =

0 commit comments

Comments
 (0)