On Mon, May 12, 2014 at 7:04 PM, Stephan Tolksdorf <s...@quanttec.com> wrote:
> 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:
>
OMG. Yes, I can see how it works, although it looks so heavy. Let me (and
others) think a bit.
>
> 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.
I just wanted to verify that asan does not catch the bug. This line is
indeed redundant.
>
>
> char *p = (char*)v.data();
>> assert(!__asan_address_is_poisoned(p));
>> assert(__asan_address_is_poisoned(p+1));
>> }
>>
>>
> - Stephan
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
