On Wed, May 7, 2014 at 7:51 PM, Marshall Clow <[email protected]> wrote:
> > On May 7, 2014, at 12:25 AM, Kostya Serebryany <[email protected]> wrote: > > On Wed, May 7, 2014 at 1:05 AM, Marshall Clow <[email protected]> > 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 > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
