[PATCH] D159105: [analyzer] ArrayBoundCheckerV2 should check the region for taint as well

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D159105#4633899 <https://reviews.llvm.org/D159105#4633899>, @steakhal wrote:

> The measurement is still running, and I'll post a few words about that once I had a look; but anyway, I feel that this patch is controversial and needs more discussion before it could land.
> I plan to abandon this and think about it. We can continue the discussion of course.

Here are the results after applying the suppression heuristic for `getenv`, plus D159108 <https://reviews.llvm.org/D159108> and D159109 <https://reviews.llvm.org/D159109>:

- `=X`: Given message appeared X times in baseline.
- `+X (+Y%)`: Patch added X new reports with this message, and that is Y percent increase compared to the baseline.
- `-X (-Y%)`: Removed X reports with this message, and that is Y percent decrease compared to the baseline.

  * Out of bound memory access (accessed memory precedes memory block)
    =1221, +0 (+0%), -4 (-0%)
  * Out of bound memory access (access exceeds upper limit of memory block)
    =10547, +7 (+0%), -5 (-0%)
  * Out of bound memory access (index is tainted)
    =624, +71 (+11%), -1 (-0%)
  
  * Memory copy function overflows the destination buffer
    =436, +45 (+10%), -1 (-0%)
  * Memory copy function accesses out-of-bound array element
    =443, +19 (+4%), -1 (-0%)
  * Memory comparison function accesses out-of-bound array element
    =102, +1 (+1%), -1 (-1%)
  * Memory set function overflows the destination buffer
    =119, +10 (+8%), -0 (-0%)
  * String copy function overflows the destination buffer
     =42, +3 (+7%), -0 (-0%)
  
  * This was not the most recently acquired lock. Possible lock order reversal
    =81, +0 (+0%), -1 (-1%)
  * Division by zero
    =775, +1 (+0%), -0 (-0%)
  * Called C++ object pointer is null
    =3719, +0 (+0%), -2 (-0%)
  * Potential leak of memory pointed to by
    =1996, +0 (+0%), -2 (-0%)
  * Returned pointer value points outside the original object (potential buffer overflow)
    =625, +2 (+0%), -0 (-0%)

I think the mentioned heuristic works quite well, and really decreased the number of FPs for `Out of bound memory access (index is tainted)`.
I still don't want to commit to this patch, so I'm abandoning this.

I don't plan to repeat the measurement for D159108 <https://reviews.llvm.org/D159108> and D159109 <https://reviews.llvm.org/D159109>, as the numbers related to those messages look nice.
I had a look at the reports for those, and as expected it's barely impossible to make sense of them. But at least nothing dumb stood out.
If you are okay, I'll commit D159108 <https://reviews.llvm.org/D159108> and D159109 <https://reviews.llvm.org/D159109> later today. @donat.nagy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159105