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

[Kostya Serebryany]

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/20131120/e1b14d3d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vector_asan_annotations_test.cc
Type: text/x-c++src
Size: 2113 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131120/e1b14d3d/attachment.cc>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vec.patch
Type: text/x-patch
Size: 7103 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131120/e1b14d3d/attachment.bin>