A strange stack overflow when to call destructor in Visual studio 2015

Question

I write a simple string class in cpp, but when I add some new features something bad happened. The stack overflow when to call the object s3 destructor. So I spent an hour to fix it, but I did not find something wrong with my code locially and the grammer is correct. So I ask my friend to recode it to help me to fix it, but when he recoded it on his Mac. There was no bad thing happened in his program. So I am REALLY REALLY confused about it and need help to fix it, if there is something wrong with my codes in truth. If not, maybe I should comment on issue Visual Studio 2015. :(

This is MySTLString.h

class MySTLString
{
public:
    // Default constructor
    MySTLString();                                  

    // Constructs the string with count copies of character ch. 
    MySTLString(size_t count, char ch);     

    // Convert char consequence to MySTLString
    MySTLString(const char *s);

    // Copy constructor 
    MySTLString(const MySTLString &s);

    // Destructor
    ~MySTLString();


    using size_type = size_t;       
    using CharT = char;                         // the character type
    using value_type = char;                    // the value_type
    using reference = value_type;               // reference 
    using const_reference = const value_type&;  // const reference

    // Returns reference to the character at specified location pos.
    reference at(size_type pos);
//  const_reference at(size_type pos) const;    // I do not know how to convert a reference to const_reference when to return 

    // Returns reference to the first character
    CharT& front();
    const CharT& front() const;

    // Returns reference to the last character, equivalent to operator[](size() - 1)
    CharT& back();
    const CharT& back() const;

    // Returns pointer to the underlying array serving as character storage.
    CharT* data();
    const CharT* data() const;

    // Operator assignment overloaded
    MySTLString& operator=(const MySTLString &s);

    // Operator [] overloaded
    reference operator[](size_type pos);
    const_reference operator[](size_type pos) const;

private:
    char* data_;
    int length_;
};

This is MySTLString.cpp

#include "MySTLString.h"
#include <cstring>
#include <iostream>
#include <stdexcept>

// Construct a empty string(zero size and unspecified capacity).
MySTLString::MySTLString():data_(nullptr), length_(0)
{

}

// Constructs the string with count copies of character ch.
MySTLString::MySTLString(size_t count, char ch)
{
    length_ = count;
    if (count == 0)
    {   
        data_ = nullptr;
        return;
    }
    else    // when count is not 0
    {
        data_ = new char[length_];
        for (size_t i = 0; i < count; ++i)
        {
            data_[i] = ch;
        }
    }

}

// Constructs the string with contents initialized 
// with a copy of the null-terminated character string pointed to by s.
// The length of the string is determined by the first null character. 
MySTLString::MySTLString(const char *s)
{
    length_ = strlen(s);
    if (length_ == 0)
    {
        data_ = nullptr;
        return;
    }
    data_ = new char[length_];
    strcpy(data_, s);
}

// Copy constructor. 
// Constructs the string with the copy of the contents of other.
MySTLString::MySTLString(const MySTLString &s)
{
    if (s.data_ == nullptr)
    {
        length_ = 0;
        data_ = nullptr;
    }
    else
    {
        length_ = strlen(s.data_);
        data_ = new char[length_];
        strcpy(data_, s.data_);
    }
}

// Destructor
// Free data_ pointer memory
MySTLString::~MySTLString()
{
    if (data_ != nullptr)
    {
        delete []data_;
    }
    std::cout << "length_ = " << length_ << std::endl;  // for test
}

// Returns a reference to the character at specified location pos.
// Bounds checking is performed, exception of type std::out_of_range will be thrown on invalid acess
MySTLString::reference MySTLString::at(size_type pos)
{
    if (pos >= strlen(data_))
    {
        throw std::out_of_range("pos is cross-border!\n");
    }
    return data_[pos];
}


// Returns reference to the first character
MySTLString::CharT& MySTLString::front()
{
    if (data_ == nullptr)
    {
        throw std::out_of_range("String is empty!\n");
    }
        return data_[0];
}

const MySTLString::CharT& MySTLString::front() const
{
    return this->front();
}

// Returns reference to the last character
MySTLString::CharT& MySTLString::back()
{
    if (data_ == nullptr)
    {
        throw std::out_of_range("String is empty!\n");
    }
    return data_[0];
}

const MySTLString::CharT& MySTLString::back() const
{
    return this->back();
}

// Returns pointer to the underlying array serving as character storage.
// The pointer is such that the range[data(); data()+strlen(data_)] is valid 
// in it correspond to the values stored in the string
MySTLString::CharT* MySTLString::data() 
{
    if (data_ == nullptr)
    {
        throw std::out_of_range("String is empty!\n");
    }
    return data_;
}

const MySTLString::CharT* MySTLString::data() const
{
    return this->data();
}

// Operator= overloaded
// Replace the contents with a copy of str.
// If *this and str are the same object, this function has no effect
MySTLString& MySTLString::operator=(const MySTLString &s)
{
    // If *this and str are the same object, this function return *this
    if (this == &s)
    {
        return *this;
    }
    if (s.length_ == 0)
    {
        length_ = 0;
        data_ = nullptr;
        return *this;
    }
    char* temp = s.data_;   // copy *s.data_
    delete data_;                       // free old memory
    data_ = new char[s.length_];            // copy data to data_ member
    strcpy(data_, temp);
    length_ = s.length_;
    return *this;                       // return this object
}

// Operator[] overloaded
// Returns a reference to the character at specified location pos. 
// No bounds checking is perfromed.
MySTLString::reference MySTLString::operator[](size_type pos)
{
    return this->at(pos);
}

MySTLString::const_reference MySTLString::operator[](size_type pos) const
{
    return this->operator[](pos);
}

This is TestMySTLString.cpp(PS: I have delete other normal functions)

#include "MySTLString.h"
#include <iostream>

using std::cout;
using std::endl;

int main(void)
{
    // test constructor that convert char consequence to MySTLString
    MySTLString s3("qwe");
    return 0;
}

This is the pic when to call the destructor

This is the pic of stack information when to call the destructor

Answer 1

While you do have infinite recursive calls (as explained in those other answers) it isn't an answer to the question you are asking. The error you are getting is a heap corruption bug.

The debug CRT of Visual Studio allocates guard bytes to the left and right of heap-allocated memory, and fills them with specific byte patters. When the memory gets deleted, those guard bytes are compared against the byte pattern. If they do not match, you get to see the heap corruption dialog, because you wrote outside the allocated memory.

The bug is not in your d'tor. The d'tor is just the place, where the heap corruption is detected, because it releases the heap-allocated memory.

There are several bugs with the same pattern in your code: strlen returns the number of characters not including the zero terminator. When you strcpy into the allocated array, the zero terminator gets written just outside the allocated memory.

You need to change the following code

MySTLString::MySTLString(const char *s)
{
    length_ = strlen(s);
    if (length_ == 0)
    {
        data_ = nullptr;
        return;
    }
    data_ = new char[length_];
    strcpy(data_, s);
}

to

MySTLString::MySTLString(const char *s)
{
    length_ = strlen(s);
    if (length_ == 0)
    {
        data_ = nullptr;
        return;
    }
    data_ = new char[length_ + 1]; // Allocate enough memory to account for zero terminator
    strcpy(data_, s);
}

Make sure to update the other occurrences, where you call strlen accordingly.

---

Other random notes:

• You don't need to check for nullptr before calling delete[]. While not harmful, it's not required either. Simply use delete[] data_; in your d'tor.
• There were no error reports for your friend, because they were presumably using XCode. XCode will not report heap corruption errors, unless it is specifically set up to do so (the tedious process is explained at Enabling the Malloc Debugging Features). And even then it's mostly useless.

Answer 2

Have a look in your visual Studio 2015 Build output. Mine says

data': recursive on all control paths, function will cause runtime stack overflow

back': recursive on all control paths, function will cause runtime stack overflow

front': recursive on all control paths, function will cause runtime stack overflow

operator[]': recursive on all control paths, function will cause runtime stack overflow

Answer 3

const MySTLString::CharT& MySTLString::front() const
{
    return this->front();
}

will result in infinite recursion, causing stack overflow. If you want to re-use implementaion of the non-const version, you will have to use:

const MySTLString::CharT& MySTLString::front() const
{
    return const_cast<MySTLString*>(this)->front();
}

Make the same change to MySTLString::back() and MySTLString::data().