SIGBUS when trying to increment a std::map iterator

Question

I am debugging a big C++98 application and I am obtaining a SIBGUS error when a method tries to increment a std::map::iterator.

By putiing traces, I have discovered that the method in question removes elements from the mentioned map (indireclty, by calling other methods that call other methods and so on...), so I suspect that the problem is to been iterating over the map while deleting its elements.

I have been searching the proper way to iterate over a std::map and delete items safelly, and I have found this:

for (auto it = m.cbegin(); it != m.cend() /* not hoisted */; /* no increment */)
{
  if (must_delete)
  {
    m.erase(it++);    // or "it = m.erase(it)" since C++11
  }
  else
  {
    ++it;
  }
}

Code quoted from How to remove from a map while iterating it?

I have some questions about this:

Is it actually necessary to distinguis between deleting elements or not, taking into account that the iterator is going to be increased in any case?

Is the following code snippet equivalent to the above one, in terms of safety?

for (auto it = m.cbegin(); it != m.cend() /* not hoisted */; /* no increment */)
{
  if (must_delete)
  {
    m.erase(it);
  }
  it++;
}

The method that is producing the SIGBUS follows the following pattern:

std::map<..., ...>::iterator it = myMap.begin(); // myMap is an instance attribute and can be accessed by any class method.

while(it != myMap.end() {
  if(somethingHappens())
    doSomethingThatMightDeleteMapElements(); // this can (or not) delete 'myMap' elements.
  it++; // The error occurs here
}

Since the deletion is performed by other method/s, I cannot distinguis if an element has been deleted or not (unless I return a boolean value or similar). Is this potentially unsafe?

`m.erase(it)` invalidates the iterator `it`, so the next time you evaluate `it != m.cend()` you have undefined behaviour. In other words, your snippet is not "equivalent to the above one", but "very very broken". — Kerrek SB, Dec 07 '17 at 16:31

score 2 · Accepted Answer · answered Dec 07 '17 at 16:37

Is the following code snippet equivalent to the above one, in terms of safety?

No, of course not, you cannot increase it after you passed it to map.erase() as that iterator is invalidated by that call. Difference is:

 map.erase(it++);

is logically equivalent to:

iterator tmp = it;
++it;
map.erase( tmp );

So in this case tmp is invalidated, but it is still valid.

Considering this code:

while(it != myMap.end() {
  if(somethingHappens())
    doSomethingThatMightDeleteMapElements(); // this can (or not) delete 'myMap' elements.
  it++; // The error occurs here
}

I think only viable way would be this:

while(it != myMap.end() {
  if(somethingHappens()) {
    key_type key = it->first();
    doSomethingThatMightDeleteMapElements(); // this can (or not) delete          'myMap' elements.
    it = myMap.upper_bound( key );
  } else
     it++;
}

score 0 · Answer 2 · answered Dec 07 '17 at 16:41

Since the deletion is performed by other method/s, I cannot distinguish if an element has been deleted or not (unless I return a boolean value or similar).

You certainly can. Keep the size of the map before the call to doSomethingThatMightDeleteMapElements. Get the the size of the map after the call to doSomethingThatMightDeleteMapElements. Then take appropriate action depending on whether they are equal or not.

while(it != myMap.end() {
  size_t size_before = myMap.size();
  size_t size_after = size_before;
  if(somethingHappens())
  {
    doSomethingThatMightDeleteMapElements(); // this can (or not) delete myMap  elements.
    size_after = myMap.size();
  }

  if ( size_before != size_after )
  {
    // Be safe. Iterate from the start again.
    it = myMap.begin();
  }
  else
  {
    it++; // The error occurs here
  }
}

SIGBUS when trying to increment a std::map iterator

2 Answers2