================
@@ -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

Reply via email to