Iterator debugging crashes when reassigning an stl iterator in an object destructor within a CArray

Posted by Microsoft on 2/6/2013 at 8:22 PM

Hi,

Thanks for reporting this issue. I've resolved it as By Design because CArray was designed for Plain Old Data only, and STL containers like std::set are inherently non-POD.

If you set a breakpoint on "CArray<Point,Point const &>::SetSize" without quotes, you can see what's happening. This invokes:

// copy new data from old
::ATL::Checked::memcpy_s(pNewData, (size_t)nNewMax * sizeof(TYPE),
m_pData, (size_t)m_nSize * sizeof(TYPE));

That is, when a CArray expands, it calls memcpy to blast bits from the old memory block to the new memory block, instead of invoking copy constructors and destructors. This is fine for Plain Old Data (ints, floats, structs of ints, etc.) but is completely wrong for objects with copy constructors and destructors, like std::set. (Physically, what's happening is that in debug mode, STL containers keep track of all of their iterators, in order to emit extremely useful assertions when invalidated iterators are used and so forth. This is done by storing extra data members in containers and iterators, and updating those data members when things are copied/destroyed/etc. Because memcpy bypasses copy constructors, the bookkeeping information for iterator debugging is totally damaged, leading to a crash.)

This is actually documented at http://msdn.microsoft.com/en-us/library/4h2f09ct.aspx : "Most methods that resize a CArray object or add elements to it use memcpy_s to move elements. This is a problem because memcpy_s is not compatible with any objects that require the constructor to be called. If the items in the CArray are not compatible with memcpy_s, you must create a new CArray of the appropriate size. You must then use CArray::Copy and CArray::SetAt to populate the new array because those methods use an assignment operator instead of memcpy_s."

My recommendation goes beyond MSDN's: avoid using CArray, period. Instead, use std::vector (which respects copy constructors, and is highly optimized - it'll invoke C++11 move constructors when possible, and when it senses int/etc. elements it'll invoke memmove to blast bits). If you must use CArray (e.g. when talking to other ATL/MFC code), make sure to use it with PODs only.

Note: Connect doesn't notify me about comments. If you have any further questions, please E-mail me.

Stephan T. Lavavej
Senior Developer - Visual C++ Libraries
stl@microsoft.com

Posted by Microsoft on 2/4/2013 at 11:02 PM

Thanks for your feedback.

We are rerouting this issue to the appropriate group within the Visual Studio Product Team for triage and resolution. These specialized experts will follow-up with your issue.

Posted by Microsoft on 2/4/2013 at 8:50 AM

Thank you for your feedback, we are currently reviewing the issue you have submitted. If this issue is urgent, please contact support directly(http://support.microsoft.com)

Posted by Viorel_ on 2/4/2013 at 11:42 PM

“Most methods that resize a CArray object or add elements to it use memcpy_s to move elements. This is a problem because memcpy_s is not compatible with any objects that require the constructor to be called” [http://msdn.microsoft.com/en-us/library/4h2f09ct(v=vs.110).aspx].

Therefore avoid mixing of certain MFC and STL classes.

Or add this specialization of ‘memcpy_s’:

#if _ITERATOR_DEBUG_LEVEL == 2
namespace ATL
{
    namespace Checked
    {
        void memcpy_s(Point * dest, size_t dest_size, const Point * source, size_t src_size)
        {
            assert(dest_size >= src_size);
            assert(src_size % sizeof(Point) == 0);

            size_t n = src_size / sizeof(Point);
            for(size_t i = 0; i < n; ++i)
            {
#pragma push_macro("new")
#undef new
                new (&dest[i]) Point(std::move(source[i]));
                source[i].~Point();
#pragma pop_macro("new")
            }
        }
    }
}
#endif

Iterator debugging crashes when reassigning an stl iterator in an object destructor within a CArray by Dave Wilcoxson