Multiplication of Matrices Crashes Program

[uber08]

In short, I was assigned the task of creating a class that dynamically allocates memory to form a matrix of int values.

Part of the class are member functions that perform the basic matrix calculations -- addition, subtraction, and multiplication. Everything compiles (on my end at least), but when I used a driver to test the multiplication portion, it keeps crashing.

I'm using Codeblocks as my IDE and haven't had much luck with the debugger there in trying to figure it out. It appears that the calculation completes (with correct values), but then things go horribly wrong somewhere.

For clarity, each object of the Matrix class has the following member data:

private:
    int rows;
    int cols;
    int **element;

Below is a snippet of the implementation file in which the overloaded operator* is fleshed out. The portion where temp.element[i][x] is set to '0' before the loop performing the multiplication is commented out because the default constructor already sets all values to '0' -- which I had forgotten when I put it in originally. It didn't work when I didn't have it commented out either.

In testing, I used one 2x3 array and one 3x2 array.

Matrix Matrix::operator*(const Matrix &aMatrix) const
{
    if(cols == aMatrix.rows)
    {
        Matrix temp(rows, aMatrix.cols);
        for(int i = 0; i < rows; i++)
        {
            for(int x = 0; x < aMatrix.cols; x++)
            {
                //temp.element[i][x] = 0;
                for(int n = 0; n < cols; n++)
                {
                    temp.element[i][x] += (element[i][n]
                                       * aMatrix.element[n][x]);
                }
            }
        }
        return temp;
    }
    else
    {
        cerr << "Matrix multiplication failed -- incompatible matrix sizes."
             << endl;
        return *this;
    }
}

Any help or insight is appreciated. Thanks.

[Smac89]

Think about what happens when you try to multiply these 2 matrices:

[2 3 4] * [5 6 7]^T

Use your code to trace it then you will see where your fault lies

[uber08]

Thanks for looking it over.

Does the superscript "T" mean something?

Otherwise, I think the "if" check should return false, go to the error message, and return the left operand matrix... Would have to be something in the else...

[Smac89]

Yes the super script T means transpose. The transpose of the matrix will be the 3 x 1 matrix. So:
| 5 |
| 6 |
| 7 |

[uber08]

I must be missing something :/.

Unless I'm reading through it incorrectly, I believe the process would be the following:

A 1x1 matrix would be created as a product of the multiplication.

The for-loop with rows as the upper threshold of i would have only one iteration.

Same goes for the for-loop with aMatrix.cols as the upper threshold of x.

The final for-loop should have three iterations that perform the following calculation (2*5) + (3*6) + (4*7)

Where am I going wrong?

[Smac89]

Oh nvm, I thought something else in the calculation must have been creating an indexOutOfBounds error somewhere. But I now see the problem.

You have created the temp array inside your multiplication function; Not only that, but you have also created it on the stack. Once the function exits, your matrix does not exist anymore because the memory allocated for it becomes invalidated.

To fix this, you need to allocate the matrix on the heap and return a pointer to it

[uber08]

I was wondering about that.

The function returns a Matrix object, but I don't know if that is sufficient to take care of what you mentioned.

If I'm understanding these things correctly (which it is highly likely that I'm not), if I were to test it with something like:

Matrix m1(2,3), m2(3,2), m3;
m3 = m1*m2;

shouldn't m3 be assigned (albeit through the overloaded assignment operator) the value of the "temp" Matrix object that's being returned by the multiplication function?

If it's easier, I can PM you the driver and more of the implementation file to see if it works for you, but I don't want to trouble you too much.

[Smac89]

No need to PM me the file, but what you said is exactly true. m3 will be referencing to memory that has been deallocated.

And as I mentioned the way to fix it is to return a pointer instead and allocate your temp matrix on the heap by using the new operator

[uber08]

Hmmm I'll have to consult some people then. The instructions call for it to return "Matrix" as it does now >_<. Thanks for the help though.

[Smac89]

If that what you need then all you have to do is de-reference the pointer when you return it, just as you did with the this pointer. So your return type will be Matrix

[uber08]

True true. Thanks for the help smac. I was able to get it functioning fairly well. When I return a pointer, there is segfault somewhere in my destructor that I need to look into. Marking this particularly issue as solved though. Thanks again.