[libcxx] r208319 - Add Address Sanitizer support to std::vector

[Stephan Tolksdorf]

On 14-05-12 16:27, Kostya Serebryany wrote:
 >         And an unrelated issue: the documentation for
 >         __sanitizer_annotate_contiguous_container states that the
 >         complete buffer should be unpoisened before it is deallocated.
 >         This doesn't seem to be happening in the destructor or in the
 >         deallocate function.
 >
 >     Here we rely on the fact that vector is using the default allocator,
 >     which immediately calls delete, which itself unpoisons the memory
 >     chunk.

Do I understand you correctly, that the global ::operator delete doesn't 
actually require the complete buffer to be unpoisened? I assume this is 
also true for ::free then?

> Reproduced. (we have very little code with exceptions so this has
> avoided our testing efforts :)
> Any suggestion for the fix?

Using a scope guard object for annotating operations that increase the 
size would be one possibility, as e.g. in the following pseudo code:

struct AnnotatedAppendScope {
   Vector& v;
   const size_t newLength;
#if USE_ASAN
   bool committed;
#endif

   /// \pre vector.length() < newLength <= vector.capacity()
   AnnotatedAppendScope(Vector& vector, size_t newLength)
   : v(vector), newLength(newLength)
   #if USE_ASAN
     , committed(false)
   #endif
   {
   #if USE_ASAN
     __sanitizer_annotate_contiguous_container(
       v.begin(), v.begin() + v.capacity(),
       v.begin() + v.length(), v.begin() + newLength;
     );
   #endif
   }

#if USE_ASAN
   ~AnnotatedAppendScope() {
     if (!committed) {
       __sanitizer_annotate_contiguous_container(
         v.begin(), v.begin() + v.capacity(),
         v.begin() + newLength, v.begin() + v.length();
       );
     }
   }
#endif

   void commitLength() {
     v.setLength(newLength);
   #if USE_ASAN
     committed = true;
   #endif
   }
};

void Vector::push_back(const T& value) {
   reserve(length() + 1);
   AnnotatedAppendScope scope(*this, length() + 1);
   new (end()) T(value);
   scope.commitLength();
}

There's another comment below.

>
> #include <vector>
> #include <assert.h>
> #include <iostream>
> #include <sanitizer/asan_interface.h>
> using namespace std;
>
> class X {
>   public:
>    X(const X &x) {
>      if (x.a == 42) throw 0;
>      a = x.a;
>    }
>    X(char arg): a(arg){}
>    char get() const { return a; }
>   private:
>    char a;
> };
>
> int main() {
>    vector<X> v;
>    v.reserve(2);
>    v.push_back(X(2));
>    assert(v.size() == 1);
>    try {
>      v.push_back(X(42));
>      assert(0);
>    } catch (int e) {
>      assert(v.size() == 1);
>    }
>    assert(v.size() == 1);
>    assert(v[1].get() != 777);

What does the above line test?
It would always fail if the index is assert checked, which currently is 
the case when _LIBCPP_DEBUG is defined.

>    char *p = (char*)v.data();
>    assert(!__asan_address_is_poisoned(p));
>    assert(__asan_address_is_poisoned(p+1));
> }
>

- Stephan