CODEWITHSUNDEEP
c++c++11memory-managementvalgrind
Pdksock
Pdksock

Reputation: 1030

Valgrind: Invalid read of size 8 error

I get the following when I run my program through valgrind 

==29852== Invalid read of size 8
==29852==    at 0x4EDEA50: std::_Rb_tree_increment(std::_Rb_tree_node_base const*) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==29852==    by 0x414EEA: std::_Rb_tree_const_iterator<std::pair... >::operator++() (stl_tree.h:284)
==29852==    by 0x4268CF: Tree::removeConstantsPair(std::set...) (Tree.h:65)
==29852==    by 0x4239C4: yy_reduce(yyParser*, int) (parser.y:251)
==29852==    by 0x425F6D: Parse(void*, int, Token*, Tree*) (parser.c:1418)
==29852==    by 0x404837: main (main.cpp:95)

Line 65 in Tree.h is

inline void removeConstantsPair(set<pair<string, string>>& vec){
    set<string>::iterator itr; 
    for(auto &v : vec){ //This is line 65
        itr = domainList.find(v.first);
        if(itr != domainList.end())
            vec.erase(v);
    }
}

However the Leak summary says that there is no memory that is lost. From what I understand invalid read happens if I am reading from a memory that has been freed so in my case &vec must have been freed before. My program runs though and does not crash.

Can someone explain why is there a memory read error.

Upvotes: 3

Views: 3255

Answers (3)

PaulMcKenzie
PaulMcKenzie

Reputation: 35440

The following small program shows the error when run under the Visual Studio compiler:

#include <string>
#include <set>
#include <map>

std::set<std::string> domainList = {"abc", "123", "456"};

using namespace std;

void removeConstantsPair(set<pair<string, string>>& vec)
{
    set<string>::iterator itr; 
    for(auto &v : vec)
    { 
        itr = domainList.find(v.first);
        if(itr != domainList.end())
            vec.erase(v);  // <--erasing this iterator makes it invalid
    }
}

int main()
{
    std::set<std::pair<string, string>> vec = {make_pair("abc", "xyz"), 
                                               make_pair("456", "xyz"),
                                               make_pair("000", "xyz")};
    removeConstantsPair(vec);
}

The Visual Studio debug runtime asserts with a "Expression map/set iterator not incrementable" when the increment is attempted on the erased iterator in the for loop.

enter image description here

So the solution is to make sure that the iterator that is going to be incremented was not the one that was erased. 

void removeConstantsPair(set<pair<string, string>>& vec)
{
    set<string>::iterator itr;
    auto iterSet = vec.begin();
    while (iterSet != vec.end())
    {
        itr = domainList.find((*iterSet).first);
        if (itr != domainList.end())
        {
            auto erasedIter = iterSet;
            ++iterSet;
            vec.erase(erasedIter);
        }
        else
            ++iterSet;
    }
}

Upvotes: 2

R Sahu
R Sahu

Reputation: 206607

The problem is most likely caused by the line:

        vec.erase(v);

Using a range for loop and deleting an item from the container is not a good idea. Change your loop to:

for ( auto iter = vec.begin(); iter != vec.end(); /* Empty on purpose*/ )
{
   if(domainList.find(iter->first) != domainList.end())
   {
      iter = vec.erase(iter);
   }
   else
   {
      ++iter;
   }
}

Upvotes: 5

Sam Varshavchik
Sam Varshavchik

Reputation: 118340

An "invalid memory read" can also happen for many other reasons, other than the ones that are stated in the question.

One example:

A request to allocate memory, via new typically allocates a little bit more memory than what's needed for the instance of a new class. For example, a particular C++ implementation might allocate memory in multiple of 16 bytes. So a new for an instance of a class whose sizeof would return 12 will actually end up allocating 16 bytes, and an attempt to read past the end of the actual instantiated object will end up getting flagged, correctly, by valgrind as an invalid memory read.

Upvotes: 1

Related Questions