Difference in behaviour (GCC and Visual C++)

Question

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?

Don't you have to allocate enough space in the vector before you use it? It is pure accident, or possibly just a different, arguably more friendly but less standard, implementation of `vector<>`. — Jonathan Leffler, May 06 '10 at 06:30
What IDE you're using isn't very useful information. What versions of the relevant compilers are you using is more appropriate. — Dennis Zickefoose, May 06 '10 at 06:37
@Jonathan: `push_back()` will allocate space for the element when necessary (including reallocating and moving the whole vector to a new contiguous block if needed).. — Michael Burr, May 06 '10 at 06:39
@Jonathan : I don't think it is mandatory to allocate enough space in the vector if the above code is under concern. — Prasoon Saurav, May 06 '10 at 06:41
Are they running on the same hardware? When posting about compilers used, also look at the optimisation settings. — ChrisBD, May 06 '10 at 06:45
@ChrisBD: That won't make much of a difference. The answer given by ereOn is correct. — Prasoon Saurav, May 06 '10 at 06:47

ereOn · Accepted Answer · 2013-04-25T18:49:38.810

14

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.

edited Apr 25 '13 at 18:49

answered May 06 '10 at 06:31

ereOn

53,676
39
161
238

4

To be a bit more precise, inside the recursive call, the call to `push_back` can cause the previous pointers to be invalidated. – R Samuel Klatchko May 06 '10 at 06:40
2

Couldn't it basically be the `i = ++i;` issue (except the modification of A on the right side is hidden in the recursive call)? The compiler is free to evaluate `A[i]` before or after `rec` and whether it "works", depends on which order it chooses to do so. – UncleBens May 06 '10 at 06:44
2

@ereOn: Yes! I think your answer is correct. The order of evaluation of operands associated with "=" operator is _unspecified_ . – Prasoon Saurav May 06 '10 at 06:46
@Prasoon Saurav: Glad I could help ;) – ereOn May 06 '10 at 07:05

Dennis Zickefoose · Answer 2 · 2010-05-06T08:25:00.977

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.

-1. This solution would surely work right up to the moment where you increase the max recursive value to 11. — deworde, May 09 '10 at 20:29
@deworde: Obviously, hence the qualification that it prevents his specific implementation from needing to reallocate. — Dennis Zickefoose, May 10 '10 at 04:37
True, but I felt it would have been good practice to put the full caveat into the answer, for clarity. If you edit it, I'll change my vote. — deworde, May 10 '10 at 15:12

ChrisBD · Answer 3 · 2010-05-06T06:38:47.247

0

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.

edited May 06 '10 at 06:38

answered May 06 '10 at 06:32

ChrisBD

9,104
3
22
35

2

But he does the indexing based on the `size()` result *after* adding an element to the `vector`. At that point the index is OK. – Michael Burr May 06 '10 at 06:34

Difference in behaviour (GCC and Visual C++)

3 Answers3