Search code examples
c++stringmemory-leaksnvccclass-members

Valgrind identifies memory leaks when using string-type member (compiling with nvcc)


I'm not sure if it's a bug or not, but when I use string-type members inside structures or classes, valgrind identifies memory leaks. I've tried to build a simple code based on my own application, I'm sorry if it is still big...

//  ====================================================================
#include <iostream>
#include <string>
#include <sstream>
#include <vector>
using namespace std;
//  ====================================================================
string int2str(const int &i) {
    return static_cast<ostringstream*>(
        &(ostringstream() << i))->str();
}
//  ====================================================================
class P;
//  ====================================================================
struct Node { 
    virtual char isType() const = 0;
};
//  ====================================================================
struct X : Node {
    string st;
    int id;
    X(const string &_st, const int &_id);
    char isType() const { return 'x'; };
    // Those member functions are after class P declaration:
    P use_as_P();
    P use_as_P(const P &arg0);
    P use_as_P(const P &arg0, const P &arg1);
};

X::X(const string &_st, const int &_id) : st(_st), id(_id) { }
//  ====================================================================
class P {
  friend struct X;
  private:
    Node *node;
    vector<P> children;
  public:
    P() : node(NULL) {};
    P(const P &source);
    void swap(P &other);
    string print_this();
    ~P();
};

P::P(const P &source) {
    this->children = source.children;
    switch(source.node->isType()) {
      case 'x':
        this->node = new X(static_cast<X*>(source.node)->st, 
            static_cast<X*>(source.node)->id);
        break;
    }
}

void P::swap(P &other) {
    std::swap(this->node, other.node); 
    std::swap(this->children, other.children); 
}

string P::print_this() {
    string msg = "( ";
    msg += static_cast<X*>(this->node)->st;
    msg += int2str(static_cast<X*>(this->node)->id);
    msg += " ";
    for(size_t i = 0; i < this->children.size(); i++)
        msg += children.at(i).print_this();
    msg += ") ";
    return msg;
}

P::~P() {
    if(this->node != NULL)
        delete node;
    this->children.clear();
}
//  ====================================================================
P X::use_as_P() {
    P ast_aux;
    ast_aux.node = new X(this->st,this->id);
    return ast_aux;
}

P X::use_as_P(const P &arg0) {
    P ast_aux;
    ast_aux.node = new X(this->st,this->id);
    ast_aux.children.push_back(arg0);
    return ast_aux;
}

P X::use_as_P(const P &arg0, const P &arg1) {
    P ast_aux;
    ast_aux.node = new X(this->st,this->id);
    ast_aux.children.push_back(arg0);
    ast_aux.children.push_back(arg1);
    return ast_aux;
}
//  ====================================================================
//  ** MAIN **
//  ====================================================================
int main(int argc, char **argv)
{
    X a("how",0), b("what",1), c("why",2), d("when",3);
    P testing = a.use_as_P(b.use_as_P(c.use_as_P()),d.use_as_P());
    cout << testing.print_this() << endl;
    return 0;
}
//  ====================================================================

Compiling with:

nvcc -arch sm_20 -o LEAK_test_with_string LEAK_test_with_string.cu

And here goes the valgrind's analysis:

