[PATCH] Adding ASAN support for libc++'s vector

[Kostya Serebryany]

On Wed, May 7, 2014 at 7:51 PM, Marshall Clow <mclow.lists at gmail.com> wrote:

>
> On May 7, 2014, at 12:25 AM, Kostya Serebryany <kcc at google.com> wrote:
>
> On Wed, May 7, 2014 at 1:05 AM, Marshall Clow <mclow.lists at gmail.com>
>  wrote:
>
>> Address sanitizer does not currently check for accesses beyond the end of
>> a vector, but within the memory block managed by the vector.
>>
>> For example:
>>         vector<int> v;
>>         v.reserve(10); // make space for 10 elements, but vector is still
>> empty
>>         cout << v[1]; // access outside the “valid elements” of the
>> vector.
>>
>> This patch adds the ability to detect these kinds of errors to libc++
>> when using Address Sanitizer.
>> Thanks to Kostya for most of the code here.
>>
>
> Looks great, ship it!
>
>
> Few minor comments below.
>
> +++ include/__config    (working copy)
> +#ifndef _LIBCPP_HAS_NO_ASAN
>
> Do you prefer double-negative statements?
> #ifdef _LIBCPP_HAS_ASAN
> might be easier to read, but that's not critical.
>
>
> The reason is that this way you can turn on address sanitizer in your
> project, but still disable ASAN in libc++ if you want
> by adding "-D _LIBCPP_HAS_NO_ASAN=1 “ to the command line.
>


You can do it like this:

#ifdef _LIBCPP_HAS_NO_ASAN
#define _LIBCPP_HAS_ASAN 0
#else
// the rest of logic with __has_feature, etc
#endif

then use #if _LIBCPP_HAS_ASAN

I don't insist (especially if this does not look like the rest of the
code).




>
> +++ test/containers/sequences/vector/asan.pass.cpp      (working copy)
> +#ifndef _LIBCPP_HAS_NO_ASAN
> +extern "C" void __asan_set_error_exit_code(int);
>
> +++ test/support/asan_testing.h (working copy)
> +#ifndef _LIBCPP_HAS_NO_ASAN
> +extern "C" int __sanitizer_verify_contiguous_container
> +     ( const void *beg, const void *mid, const void *end );
>
> I understand the desire to not incluide <sanitizer/asan_interface.h> in
> include/__config,
> but why not include it in the test files?
>
>
> Because I didn’t want to add an external dependency to the libc++ test
> suite.
> Interestingly enough, people are using the libc++ test suite on other
> compilers than clang,
> and on other standard library implementations, too.
>
> Is that the right path when using GCC as well?
>

Good point. No, gcc does not install these headers, filed
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=61100
Meanwhile, yes, we'll need to declare the functions instead of including
the header.

>
>
> +++ test/containers/sequences/vector/asan.pass.cpp      (working copy)
> +#if __cplusplus >= 201103L
> +    {
> +        typedef int T;
> +        typedef std::vector<T, min_allocator<T>> C;
> +        const T t[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
> +        C c(std::begin(t), std::end(t));
> +        c.reserve(2*c.size());
> +        T foo = c[c.size()];    // bad, but not caught by ASAN
> +    }
> +#endif
>
> Maybe add a comment explaining why ASAN does not catch it
> (because asan can't handle arbitrary allocator)?
>
>
> Can do.
>
> — Marshall
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140507/91ee2035/attachment.html>