A breakpoint instruction (__debugbreak() or similar.. Debug Assertion Failed: _CrtIsValidHeapPointer(block)

Question

I'm getting this error, I dissected the code even to make sure that I didn't miss a step. I added a extra size when allocating to make sure no memory leakage, I deleted [] . can you guys see what could cause those issues? maybe I am missing something

This is the Error CPP file.


#define _CRT_SECURE_NO_WARNINGS
#include<iostream>
#include<cstring>
#include<string>
#include <fstream>

#include "Error.h"

using namespace std;
namespace sdds {

    Error::Error()
    {
        this->m_message = nullptr;
    }

    Error::Error(const char* errors) {
        
        
        this->operator=(errors);
    }
    int Error::clear() {
    
        
            delete[] this->m_message;
            this->m_message = nullptr;
        
        
            return 0;
        
    }


    Error& Error::operator=(const char* error)
    {
    

        if (error == nullptr)
        {

            this->m_message = nullptr;
        }
        else
        {
            
            delete[] m_message;
            
                int size = strlen(error);
                int size2 = size + 1;
            m_message = new char[size2];
            strcpy(m_message, error);
        }
        return *this;
    }


    Error::~Error()
    {
        

        clear();
        
    }


    Error::operator bool() const {
    
        bool flag3;
        
    
        if (this->m_message != nullptr)
        {
                flag3 = true;
            }
            else {
                flag3 = false;
            }
            return flag3;
    }



    std::ostream& Error::display(ostream& ostr ) const {

        if (this->m_message != nullptr)
        {
            ostr << this->m_message;
        }
        
        return ostr;
    }

    std::ostream& operator<<(std::ostream& ostr, const Error& right) {
        return right.display(ostr);

    }
    

};

This is the header Error File, yes I was told I have bad habit that using namespace std.

``
#ifndef SDDS_ERROR_H
#define SDDS_ERROR_H
#include <iostream>

using namespace std;
namespace sdds
{

    class Error {
        char *m_message;

    public:
        Error();
        Error(const char* errors);
        Error& operator=(const char* error);
        operator bool() const;
        ostream& display(ostream& ostr=cout) const;
        int clear();

        ~Error();
    };
    std::ostream& operator<<(std::ostream& ostr, const Error& right);

}
#endif
``

This is the main file

`#define _CRT_SECURE_NO_WARNINGS
#include <iostream>
#include <fstream>
#include <sstream>

#include "Error.h"
using namespace std;
using namespace sdds;


void line(char fill);
bool T1();

int main() {
   line('1');
   bool ok = T1();

   return 0;
}


bool T1() {
   bool passed = true;
   Error F;
   Error G("Error G has a message");
   Error X("Error X has a message too");
   Error C = G;
   X = G;
   if(!F) {
      cout << "This is correct >" << F << "<" << endl;
   } else {
      cout << "This message should not get printed: " << F << endl;
      passed = false;
   }
   if(G) {
      cout << "This is correct >" << G << "<" << endl;
   } else {
      cout << "This message should not get printed: " << G << endl;
      passed = false;
   }
   if(X) {
      cout << "This copy assign is correct >" << X << "<" << endl;
   } else {
      cout << "This message should not get printed: " << X << endl;
      passed = false;
   }
   if(C) {
      cout << "This copy is correct >" << C << "<" << endl;
   } else {
      cout << "This message should not get printed: " << C << endl;
      passed = false;
   }
   G.clear();
   if(!G) {
      cout << "This is correct >" << G << "<" << endl;
   } else {
      cout << "This message should not get printed: " << G << endl;
      passed = false;
   }
   return passed;


}



These are all the functions i tried and their errors.I plyed around with the destructor to see what could cause the issue
```

Error::~Error() Leakage
      {
            
            m_message = nullptr;

                  delete[] m_message;
            
                  m_message = nullptr;
            

      }

Error::~Error() Segmentaion fault
      {
            

           clear();
      }

Error::~Error() Segmentaion fault
      {
            

         if (m_message != nullptr)
                  {
                        clear();
                  }
      }

Error::~Error() Segmentaion fault
      {
            
           
                  delete[] m_message;
            
                  m_message = nullptr;
            

      }

Error::~Error() Segmentaion fault
      {
            
           
                  delete m_message;
            
                  m_message = nullptr;
            

      }

Error::~Error() Segmentaion fault
      {
            
             delete[] m_message;
            
                  delete m_message;
            
                  m_message = nullptr;
            

      }

Here's your code with a small tweak (removed call to `line`), a reasonable level of diagnostic output and a couple sanitizers enabled: https://godbolt.org/z/Pq1zj8jrP . Definitely resolve the warnings as they pretty much spill the beans on what went wrong. Never ignore compiler warnings. They are the first line of defense against bugs. — user4581301, Mar 28 '23 at 18:46
`Error C = G;` This uses the Error copy constructor. Since you didn't write one, the compiler uses a default one, which copies the raw pointer, and then you get double-deletion when C and G destruct. You need to follow the Rule of Three. Or use `std::unique_ptr` instead of a raw pointer, and then you don't need a custom destructor, which puts you in the Rule of Zero. — Raymond Chen, Mar 28 '23 at 18:51
Does this answer your question? [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — Raymond Chen, Mar 28 '23 at 18:54
@RaymondChen I actually have a copy constructor Error& operator=(const char* error); Im still getting it though. — roberto roldan, Mar 28 '23 at 19:07
That is not a copy constructor or a copy assignment operator. The Rule of Three is not being observed. — user4581301, Mar 28 '23 at 19:08
Replace `char *m_message;` with `std::string m_message;`. Then when that fixes the problem replace all of the remain `char *`s with `std::string` or `std::string const &`. — Richard Critten, Mar 28 '23 at 19:20
[What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) — Jesper Juhl, Mar 29 '23 at 00:23

user4581301 · Answer 1 · 2023-03-28T23:44:02.383

    Error::Error(const char* errors) {
        
        
        this->operator=(errors);
    }

Uses the assignment operator to set up the object before it is fully constructed. Pay special attention to m_message receiving no attention in this constructor. This is fatal because Error& Error::operator=(const char* error) will delete[] m_message; before it's been nulled or pointed at a valid object. Invoking delete or delete[] on an uninitialized pointer results in undefined behaviour, manifested in this case by the mistake being intercepted by a debugging tool and the program being aborted. Initializing m_message to a safe value,

    Error::Error(const char* errors): m_message(nullptr) {
        
        
        this->operator=(errors);
    }

for example, would solve this, but it is probably better if the constructor did the work itself or both it and operator= called a function that did the grunt work.

    class Error {
        char *m_message;
        void allocate_and_copy(const char* error);
        ...
    };

and

    void Error::allocate_and_copy(const char* error)
    {
        int size = strlen(error);
        m_message = new char[size + 1];
        strcpy(m_message, error);
    }
    Error::Error(const char* errors){
        allocate_and_copy(errors);
    }
    Error& Error::operator=(const char* error)
    {
        delete[] m_message; // both cases must clean up any existing allocation
        if (error == nullptr)
        {
            this->m_message = nullptr;
        }
        else
        {
            allocate_and_copy(error);
        }
        return *this;
    }

Now we're in position to deal with the second mistake: The Rule of Three has not been observed. This can be avoided using smarter resource management like std::string or perhaps smart pointers, but if this is an assignment forbidding the use of modern, but ancient in C++, concepts like std::string, you need to provide a copy constructor and a copy assignment operator. To simplify this task, I recommend reading What is the copy-and-swap idiom?

Using Copy-and-Swap and the helper function above, the copy constructor and assignment operator could be as simple as

    Error::Error(const Error& src)
    {
        allocate_and_copy(src.m_message);
    }
    Error& Error::operator=(Error src)
    {
        std::swap(m_message, src.m_message);
        return *this;
    }

Side note: If you keep or calculate the size of the existing error message, you can prevent allocating new storage for error messages that are smaller or the same size.

A breakpoint instruction (__debugbreak() or similar.. Debug Assertion Failed: _CrtIsValidHeapPointer(block)

1 Answers1