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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to