==5877== Memcheck, a memory error detector
==5877== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==5877== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==5877== Command: ./LEAK_test_with_string
==5877== 
( how0 ( what1 ( why2 ) ) ( when3 ) ) 
==5877== 
==5877== HEAP SUMMARY:
==5877==     in use at exit: 114 bytes in 4 blocks
==5877==   total heap usage: 47 allocs, 43 frees, 3,701 bytes allocated
==5877== 
==5877== 28 bytes in 1 blocks are definitely lost in loss record 1 of 4
==5877==    at 0x4C2A879: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5877==    by 0x5516F38: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x5518640: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x5518A57: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x403691: main (in /home/igor/projects/system_modeling/LEAK_test_with_string)
==5877==  : st(_st), id(_id) {}
==5877== 28 bytes in 1 blocks are definitely lost in loss record 2 of 4
==5877==    at 0x4C2A879: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5877==    by 0x5516F38: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x5518640: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x5518A57: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x40375A: main (in /home/igor/projects/system_modeling/LEAK_test_with_string)
==5877== 
==5877== 29 bytes in 1 blocks are definitely lost in loss record 3 of 4
==5877==    at 0x4C2A879: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5877==    by 0x5516F38: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x5518640: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x5518A57: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x4036F7: main (in /home/igor/projects/system_modeling/LEAK_test_with_string)
==5877== 
==5877== 29 bytes in 1 blocks are definitely lost in loss record 4 of 4
==5877==    at 0x4C2A879: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5877==    by 0x5516F38: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x5518640: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x5518A57: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==5877==    by 0x4037BD: main (in /home/igor/projects/system_modeling/LEAK_test_with_string)
==5877== 
==5877== LEAK SUMMARY:
==5877==    definitely lost: 114 bytes in 4 blocks
==5877==    indirectly lost: 0 bytes in 0 blocks
==5877==      possibly lost: 0 bytes in 0 blocks
==5877==    still reachable: 0 bytes in 0 blocks
==5877==         suppressed: 0 bytes in 0 blocks
==5877== 
==5877== For counts of detected and suppressed errors, rerun with: -v
==5877== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 2 from 2)

The "it's a bug" wondering comes when I change the C++ string member into a plain C char* member. One just need to change this part of the above written code:

//  ====================================================================
struct X : Node {
    char st[6]; // <=============== HERE!
    int id;
    X(const string &_st, const int &_id);
    char isType() const { return 'x'; };
    // Those member functions are after class P declaration:
    P use_as_P();
    P use_as_P(const P &arg0);
    P use_as_P(const P &arg0, const P &arg1);
};

X::X(const string &_st, const int &_id) : id(_id) { // <=============== HERE!
    strcpy(st, _st.c_str());  // <=============== HERE!
}
//  ====================================================================

Note that I still use string in my code, but now there isn't any string member declared. This way, valgrind doesn't complain anymore:

==5977== Memcheck, a memory error detector
==5977== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==5977== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==5977== Command: ./LEAK_test_without_string
==5977== 
( how0 ( what1 ( why2 ) ) ( when3 ) ) 
==5977== 
==5977== HEAP SUMMARY:
==5977==     in use at exit: 0 bytes in 0 blocks
==5977==   total heap usage: 57 allocs, 57 frees, 3,986 bytes allocated
==5977== 
==5977== All heap blocks were freed -- no leaks are possible
==5977== 
==5977== For counts of detected and suppressed errors, rerun with: -v
==5977== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)

Does anybody have an insight on this? I mean, is this a bug that should be reported or is there something that I'm actually missing in my string-version code?


Solution

  • Since no one's bitten the bullet I'll expand my comment: Your Node class does not have a virtual destructor, this makes the following line

    delete node;
    

    invoke undefined behaviour - this will only call the destructor for Node, the destructor for X is never called.

    The solution is simple, provide a virtual destructor for Node:

    struct Node { 
        virtual char isType() const = 0;
        virtual ~Node() =default;
        // or virtual ~Node(){} if your compiler does not support defaulted functions.
    }
    

    You can now safely delete base pointers of Node that point to inherited classes.

    The reason for the memory leak when using a std::string is because when you delete the base pointer node, the destructor for X is not called, and so neither is the destructor for st. The char array version doesn't leak because node points to a simple block of memory, and X in this case doesn't have any complex members that require destruction, so delete node luckily releases all of the memory allocated for X and its members. Remember that this is undefined behaviour however and the compiler can legitimately do whatever it wants to.

    If you have a base class with even a single virtual function, always add a virtual destructor. You can make a case for omitting a virtual destructor if you are absolutely sure you won't be deleting inherited classes using a pointer to the base class, but it's safer to just add one anyway just in case (the gcc compiler flag -Weffc++ will also tell you as much).