[cfe-dev] annotating libc++ to catch buffer overflows in vector/string with asan

[Kostya Serebryany]

Marshall,
Here is a more complete patch for asan vector annotations.
It passes all tests in libcxx/test/containers/sequences/vector, but doesn't
yet work on larger programs (I tried Chrome).
Two questions:
   1. Does it look like something potentially committable?
   2. Would you suggest some bigger test suite specifically targeted to
test std::vector?

Thanks,

--kcc



On Wed, Nov 20, 2013 at 6:35 PM, Kostya Serebryany <kcc at google.com> wrote:

> Attached is an intermediate patch that passes my tests (also attached).
> Few questions:
>
> The patch gets a bit too verbose and it has a few repetitions, e.g. we
> have this snippet repeated multiple times:
> +            __annotate_contiguous_container(data(), data() + capacity(),
> +                                            data() + size(), data() +
> size() + 1);
>
> WDYT about adding a private method to vector like this (and a couple more)?
>
> void __annotate_size_increase(size_t __n) {
>     __annotate_contiguous_container(data(), data() + capacity(),
>                                             data() + size(), data() +
> size() + __n);
> }
>
> In order to test the annotation properly I need to do something like this
> as often as possible:
> template <class T> void TestVector(std::vector<T> &v) {
>
>   if (v.capacity() == 0) return;
>
>   char *beg = (char*)&v.front();
>
>   char *end = beg + v.capacity() * sizeof(T);
>
>   char *mid = beg + v.size() * sizeof(T);
>
>   for (char *p = beg; p < mid; p++)
>
>     assert(!__asan_address_is_poisoned(p));
>
>   for (char *p = mid ; p < end; p++)
>
>     assert(__asan_address_is_poisoned(p));
>
> }
>
> Is it possible to add code like this as another private method of vector
> (under some ifdef)?
>
> --kcc
>
>
> On Tue, Nov 19, 2013 at 1:57 PM, Kostya Serebryany <kcc at google.com> wrote:
>
>>
>>> > Do you like the idea?
>>>
>>> Yes - very much so.
>>>
>>> > Any comments about the prototype patch?
>>>
>>> Yes.
>>>
>>> 1) I would implement it with inline functions other than macros.
>>>
>>> Like this:
>>>
>>>
>>> extern "C" void __sanitizer_annotate_contiguous_container(void *, void
>>> *, void *, void *);
>>> void inline __annotate_contiguous container
>>>         ( const void *__beg, const void *__end, const void *__old_mid,
>>> const void *new_mid )
>>> #if __has_feature(address_sanitizer)
>>>         {  __sanitizer_annotate_contiguous_container(beg, end, old_mid,
>>> new_mid); }
>>> #else
>>>         {}
>>> #endif
>>>
>>
>> Makes sense, see below.
>> We''l probably want to move __annotate_contiguous container into a
>> separate file so that both string and vector use it.
>> But let's do the whole thing with vector before moving to string.
>>
>>
>>
>>>
>>> 2) I'd like more information about what
>>> __sanitizer_annotate_contiguous_container does ;-)
>>>
>>
>> In short, it [un]poisons the parts of shadow memory that correspond to
>> the container's memory.
>> More details in compiler-rt files:
>>  include/sanitizer/common_interface_defs.h and lib/asan/asan_poisoning.cc
>>
>>
>>
>>>
>>> > Any suggestion how to test these annotations in libc++ (we need some
>>> kind of DEATH tests)?
>>>
>>> We already have failing tests in libc++.
>>> they are named XXXX.fail.cpp.
>>>
>>
>> These are tests that fail to compile.
>> for asan annotations we need tests that build and run, and then either
>> fail at run-time or check the state of shadow.
>> Like these:
>> compiler-rt/lib/asan/lit_tests/TestCases/contiguous_container.cc
>> compiler-rt/lib/asan/lit_tests/TestCases/contiguous_container_crash.cc
>>
>> Suggestions?
>>
>> --kcc
>>
>> --- include/vector      (revision 195116)
>> +++ include/vector      (working copy)
>> @@ -288,6 +288,16 @@
>>
>>  _LIBCPP_BEGIN_NAMESPACE_STD
>>
>> +extern "C" void __sanitizer_annotate_contiguous_container(
>> +  const void *, const void *, const void *, const void *);
>> +void inline __annotate_contiguous_container
>> +    (const void *__beg, const void *__end, const void *__old_mid, const
>> void *__new_mid)
>> +#if __has_feature(address_sanitizer)
>> +    {  __sanitizer_annotate_contiguous_container(__beg, __end,
>> __old_mid, __new_mid); }
>> +#else
>> +    {}
>> +#endif
>> +
>>  template <bool>
>>  class __vector_base_common
>>  {
>> @@ -1525,7 +1535,12 @@
>>      // __v.push_back(_VSTD::forward<_Up>(__x));
>>      __alloc_traits::construct(__a, _VSTD::__to_raw_pointer(__v.__end_),
>> _VSTD::forward<_Up>(__x));
>>      __v.__end_++;
>> +    if (data())
>> +      __annotate_contiguous_container(
>>  +          data(), data() + capacity(), data() + size(), data() +
>> capacity());
>>      __swap_out_circular_buffer(__v);
>> +    __annotate_contiguous_container(data(), data() + capacity(),
>> +                                          data() + capacity(), data() +
>> size());
>>  }
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131209/d2573283/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vector_asan.patch
Type: text/x-patch
Size: 8512 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131209/d2573283/attachment.bin>