================
@@ -576,8 +576,20 @@ ProgramStateRef
CStringChecker::CheckLocation(CheckerContext &C,
auto [StInBound, StOutBound] = state->assumeInBoundDual(*Idx, Size);
if (StOutBound && !StInBound) {
+ // The analyzer determined that the access is out-of-bounds, which is
+ // a fatal error: ideally we'd return nullptr to terminate this path
+ // regardless of whether the OutOfBounds checker frontend is enabled.
+ // However, the current out-of-bounds modeling produces too many false
+ // positives, so when the frontend is disabled we return the original
+ // (unconstrained) state and let the analysis continue. This is
+ // inconsistent: returning `state` instead of `StOutBound` discards the
+ // constraint that the index is out-of-bounds, and callers cannot
+ // distinguish "we proved an error" from "we couldn't determine anything"
+ // since both return the original state.
+ // TODO: Once the OutOfBounds frontend is stable, return nullptr here
+ // unconditionally to stop the analysis on this path.
----------------
steakhal wrote:
I love these comments. I just measured and it takes for me 60 seconds to read
it.
I hope not a lot of people will pay this 1 minute tax in the future realising
that it wasn't useful for them -.-
I know @NagyDonat you love verbose comments.
To be clear, I'm not challenging this here - so no actions expected.
I just wanted to mention that this costs 1 minute for every future reader +
extra for actually understanding what was written.
https://github.com/llvm/llvm-project/pull/186802
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits