[PATCH] [libcxxabi] Cleanup memory in tests to placate ASAN.

David Blaikie dblaikie at gmail.com
Mon Nov 17 11:03:23 PST 2014

Comment at: test/catch_ptr_02.cpp:145
@@ -144,1 +144,3 @@
         assert(p != 0);
+        // NOTE: delete needed to placate ASAN.
+        delete p;
EricWF wrote:
> dblaikie wrote:
> > Could just rephrase this:
> > 
> >   vDerived d;
> >   try
> >   {
> >       throw &d;
> > 
> > I assume that'd be sufficiently equivalent without having to pass owning pointers through the throw/catch?
> That's essentially what the functions `test1`, `test2`, `test3`, and `test4` are doing. That is why I was hesitant to change the form. 
test 1-4 are testing that the constness matches correctly, this is testing that catching by base from a derived throw works correctly - I think that's orthogonal to the way the memory is allocated.

Comment at: test/test_vector1.cpp:238
@@ -237,1 +237,3 @@
+// Delete "three" to placate ASAN.
+    __cxxabiv1::__cxa_vec_delete3(three, 40, 8, my_destruct, my_dealloc3);
EricWF wrote:
> dblaikie wrote:
> > & I don't understand this one well enough to figure out why 'three' needs to be handled here but 'one' and 'two' are fine... all the deletes/news seem to appear in triples, so why is this one weird?
> There is a counter that throws after 15 objects have been destroyed. There are 10 objects in each of the vectors. That means the exception is throws in the call to `__cxa_vec_delete2`. I'm assuming the test checks that `two`'s memory gets freed even when there is an exception thrown during `__cxa_vec_delete2`. 
I think the better way to phrase this part of the test might be to remove 'three' entirely (its allocations aren't part of the test, the test is about throwing from the dtor) and add an "assert(false)" where the delete3 is currently (and change the expectations to match the new behavior - 20 constructions, 20 destructions I guess). Yes/no/maybe?


More information about the cfe-commits mailing list