Search code examples
c++pointersoperator-overloadingdestructorrvo

munmap_chunk invalid pointer when operator and then destroctor called


I faced with a weird situation while writing my C++ code. Inside my code, I have a matrix object called C. C would be equal to sum of matrix A and B. The value of A+B is calculated through an operator+. When life of C is ended, I face with an invalid pointer error.

This is my complete code:

#include <iostream>
#include <vector>
using namespace std;

class CMatrix
{
private:
    int rows=0,columns=0;
    int **members=NULL;
    void allocate(int rows,int columns);
public:
    CMatrix(int rows,int columns);
    CMatrix(int rows,int columns,vector<vector<int>> clone);
    CMatrix(const CMatrix& clone);
    ~CMatrix();
    void print();
    void sum(const CMatrix &A,const CMatrix &B);
    CMatrix operator+(const CMatrix &B);
};

void CMatrix::allocate(int rows,int columns)
{
    this->rows=rows;
    this->columns=columns;
    members=new int*[rows];
    for(int i=0;i<columns;i++)
        members[i]=new int[columns];
}

CMatrix::CMatrix(const CMatrix& clone)
{
    allocate(clone.rows,clone.columns);
    for(int i=0;i<rows;i++)
        for(int j=0;j<columns;j++)
            members[i][j]=clone.members[i][j];
}

CMatrix::CMatrix(int rows,int columns)
{
    allocate(rows,columns);
}

CMatrix::CMatrix(int rows,int columns,vector<vector<int>> clone)
{
    allocate(rows,columns);
    for(int i=0;i<rows;i++)
        for(int j=0;j<columns;j++)
            members[i][j]=clone[i][j];
}

CMatrix::~CMatrix()
{
    for(int i=0;i<rows;i++)
        delete [] members[i];
    delete [] members;
}

void CMatrix::print()
{
    cout<<"["<<endl;
    for(int i=0;i<rows;i++)
    {
        for(int j=0;j<columns;j++)
            cout<<members[i][j]<<(j==columns-1?"":",\t");
        cout<<endl;
    }
    cout<<"]"<<endl;
}

void CMatrix::sum(const CMatrix &A,const CMatrix &B)
{
    // at this point, all matrices must have the same dimention
    for(int i=0;i<rows;i++)
        for(int j=0;j<columns;j++)
            members[i][j]=A.members[i][j]+B.members[i][j];
}

CMatrix CMatrix::operator+(const CMatrix &B)
{
    CMatrix result(rows,columns);
    result.sum(*this,B);
    return result;
}

int main()
{
    CMatrix A(3,3,{{1,2,3},{4,5,6},{7,8,9}});
    CMatrix B(3,3,{{3,4,-1},{7,-2,1},{3,2,-4}});
    CMatrix C(3,3);
    cout<<"A is:"<<endl;
    A.print();
    cout<<"B is:"<<endl;
    B.print();
    cout<<"C=A+B is:"<<endl;
    C=A+B;
    C.print();
    return 0;
}

When I run my code I face with a wrong result and invalid pointer error:

A is:
[
1,  2,  3
4,  5,  6
7,  8,  9
]
B is:
[
3,  4,  -1
7,  -2, 1
3,  2,  -4
]
C=A+B is:
[
0,  0,  33
17019120,   0,  7
17019408,   0,  5
]
*** Error in `./a.out': munmap_chunk(): invalid pointer: 0x000000000103b230 ***
Aborted (core dumped)

How to fix it?

Again I ran the program using valgrind:

$ valgrind --tool=memcheck --db-attach=yes ./a.out
==11847== Memcheck, a memory error detector
==11847== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==11847== Using Valgrind-3.10.0.SVN and LibVEX; rerun with -h for copyright info
==11847== Command: ./a.out
==11847== 
A is:
[
1,  2,  3
4,  5,  6
7,  8,  9
]
B is:
[
3,  4,  -1
7,  -2, 1
3,  2,  -4
]
C=A+B is:
[
==11847== Invalid read of size 8
==11847==    at 0x400FEE: CMatrix::print() (test.cpp:64)
==11847==    by 0x401655: main (test.cpp:96)
==11847==  Address 0x5a1d910 is 0 bytes inside a block of size 24 free'd
==11847==    at 0x4C2C83C: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11847==    by 0x400F7D: CMatrix::~CMatrix() (test.cpp:55)
==11847==    by 0x401646: main (test.cpp:95)
==11847== 
==11847== 
==11847== ---- Attach to debugger ? --- [Return/N/n/Y/y/C/c] ---- 

It seems NRVO is not implemented correctly! What to do?

Note: I compile using: g++ -g -std=c++11 test.cpp

Edit:

adding this part of code solves the problem but adds a burden on my program. How can I enjoy advantage of NRVO?

void CMatrix::operator=(const CMatrix &B)
{
    for(int i=0;i<rows;i++)
        for(int j=0;j<columns;j++)
            members[i][j]=B.members[i][j];
}

Solution

  • First of all you have a mishap in your 'allocate()': you should check 'i < rows' instead of 'i < columns'. About NRVO: you don't need to specify std=c++11 to enable return value optimizations. In order to activate it you should put some

     CMatrix D=A+B
    

    instead of

     C=A+B. 
    

    If you add some logging to the constructor/destructor you will see that optimization is performed perfectly. C was created earlier with other params, it can't be initialized by another object without copy constructor.