[PATCH] D125524: [BoundV2] ArrayBoundV2 checks if the extent is tainted

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:208
     if (state_exceedsUpperBound && state_withinUpperBound) {
-      SVal ByteOffset = rawOffset.getByteOffset();
-      if (isTainted(state, ByteOffset)) {
+      if (isTainted(state, *upperboundToCheck)) {
         reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted,
----------------
steakhal wrote:
> martong wrote:
> > Could you please explain why we change `rawOffset` to `*upperBoundToCheck`? And perhaps the same explanation could infiltrate into the checker's code itself as a comment to `upperbound`.
> In the test attached you can see that the extent is tainted, not the offset.
> Thus checking the offset for taint won't suffice.
> The bug condition should depend on the calculation itself, which is basically what is done here.
Okay makes sense, but then please update the comment
```
// If we are under constrained and the index variables are tainted, report.
```
to mention the extent as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125524/new/

https://reviews.llvm.org/D125524