C++ rvalue assignment what caused this program to crash but lvalue assignment not

user_1348691 0
Viewed 43

I am studying C++ rvalue reference by example. It was like this:

Intvec::~Intvec()
{
    log("destructor");
    if (m_data) {
        delete[] m_data;
        m_data = 0;
    }
}

Intvec::Intvec(const Intvec& other)
    :m_size(other.m_size), m_data(new int[m_size])
{
    log("copy constructor");
    for(size_t i = 0; i<m_size; ++i)
        m_data[i] = other.m_data[i];
}

Intvec& Intvec::operator=(const Intvec& other)
{
    log("copy assignment operator");
    Intvec tmp(other);
    std::swap(m_size, tmp.m_size);
    std::swap(m_data, tmp.m_data);
    return *this;
}

Then I changed it to be following. When debugging step by step, it calls delete[] many times until it crashes: read access violation at: 0x3f7a2f91

Intvec& Intvec::operator=(const Intvec& other)
{
    log("copy assignment operator");
    size_t othersize = other.m_size;
    int* otherdata = other.m_data;
    std::swap(m_size, othersize);
    std::swap(m_data, otherdata);
    return *this;
}

I am confused at:

  1. In example, Intvec tmp(other) returns a tmporary tmp object, it would be destroyed after return *this, it does not crash.
  2. In my code, other is an rvalue, it is also destroyed after return *this, why it crashes because of read access at wrong memory.

It only happens to rvalue, not lvalue.

    Intvec v1 = Intvec(10);
    Intvec v2;
    cout << "assigning lvalue...\n";
    v2 = v1;
    cout << "ended assigning lvalue...\n";

    Intvec v2;
    cout << "assigning rvalue...\n";
    v2 = Intvec(33);
    cout << "ended assigning rvalue...\n";
c++
2 Answers

You are deleting the same m_data pointer twice. The problem starts here:

Intvec& Intvec::operator=(const Intvec& other)
{
    log("copy assignment operator");
    size_t othersize = other.m_size;
    int* otherdata = other.m_data;
    std::swap(m_size, othersize);
    std::swap(m_data, otherdata);
    return *this;
}

Decide if you want to transfer ownership, or duplicate. If you want to transfer ownership, then you need to set other.m_data = NULL after swapping it so that the r-value temporary object doesn't delete its own m_data when it destructs. You also need to delete the original m_data in the destination object PRIOR to swapping if it's not NULL (Apparently it is not NULL because otherwise the swap function would set other.m_data to NULL for you and there would be no problem).

If instead you'd rather duplicate it, then you need to structure your assignment operator more similarly to your copy constructor.

user_36625430

For your operator=, after v2 = Intvec(33);, the data member m_data of both v2 and the temporary Intvec point to the same array allocated. Then with the destroy of the temporary and v2, m_data would be deleted twice, which leads to UB.

user_33097900
Related