Partial fix for #15048 (conditional containerOutOfBounds) - #8881
chrchr-github wants to merge 2 commits into
Conversation
| " return v;\n" | ||
| "}\n"); | ||
| ASSERT_EQUALS("[test.cpp:4:10]: error: Out of bounds access in 'v[i]', if 'v' size is 10 and 'i' is 10 [containerOutOfBounds]\n", | ||
| ASSERT_EQUALS("[test.cpp:4:10]: warning: Out of bounds access in 'v[i]', if 'v' size is 10 and 'i' is 10 [containerOutOfBounds]\n", |
There was a problem hiding this comment.
This looks like a ValueFlow issue, even known loop iterations have conditional values.
| " return v;\n" | ||
| "}\n"); | ||
| ASSERT_EQUALS("[test.cpp:4:10]: error: Out of bounds access in 'v[i]', if 'v' size is 10 and 'i' is 10 [containerOutOfBounds]\n", | ||
| ASSERT_EQUALS("[test.cpp:4:10]: warning: Out of bounds access in 'v[i]', if 'v' size is 10 and 'i' is 10 [containerOutOfBounds]\n", |
There was a problem hiding this comment.
This is an AI review take it with a grain of salt. Please feel free to reject it by clicking on Resolve button
The change only lowers the severity, so it can't add false positives, and it matches what negativeIndex does in checkbufferoverrun.cpp:464. Looks good to me.
As you noted, this #10779 case is a definite bug (the loop always reaches i == 10), and it is now downgraded only because of the ValueFlow issue with conditional values in known loop iterations. Could you add a short comment or a TODO_ASSERT_EQUALS with the error: expectation here? A ticket reference would also work. Otherwise the downgrade is easy to forget once the ValueFlow side is fixed.
3 participants
No description provided.