3

I've got the following bug with the small program below:

#include <vector>
#include <iostream>

using namespace std;

int main() {
    vector<int> t;
    t.push_back(0);
    for(int i = 1; i < 1024; i++) {
        auto& x = t[i-1];
        t.push_back(x);
        t.push_back(x);
        t.push_back(x);
    }
    return 0;
}

It compiles fine and execute without any error. But if you run it (Linux machine) with valgrind, you get a memory error:

==122572== Invalid read of size 4
==122572==    at 0x10A051: void __gnu_cxx::new_allocator<int>::construct<int, int const&>(int*, int const&) (in /home/casse/tmp/bug)
...

Now, if you change the code above a bit:

auto x = t[i-1];

(instead of taking the reference, you copy the element from the vector), valgrind doesn't complain anymore.

Any idea?

Alan Birtles
  • 32,622
  • 4
  • 31
  • 60
Hugues Cassé
  • 41
  • 1
  • 2
    When vector resize, previous iterator/reference might be invalidated (`x` actually), You have UB. – Jarod42 Jun 18 '21 at 09:18
  • Related to [is-it-safe-to-push-back-an-element-from-the-same-vector](https://stackoverflow.com/questions/18788780/is-it-safe-to-push-back-an-element-from-the-same-vector). – Jarod42 Jun 18 '21 at 09:23
  • From further reading, the first `push_back` should be ok, just the second(third) one is problematic. – Jarod42 Jun 18 '21 at 09:34

1 Answers1

5

push_back might cause reallocation then all the references are invalidated, and dereference on invalidated reference leads to UB.

If the new size() is greater than capacity() then all iterators and references (including the past-the-end iterator) are invalidated.

You can use reserve in advance to avoid reallocation.

vector<int> t;
t.reserve(1 + 1023 * 3);
t.push_back(0);
for(int i = 1; i < 1024; i++) {
    auto& x = t[i-1];
    t.push_back(x);
    t.push_back(x);
    t.push_back(x);
}

On the other hand, for auto x = t[i-1];, x is copied from t[i-1] and independent of t[i-1], no invalidated reference issue again.

songyuanyao
  • 169,198
  • 16
  • 310
  • 405
  • @Aconcagua It’s the other way round: Valgrind is too stupid to know *why* it previously failed (and now stops failing). After all, Valgrind knows nothing about C++ vectors. It only knows about memory access. This answer removes the invalid memory access. That’s all Valgrind cares about. – Konrad Rudolph Jun 18 '21 at 09:32
  • @Aconcagua The contradiction is in the *explanation* of the observed effect (i.e. in how Valgrind works). Valgrind fundamentally doesn’t reason about why memory accesses happen. All it does is catch access to invalid memory. Understanding this is somewhat important when using Valgrind. – Konrad Rudolph Jun 18 '21 at 09:42
  • Well, seems as I've been too sloppy in my wording – once more. I've deleted my comments... – Aconcagua Jun 18 '21 at 09:44