<div dir="ltr"><div>My main concern with container poisoning is that it is all-or-nothing - you need to build all translation units that include the container definition with ASan, or risk false positive due to a missing shadow update. This is not a property of ASan in general, and can result in confusing error reports. I'm fine with it as long as it can be easily disabled, see below.</div><div><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 8, 2020 at 10:39 AM Mitch Phillips via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">It sounds like a very reasonable strategy!<div><br></div><div>Have you seen __sanitizer_annotate_contiguous_container (compiler-rt/include/sanitizer/common_interface_defs.h)? In case you missed it - we have container-overflow built into ASan and implemented in std::vector in libcxx, and this is the canonical way of annotating containers like this.</div></div></blockquote><div><br></div><div>Yes, this will produce better error reports, and it can be disabled with a runtime flag.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>Have you tried building LLVM/Clang with an ASan-ified LLVM/Clang? Curious to see whether there's any latent bugs around.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Sep 7, 2020 at 7:50 AM Nathan James via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Dear list,<br>
<br>
I recently tried to add instrumentation to SmallVector for using<br>
Address sanitizer to detect cases where references used after they are<br>
invalidated. This basic implementation for this is here - <br>
<a href="https://reviews.llvm.org/D87237" rel="noreferrer" target="_blank">https://reviews.llvm.org/D87237</a><br>
<br>
However, in adding/testing this, I did uncover some questionable code.<br>
Firstly `SmallString<unsigned>::c_str()` and<br>
`Twine::toNullTerminatedStringRef(SmallVectorImpl<char>&)` both use<br>
bytes outside the range of the SmallVectors storage. This isn't<br>
inherently bad.<br>
Secondly calling `SmallVectorImpl<T>::insert(iterator, const T&)`<br>
results in a reference invalidation when the element to insert is<br>
contained inside the SmallVector and the SmallVector needs to grow for<br>
the insert. This has been fixed inside the aforementioned PR.<br>
<br>
My main point here is how does everyone feel about using ASAN to catch<br>
bugs like this not only inside SmallVector but also adding the<br>
instrumentation to some other containers used by llvm. If people are<br>
happy with this implementation for SmallVector I'd be happy for<br>
feedback on the PR. It would likely need some specific asan test cases<br>
however I'm not entirely sure how to go about adding those.<br>
<br>
Thanks for reading,<br>
<br>
~Nathan<br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>