[PATCH] D27061: [asan] Avoid redundant poisoning checks in __sanitizer_contiguous_container_find_bad_address

[Evgeniy Stepanov via Phabricator via llvm-commits]

eugenis added a comment.

In https://reviews.llvm.org/D27061#605247, @filcab wrote:

> In https://reviews.llvm.org/D27061#605225, @i.baravy wrote:
>
> > Thank you for the review, guys!
> >
> > Vitaly,
> >  could you suggest a test to update, please? I have no idea how to test this typo.
>
>
> I'm unsure you can easily test this since, due to the `CHECK_LE(mid, end);` (and no overflow), we know that `end + kMaxRangeToCheck` is bigger than `mid`, so we'll always get mid.
>  Maybe you can test it by having more than 64 bytes between beg and mid, and poisoning the 33rd byte. That way you know the check shouldn't be able to figure it out and you can check for lack of error (which is what should happen).
>  Of course, you'd need to add comments explaining you're testing that exact case, and any change to `kMaxRangeToCheck` would need a change in the test.


I don't think such test adds any value. I'm fine w/o any test changes.



================
Comment at: lib/asan/asan_poisoning.cc:416
+  uptr r1_end = Min(beg + kMaxRangeToCheck, mid);
+  uptr r2_beg = Max(r1_end, mid - kMaxRangeToCheck/2);
+  uptr r2_end = Min(end, mid + kMaxRangeToCheck/2);
----------------
m.ostapenko wrote:
> Why do you need all these changes (416, 417, 418)? If I read the code correctly, originally ASan checks bytes around **beg**, **mid** and **end**. What's incorrect here? Overlapping of memory ranges isn't a big deal here IMHO.
Well, with this change we get the same result with less work, so why not?



Repository:
  rL LLVM

https://reviews.llvm.org/D27061