Why are std::begin and std::end "not memory safe"?

55
votes

In this blog post, Eric Niebler states that:

What is wrong with std::begin and std::end? Surprise! they are not memory safe. Consider what this code does:

extern std::vector<int> get_data();
auto it = std::begin(get_data());
int i = *it; // BOOM

std::begin has two overloads for const and non-const lvalues. Trouble is, rvalues bind to const lvalue references, leading to the dangling iterator it above.

I'm having trouble understanding his point and why it is a dangling reference. Could someone explain?

6
c++c++11iteratorstd
How is the above code any different from auto it = get_data().begin();. That code has the same problem I think, so what point is the author is making? – john
@john Eric is saying that ranges::begin is better than std::begin, he isn't claiming anything about .begin. – Marc Glisse
@MarcGlisse OK, I was misled by the title of this post, and I couldn't easily find the original quote. – john
Related question: stackoverflow.com/questions/48976859/… – Arne Vogel
Personally, I would say that's more a combination of "not Machiavelli-safe" or "not derp-safe" and a quirk of rvalue references than "not memory-safe", since it demonstrates that std::begin() is exactly as memory-safe as the parameter you pass to it, and not inherently unsafe solely in and of itself. That's just me, though. – Justin Time - Reinstate Monica

6 Answers

60
votes

The get_data function returns an object. When used the way shown, that object will be a temporary object, which will be destructed once the full expression ends. The iterator now references a vector object which no longer exists, and can't be dereferenced or used in any useful way.

53
votes

I think Eric's point about std::begin is that it silently accepts an rvalue container as an argument to begin with. On the face of it, the problem with the code is also exemplified in

auto it = get_data().begin();

But std::begin is a free function template, it can be made to reject rvalues without needing to add the proper reference qualifiers to each container's begin members. By "just" forwarding it misses an opportunity to add a layer of memory safety to code.

Ideally, the overload set could have benefited from the addition of

template< class C > 
void begin( C&& ) = delete;

That one would have caused the code in the blog post to be flat out rejected on the spot.

14
votes

The temporary vector returned by get_data goes out of scope after std::begin is done. It is not kept alive, so it is an iterator into a destroyed object.

4
votes

Is it fair to say this is a memory safety failing of std::begin? There is no 'safe' way of creating an iterator such that it will be valid to follow the pointer after the container is deleted.

It lacks a check that another method has, but it's not like there is no way to make a ranges derived iterator go pop. You just have to try a bit harder...

2
votes

Because it allows initialisation from an rvalue, which is bad.

0
votes

In fact, the complicated function can be simplified to two short functions, such as:

int& foo(int x){
   return x;
}
int generate_a_int(){
   return 42;
}

and then invoke it foo(generate_a_int()), a temporary value is generated, once out of the function body, the temporary value generated by generate_a_int() is destroyed, and then a dangle reference hass happened.

Do you understand now?