CODEWITHSUNDEEP
c++segmentation-faultvalgrind
Prasoon Saurav
Prasoon Saurav

Reputation: 92864

Difference in behaviour (GCC and Visual C++)

Consider the following code.

#include <stdio.h>
#include <vector>
#include <iostream>

struct XYZ { int X,Y,Z; };
std::vector<XYZ> A;

int rec(int idx)
{

   int i = A.size();
   A.push_back(XYZ());
   if (idx >= 5)
     return i;

   A[i].X = rec(idx+1);

   return i;
}

int main(){
  A.clear();
  rec(0);
  puts("FINISH!");

}

I couldn't figure out the reason why the code gives a segmentation fault on Linux (IDE used: Code::Blocks) whereas on Windows (IDE used: Visual C++) it doesn't.

When I used Valgrind just to check what actually the problem was, I got this output.

I got Invalid write of size 4 at four different places. Then why didn't the code crash when I used Visual C++?

Am I missing something?

Upvotes: 12

Views: 793

Answers (3)

ereOn
ereOn

Reputation: 55736

The recursive call to rec() might modify the vector while you're assigning a value to it.

What happens if you replace

A[i].X = rec(idx+1);

with

int tmp = rec(idx+1);
A[i].X = tmp;

?

Also, just to summarize the useful comments: the operand evaluation order of a = operation is unspecified and since the vector wasn't preallocated, several resizes can occur during a recursive call to rec(), thus invalidating any iterator to values in the vector.

Upvotes: 14

Dennis Zickefoose
Dennis Zickefoose

Reputation: 10969

I get "* error for object 0x300180: incorrect checksum for freed object - object was probably modified after being freed. *" when I run that code.

As I recall, A[i].X = rec(idx+1) has three sequence points. When operator[] is called on A, when rec is called, and at the end. But the order of the first two is unspecified. So if g++ calculates A[i] first, and then calls rec(idx+1), then when rec returns the reference returned by A[i] could have been invalidated by a reallocation of the vector's internal memory. Under VC++, it might be evaluating rec(idx+1) first, so all the push_back calls are done up front, which means the A[i] calls refer to the correct block of memory. Alternatively, it might do things the same way, and you just happen to not segfault... that's one of the problems of undefined behavior.

Changing std::vector<XYZ> A; to std::vector<XYZ> A(10); will reserve enough space for 10 elements. This prevents your specific implementation of rec from ever needing to reallocate, and that fixes the error on my end.

Upvotes: 1

ChrisBD
ChrisBD

Reputation: 9209

You're using int i = A.size()

And then you're indexing your struct as an array, but using the size value. You need to reduce it by 1 e.g. A[i-1].X = rec(idx+1);

Ah my mistake - I didn't take account of the vector push_back.

Upvotes: 0

Related Questions