views:

172

answers:

7

Hi,

I'm trying to debug a legacy code written for Linux. Sometimes the application gets a segfault when it reaches the memcpy call in the following method:

std::vector<uint8> _storage;
size_t _wpos;
void append(const uint8 *src, size_t cnt)
{
  if (!cnt)
  return;

  if (_storage.size() < _wpos + cnt)
    _storage.resize(_wpos + cnt);
  memcpy(&_storage[_wpos], src, cnt);
  _wpos += cnt;
}

The values are as follows:

_storage.size() is 1000
_wpos is 0
*src points to an array of uint8 with 3 values: { 3, 110, 20 }
cnt is 3

I have no idea why this happens since this method gets called thousands of times during the application's runtime but it sometimes gets a segfault.

Any one has any idea how to solve this?

+4  A: 

Your code looks good in terms of the data that is written. Are you absolutely sure that you're passing in the right src pointer? What happens when you run the code with a debugger such as gdb? It should halt on the segfault, and then you can print out the values of _storage.size(), src, and cnt.

I'm sure you'll find that (at least) one of those is not at all what you're expecting. You might have passed an invalid src; you might have passed an absurdly large cnt.

Adam Rosenfield
I have extracted those values from a core dump file using GDB. However the _storage.size() value is my guess since i couldn't get its value there. But the other values are the exact values that caused the segfault
O. Askari
I see what you did there...
caf
+1  A: 

For the values you give, I can't see why that would segfault. It's possible that your segfault is a delayed failure due to an earlier memory management mistake. Writing past the end of the vector in some earlier function could cause some of the vector's internal members to be corrupted, or you may have accidentally freed part of the memory used by the vector earlier. I'd check the other functions that manipulate the vector to see if any of them are doing any suspicious casting.

Joseph Garvin
A: 

The only thing I can think of is that _storage.resize() fails (which should throw a bad_alloc exception).

Another alternative would be to append each value separately with a call to push_back() (probably far slower though).

Ephphatha
+1  A: 

I see the size of the vector increasing. I never see it decreasing.

Next to that, vector has expquisit memory management support builtin. You can insert your values right to the end:

vector.insert( src, src+cnt );

This will both expand the vector to the right size, and copy the values.

xtofl
The vector gets cleaned up in another method since it is a class member variable. I'll test insert and see what happens
O. Askari
A: 

I see one problem here. The memcpy() function copies n bytes from memory, so if cnt is the number of elements, you need a *sizeof(uint8) in the call to memcpy.

Martin Tilsted
sizeof(uint8) is very likely to be 1, so that is probably not the problem.
Thomas Padron-McCarthy
You've got a point, but does it really make a difference since the type is uint8?
O. Askari
A: 

In a comment to my other answer, you said that "The vector gets cleaned up in another method since it is a class member variable. I'll test insert and see what happen".

What about thread-safety? Are you absolutely sure that the clearing method does not clear 'while' the resize is happening, or immediately after it? Since it's a 'sometimes' problem, it may be induced by concurrent access to the memory management in the vector.

xtofl
+2  A: 

I'd suggest to run valgrind on your program. It's very valuable to spot early memory corruption as it may be the case with your program (since it's not a systematic crash you got).

Zeograd