Debug Assertion Failed while creating Object

Question

I'm trying to build a simple matrix algebra application for University. When trying to add data from input files I use the following Method:

Matrix createMatrix(string filename, int rowRestriction, int colRestriction)
{
    try{
    ifstream file;
    string line = "";
    vector<string> curLine;
    int cols = -1, rows = 0;
    vector<vector<double>> values;
    file.open(filename);
    if(!file.is_open())
    {
        cout << "No file could be loaded, please check whether the input file is placed inside the working directory.\n";
        throw 1;
    }
    while(getline(file,line))
    {
        rows+=1;
        curLine = split(line);
        if(cols == -1)
        {
            cols = curLine.size();
            cout << "Matrix appears to have " << cols << " Columns.\n";
            if(colRestriction != NO_RESTRICTION && cols != colRestriction)
            {
                cout << "The Matrix you provided does not fulfill the column restriction of " << colRestriction << " Columns, please check your input file.\n";
                throw 2;
            }
        }
        else if(cols != curLine.size())
        {
            cout << "Invalid Matrix supplied. Varying amount of columns. Please check input file " << filename << ".\n";
            throw 3;
        }
            cout << "Saving Row "<<rows<<"\n";
            values.resize(rows);
            values[rows-1].resize(cols);
            for(int i = 0; i < curLine.size(); i++)
            {
                if(isValidNumber(curLine[i]))
                    try
                    {
                        values[rows-1][i] = atof(curLine[i].c_str());
                    }
                    catch(int e)
                    {
                        cout << "Exception No. " << e << " has occurred. Presumably your input file does not contain valid floating point numbers.\n";
                        throw 4;

                    }
                else
                {
                    cout << "Your file contains invalid characters, please check your input file \"" << filename << "\".\n";
                    throw 5;
                }
            }
    }
    if(rowRestriction != NO_RESTRICTION && rowRestriction != rows)
    {
        cout << "The Matrix you provided does not fulfill the row restriction of " << rowRestriction << " Rows, please check your input file.\n";
        throw 6;
    }
    cout << "Matrix Data has been read successfully, your matrix has " << rows << " Rows and " << cols << " Columns. It is " << ((rows==cols)?"":"not ") << "quadratic.\n";
    Matrix m = Matrix(rows, cols, values);
    m.setValidity(true);
    return m;
    }
    catch(int e)
    {
        cout << "Exception No. " << e << "occurred.\n";
    }
}

Here is the Constructor for 'Matrix':

Matrix::Matrix(int rows, int cols, vector<vector<double>> data)
{
    this->rows = rows;
    this->cols = cols;
    this->data = data;
}

And here's the header file:

#pragma once
#include <vector>
using std::vector;
class Matrix
{
public:
    Matrix(int rows, int cols, vector<vector<double>> data);
    ~Matrix(void);
    int getCols();
    int getRows();
private:
    int rows, cols;
    vector<vector<double>> data;
};

I receive the following Error Message - it only appears when the line Matrix m = Matrix(rows, cols, values); is added (see above).

    ---------------------------
Microsoft Visual C++ Runtime Library
---------------------------
Debug Assertion Failed!

Program: ...al studio 2012\Projects\Matrixalgebra\Debug\Matrixalgebra.exe
File: f:\dd\vctools\crt_bld\self_x86\crt\src\dbgheap.c
Line: 1322

Expression: _CrtIsValidHeapPointer(pUserData)

For information on how your program can cause an assertion
failure, see the Visual C++ documentation on asserts.

(Press Retry to debug the application)

---------------------------
Abort   Retry   Ignore   
---------------------------

I just know that it's some rookie mistake, but I've been trying around for quite a while now without success. The algorithm itself works just fine up until the last few lines.

EDIT: Changed OP to reflect new problem

EDIT2: This new error throws because of my deconstructor, see below

Matrix::~Matrix(void)
{
    delete &data;
}

Why is this the case - I'd really appreciate an explanation for this, or some learning material on the matter.

