1

I'm doing a matrix-product exercise (strassen's algorithm actually) using C++. Since the biggest data set given reaches 2048 * 2048, I tried to free the temp memory using delete[]. But it says there is a memory access violation in it, why?

Here are some of the code that may help:

struct Matrix {
    int row, column;
    int** m;

    Matrix(int row, int column) {
        m = new int* [2048];
        for (int i = 0; i < row; i++)
            m[i] = new int[2048];
    }

    ~Matrix() {
        if (m != NULL) {
            for (int i = 0; i < 2048; i++) {
                delete[] m[i]; //access violation happens here
            }
            delete[] m;
        }
    }
};

Matrix matAdd(Matrix matA, Matrix matB) {
    Matrix matR = Matrix(matA.row, matA.column);
    for (int i = 0; i < matA.row; i++)
        for (int j = 0; j < matA.column; j++) {
            matR.m[i][j] = matA.m[i][j] + matB.m[i][j];
        }
    return matR;
}

//There are some other functions below but the structure is basically the same
  • 1
    You must implement a copy constructor and an assignment operator, at the least, for your class. See the duplicate question for more information. You do understand that you're passing ***and returning*** your matrixes by value, which creates duplicate objects. Without an explicit copy constructor, the multiple copies have the same pointer, and their destructors will then try to delete the same pointer multiple times. Hillarity ensues. Or, alternatively, forget `new` and `delete`ing anything, and use a `std::vector`, which will do everything for you. That's what it's there for. – Sam Varshavchik Oct 04 '20 at 16:26
  • At the duplicate link, pay special attention to the **Managing resources** section. That section describes exactly what the problem is. – PaulMcKenzie Oct 04 '20 at 16:29

2 Answers2

1

The pointers in the array, starting from the index row were not initialized.

Matrix(int row, int column) {
    m = new int* [2048];
    for (int i = 0; i < row; i++)
        m[i] = new int[2048];

    // Initialize rest elements with null pointer.
    for (int i = row; i < 2048; i++)
        m[i] = nullptr;
}

The rest initialization will fix the crash, but then you will get crash on double free. The reason: you use copies of your class, thus you need to implement the copy constructor and the assignment operator or use shared pointers instead of raw pointers. The default copy makes pointers pointing to the same allocated objects.

Also look at the answer https://stackoverflow.com/a/1403180/6752050 with the example how to create a 2d matrix with a single allocation.

273K
  • 29,503
  • 10
  • 41
  • 64
0

You should consider using std::vector<std::vector<int>>.

Your class is lacking copy constructor and copy assignment and I am certain it is a case of double free.

Tanveer Badar
  • 5,438
  • 2
  • 27
  • 32