Segmentation fault when using overloaded operator

Question

The following line in my code is causing a mysterious segfault:

N = N + M;

Where N and M are objects of the Matrix class:

class Matrix {
    vector<int> A; 
    int m, n;
}

+ operator function:

Matrix Matrix::operator+(const Matrix &other){
    //matrices must be of same dimensions
    //if not, return null Matrix
    if (m != other.m || n != other.n)
        return Matrix(0, 0);

    Matrix T(m, n);
    for (int i = 0; i < m*n; i++){
        T.A.at(i) = this->A.at(i) + other.A.at(i);
    }
    return T;
}

Of course N and M are of the same size (3x3).

The segfault appears even with this:

M = N + M;

or

M = M + N;

or

Matrix P;
P = M + N;

but not with:

Matrix P = M + N;

What could possibly be my mistake? I am new to C++.

EDIT: Here's the = operator:

Matrix Matrix::operator=(const Matrix &other){
    A = other.A;
    m = other.m, n = other.n;
}

EDIT 2: My constructor can be helpful

Matrix::Matrix(int r, int c):
    m(r), n(c), A(r*c)
{ }

Are you sure the problem is in the Matrix::operator+? Could be in Matrix::operator=. Also, Matrix::operator+ probably should be a const member function. — Eljay
– Eljay, Commented Jan 15, 2018 at 13:53
Smells like undefined behaviour. Please show a minimal reproducible example — Jabberwocky
– Jabberwocky, Commented Jan 15, 2018 at 13:53
Maybe a vector index out of range. Compiling in debug mode and running under a debugger will probably relveal some information. — Jabberwocky
– Jabberwocky, Commented Jan 15, 2018 at 13:59
as @Eljay pointed, it seems like problem is in = operator, cause you do not have problem with Matrix P = M + N; (in this case = operator will not be called because of copy elision). And in cases you have problem, = operator will be called — Andrew Kashpur
– Andrew Kashpur, Commented Jan 15, 2018 at 14:04

GuyRT · Accepted Answer · 2018-01-15 16:04:21Z

0

I think the problem is with your assignment operator:

Matrix Matrix::operator=(const Matrix &other){
    A = other.A;
    m = other.m, n = other.n;
}

You declare it as returning a Matrix object, but don't actually return anything.

Solution (note the return type is now a reference):

Matrix &Matrix::operator=(const Matrix &other){
    A = other.A;
    m = other.m, n = other.n;
    return *this;
}

Note that the default compiler-generated assignment will do the right thing anyway. So a better solution is to use that one (within the class declaration):

Matrix &operator=(const Matrix &other) = default;

edited Jan 15, 2018 at 16:04

answered Jan 15, 2018 at 15:59

GuyRT

2,91715 silver badges8 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

YSC Over a year ago

m = other.m, n = other.n is ugly. (and yeah I know it comes from OP)

Collectives™ on Stack Overflow

Segmentation fault when using overloaded operator

1 Answer 1

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related