1. I think you used `capacity` where you meant `size`. Better check that. 2. Turn on compiler warnings. They would tell you never to return the address of a local variable. — Dark Falcon, Feb 21 '14 at 15:40
possible duplicate of [Debug Assertion Failed! Expression: \_BLOCK\_TYPE\_IS\_VALID](http://stackoverflow.com/questions/1102123/debug-assertion-failed-expression-block-type-is-valid) — starsplusplus, Feb 21 '14 at 15:41
Run your favorite memory profiler to find places where you double-release something, or write over an object. The error that you get is an indication that something went terribly wrong some time ago, in some other part of your code. — Sergey Kalinichenko, Feb 21 '14 at 15:45

score 1 · Answer 1 · answered Feb 21 '14 at 15:44

1

Your issue is that you're returning the address of a local variable in your "createMatrix" function.

Why does createMatrix return a Matrix*? You avoided pointers OK, up until then. Your createMatrix should have returned a Matrix object, not a pointer.

Matrix createMatrix(string filename, int rowRestriction, int colRestriction)
{
    //....
    Matrix m = //;
    return m;
}

That is what I would have expected, or something similar to that.

answered Feb 21 '14 at 15:44

PaulMcKenzie

34,698
4
24
45

According to what I've tried earlier, I needed to return a Matrix* since I wouldn't have been able to do `return NULL;` earlier - how can I still do this AND return the actual object? – Dennis Röttger Feb 21 '14 at 15:45
Regardless of your goal, you cannot return the address of a local variable. If you cannot create the Matrix, then throw an exception (that would be one of many solutions to your problem). – PaulMcKenzie Feb 21 '14 at 15:47

cli_hlt · Accepted Answer · 2014-02-21T20:49:11.127

1

The problem is here:

Matrix *createMatrix(string filename, int rowRestriction, int colRestriction) {  
    // ...
    Matrix m = Matrix(rows, cols, values);
    return &m;
}

You are returning a pointer to a variable that was created on the stack within the function. By the time your function returns, that pointer will be invalid and using it will cause undefined behavior. Luckily, the debug runtime tells you this by throwing an assertion.

(Edit) Possible solutions for signaling an error case:

A. Allocate storage for your matrix dynamically

Matrix *createMatrix(string filename, int rowRestriction, int colRestriction) { 
  // ... 
  Matrix *m = new Matrix(rows, cols, values); 
  return &m; 
}

Pros: does what you wants, don't need to change your code

Cons: Creates a matrix on the heap, who is going to free it?

B. Modify your function

bool loadMatrixFromFile(string filename, int rowRestriction, int colRestriction, Matrix& m) { 
  // ... 
  // If something goes wrong -> return false
  Matrix newMatrix(rows, cols, values); 
  m = newMatrix;
  return true; 
}

// Call like this:
Matrix m;
bool retVal = loadMatrixFromFile("",bla , bla, m);

Pros: I think this is a way that can be recommended, avoids the overhead of copying the matrix when RVO cannot be used, allows you to signal an error condition. Plus: the function name now also describes what it actually does :-)

Cons: None that I can think of.

edited Feb 21 '14 at 20:49

answered Feb 21 '14 at 15:44

cli_hlt

7,072
2
26
22

So my options are to either create the object earlier or to simply return the object as opposed ot the pointer, correct? What can I do so that I return an object, but can still do `return NULL;` earlier in the code? – Dennis Röttger Feb 21 '14 at 15:47
When you want to return a pointer that you can use, you need to allocate the memory for it dynamically (using `new`). In your case, I would avoid it altogether and implement a method like `bool readMatrixFromFile(string filename, int rowRestriction, int colRestriction, Matrix&m)` - there you have the possibility to signal an error. As a side effect, your function more clearly states what it is actually supposed to do :-) – cli_hlt Feb 21 '14 at 15:54
Thank you! As a first step I changed the method to return an actual object and throw errors as opposed to returning flags, but now there's a new problem that I've outlined in the OP. – Dennis Röttger Feb 21 '14 at 16:01
Your `data` member is **NOT** dynamically allocated with `new`. Hence: **DO NOT** delete it using `delete`. Get a book on C++, please ;-) – cli_hlt Feb 21 '14 at 16:07
@DennisRöttger - Imagine if C++ was built that way, where whenever we declare a variable and set it to a value, we need to "delete" it somewhere in the code. You didn't call "delete" with that internal vector data member, right? So what would make C++ change its rules for your Matrix object? The bottom line is that unless you explicitly used "new" would you need or worry about calling "delete". – PaulMcKenzie Feb 21 '14 at 16:32

score 0 · Answer 3 · answered Feb 21 '14 at 15:50

0

This is absolutley an antipattern: return a pointer (or a reference) from ab object that resides in autoamtic memory, i.e., on the stack. Next thing that happens after the return, it destroys the variable contents - anything.

Matrix * x(...){
    ...
    Matrix m = ...;
    return &m;
}

In your case you might use new and return a pointer to a piece of dynamic memory on the heap:

    Matrix * m = new Matrix( nrow,... );
    return m;

answered Feb 21 '14 at 15:50

laune

31,114
3
29
42

And then we get into "who is responsible for destroying the memory" issues. Might as well create a Matrix and return it, as suggested in the other answers. – PaulMcKenzie Feb 21 '14 at 15:52

Debug Assertion Failed while creating Object

3 Answers3