push_back crashed when using std::vector if the type only defined copy constructor with c++11

Question

The code like this:

#include <iostream>
#include <vector>

struct Foo
{
    int i;
    double d;
};

class Boo
{
public:
    Boo() : fptr(nullptr) 
    {
        std::cout << "Boo default construct..." << std::endl;
    }
    Boo(int i, double d):fptr(new Foo{i,d})
    {
    }

    Boo(const Boo &rhs) :fptr(new Foo{rhs.fptr->i,rhs.fptr->d})
    {
    }

    Foo *fptr;
};

int main(int argc, char const *argv[])
{
    std::vector<Boo> vec(1);
    Boo b(42, 10.24);
    vec.push_back(b);
    return 0;
}

Env: Ubuntu 16.4 LTS with gcc 5.4 g++ -std=c++11 test.cpp -o test -g

./test

If define move constructor,this will work good. I debuged it several hours but can not found what's wrong with it, maybe because some rules in c++ 11 that i did't know.

Could somebody help me? Thanks!

You didn't define a user-defined copy constructor or destructor. — PaulMcKenzie, Jan 27 '22 at 03:58
You've to free the memory you allocated via `new` using `delete` otherwise you'll have **memory leak** in your program. — Jason, Jan 27 '22 at 03:58
At the duplicate link, go to the **Managing resources** section. The code there is almost identical to what you are trying to do (and what is missing). — PaulMcKenzie, Jan 27 '22 at 04:00
The root reason of this problem is that the std::vector vec(1) has one element of Boo with fptr is null. When push_back(b), at that time the capcity is 1, so vector must reallocating to increase the capcity to accommodate more elements.At that time, copy constructor has been called and it will dereference nullptr. — Forlight, Jan 27 '22 at 06:16
if move constructor added, the move constructor will be called priority when reallocating. So this problem will be avoided.Otherwise, this code above is missing Destructor. — Forlight, Jan 27 '22 at 06:24

Drew Dormann · Answer 1 · 2022-01-27T04:25:37.547

0

Your Boo copy constructor derefernces a pointer fptr that may be null.

A std::vector::push_back may reallocate vector storage. Reallocating may use copy constructors.

That is what is happening in your program. You are copying a Boo that has a null pointer.

edited Jan 27 '22 at 04:25

answered Jan 27 '22 at 04:12

Drew Dormann

59,987
13
123
180

Thanks! Your answer is very simple but helped me a lot. – Forlight Jan 27 '22 at 06:18

Jason · Accepted Answer · 2022-01-27T05:10:05.080

The problem is that in the copy constructor of Boo you're dereferencing a nullptr. This is because when you wrote:

std::vector<Boo> vec(1); //this creates a vector of size 1 using Boo's default constrcutor

This is what happens due to the above statement:

The above statement creates a vector named vec of size 1 using Boo's default constructor.
This means that there is already 1 Boo object inside vec. Moreover since this Boo object was created using the default constructor, its fptr is set to nullptr.

Now, when you wrote:

vec.push_back(b);// this adds/push_back object b onto the vector vec so that now its size will be 2

Now, these are the important things that you have to note here:

The vector vec has already 1 Boo object inside it which has its fptr as nullptr.
Now you're adding one more element b into the vector, due to which reallocation may happen. And if/when this reallocation happens, the copy constructor of Boo will be used.
And when this copy constructor is used to copy the already present Boo object, it dereferences its fptr. But since that fptr is nullptr and we know that dereferencing a nullptr is undefined behavior, so this may cause the program to segmentation fault as in your case.

Solution

First, before dereferencing fptr you should check it is a null pointer or not.

Second, you must free the memory you allocated via new using delete. This can be done by adding a destructor for class Boo and using delete on fptr inside it.

If you don't free this memory then you will have memory leak in your program.

#include <iostream>
#include <vector>

struct Foo
{
    int i;
    double d;
};

class Boo
{
public:
    Boo() : fptr(nullptr) 
    {
        std::cout << "Boo default construct..." << std::endl;
    }
    Boo(int i, double d):fptr(new Foo{i,d})
    {
        std::cout <<"Boo parameterized const"<<std::endl;
    }

    Boo(const Boo &rhs) 
    {
        std::cout <<"Boo copy const"<<std::endl;
        //add a check before dereferencing
        if(rhs.fptr!= nullptr)
        {
           fptr = new Foo{rhs.fptr->i,rhs.fptr->d};
           std::cout<<"i: "<<rhs.fptr->i <<" d: "<<rhs.fptr->d<<std::endl;
        }
        else 
        {
            std::cout<<"nullptr found"<<std::endl;
        }
        
       
    }

    Foo *fptr;
    //add destructor
    ~Boo()
    {
        std::cout<<"Boo destructor"<<std::endl;
        //for debugging check if we got a null pointer
        if(fptr!=nullptr) //just a check before deleting, you can remove this. This is for printing(debugging) when we get a nullptr
        {
            delete fptr;
            fptr = nullptr;
            std::cout<<"delete done"<<std::endl;
        }
        else 
        {
            std::cout<<"null pointer found while delete"<<std::endl;
        }
        
    }
};

int main(int argc, char const *argv[])
{
    std::vector<Boo> vec(1);
    Boo b(42, 10.24);
    vec.push_back(b);
    return 0;
}

push_back crashed when using std::vector if the type only defined copy constructor with c++11

2 Answers2

Solution