Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Mysterious Moving Pointers

blomqu.ist

Post reply on HN
Print view View original

11–20 of 68 posts

Re: Mysterious Moving Pointers

#11

Post by inetknght »

chaboud wrote: post #7
olliej wrote: post #2

I know C++ the language but not the STL (the overwhelming abundance of UB and total lack of safety make it an anathema), so my question is why the STL allows/requires non-move here copying here dependent on whether an object has a no throw move constructor? Note I’m not asking about move constructor vs memmove/cpy but rather the use of copy constructor vs move depending on exception behavior? Is it something like pre…

That’s a bit like saying you know C++ but not streams or templates, or C but not floating point operations. It’s probably worth learning STL. Anyway, the reason to use move instead of copy is for performance. Move constructors are faster because they can leave the source object modified (e.g., take over control of a pointer to deep contents). This falls apart when the move constructor can throw, because the container…

> leaving the object before the exception modified and the code in an unrecoverable state

It isn't likely to leave the code in an unrecoverable state even if recovery is calling std::terminate (or worse).

It is likely to leave the data in an unrecoverable state. Imagine that a vector of 4 items was resized -- the first two objects move successfully, but the third one throws an exception. Then in your move function, you catch that and decide to undo your changes before propagating the error. Then when you're undoing the changes the first object throws an exception when it's being moved back. Oof! At best you've got multiple active exceptions (legal if you're in the catch handler, but should be rare and definitely should be avoided) and at worst your data is indeed unrecoverable (thus one of many reasons why std::terminate is the default option when multiple exceptions are alive on the same stack).

Re: Mysterious Moving Pointers

#13

Post by inetknght »

kentonv wrote: post #10
TillE wrote: post #3

Storing a pointer to memory that you did not explicitly allocate is always a red flag, I think. You really need to understand how everything works, and be very careful. I would default to just using std::unique_ptr in a situation like this, especially since using std::list suggests performance isn't critical here, so the additional indirection probably doesn't matter.

unique_ptr seems inappropriate here since the pointers aren't unique. shared_ptr doesn't even work because it looks like this data structure is representing a graph and would expect to have cycles. Perhaps you could use some sort of weak pointer that gets nulled out when the target object is destroyed, but that would not have fixed the bug here, just replaced segfaults with some more controlled exception or panic. In…

> Perhaps you could use some sort of weak pointer that gets nulled out when the target object is destroyed

std::shared_ptr comes with std::weak_ptr. Referencing counting is rather ham-fisted approach but is certainly a solution.

> IMO the language-level problem, if there is one, is that C++ is too willing to copy in cases where you would expect it to move instead.

IMO that's not a problem in the language but a problem with the engineer (misunderstanding when std::move is necessary) and the tooling (linter/static analyzer not clearly identifying that something should be moved instead, and raising a linter warning for it).

For that matter, the places where I see std::list used aren't places where "performance isn't important" but rather places where an inexperienced engineer was put in charge of implementation and a senior engineer accepted it. I can't remember the last time I accepted someone using std::list in a code review because there has always been a better design available even if it necessitated some teaching. If a stable pointer address is needed then indeed a smart pointer is the correct solution (perhaps std::vector). There are other reasons I've had coworkers cite for using std::list (eg constant allocation time) but that's generally resolved with std::vector.reserve(upper bound to size) or eg a slab allocator (unfortunately, I'm not aware of a standard-provided slab allocator, though to be fair I'm not very familiar with C++ standard allocators in general).

> I think life would be better these days if all non-trivial copies had to be requested explicitly

While I don't agree superficially (smells like bringing along deep-copy problems), I think the idea merits some thought experiments.

It would be fairly trivial to do that for non-plain-old-data types by deleting the copy constructor/operator (so it cannot happen implicitly) and providing a `make_copy(...)` function instead.

Re: Mysterious Moving Pointers

#14

Post by kentonv »

olliej wrote: post #2

I know C++ the language but not the STL (the overwhelming abundance of UB and total lack of safety make it an anathema), so my question is why the STL allows/requires non-move here copying here dependent on whether an object has a no throw move constructor? Note I’m not asking about move constructor vs memmove/cpy but rather the use of copy constructor vs move depending on exception behavior? Is it something like pre…

I think the other replies may have misunderstood your question. I think you are asking:

Why does std::vector require T's move constructor to be noexcept (or else it falls back to copying instead)?

The reason goes something like this:

When std::vector grows, it needs to move or copy all of its elements into a new, larger-capacity array. It would prefer to move them, since that's a lot more efficient than copying (for non-trivial types). But what happens if it moves N elements, and then the move constructor for element N+1 throws an exception? Elements 0-N have been moved away already, so the vector is no longer valid as-is. Should it try to move those elements back to the original array? But what if one of those moves fails?

The C++ standards body decided to sidestep this whole problem by saying that std::vector will refuse to use T's move constructor unless it is declared noexcept, so the above problem can't happen.

In my opinion, this was a huge mistake. Intuitively, everyone expects that when an std::vector grows, it's going to move the elements, not making a ton of copies. Often, these copies result in hidden performance problems. Arguably the author of this post is lucky than in their case, the copies resulted in outright failure, thus revealing the problem.

There seem to be two other possibilities:

* std::vector could simply refuse to compile if the move constructor was not `noexcept`. I think this could have been done in a way that wouldn't have broken existing code, if it had been introduced before move constructors existed in the wild -- unfortunately, that ship has now sailed and this cannot be done now without breaking people.

* std::vector could always use move constructors, even if they are not declared `noexcept`, and simply crash (std::terminate()) in the case that one actually throws. IMO this would be fine and is the best solution. Move constructors almost never actually throw in practice, regardless of whether they are declared as such, because move constructors are almost always just "copy pointer, null out the original". You don't put complex logic in your move constructor. And anyway, C++ already has plenty of precedent for turning poorly-timed exceptions into terminations; why not add another case? But I think it's unlikely the standards committee would change this now.

Re: Mysterious Moving Pointers

#15

Post by pjmlp »

jnwatson wrote: post #5

This is a great reminder of the pox that was Microsoft of the early part of the millennium. Besides an allergy to investing in web standards, they were woefully behind in their language support. Their non-adoption of modern C++ standards held client security back for a decade, and arguable held language standards development back.

There is a certain irony complaining about Microsoft, while praising everyone else in regards to C and C++ compilers, as if outside the beloved GCC, in a age where clang did not exist, the other proprietary compilers were an example of perfection.

Apparently the folks didn't learn their lesson with Web standards, given the power they gave Google to transform the Web into ChromeOS.

Re: Mysterious Moving Pointers

#16

Post by o11c »

kentonv wrote: post #10
TillE wrote: post #3

Storing a pointer to memory that you did not explicitly allocate is always a red flag, I think. You really need to understand how everything works, and be very careful. I would default to just using std::unique_ptr in a situation like this, especially since using std::list suggests performance isn't critical here, so the additional indirection probably doesn't matter.

unique_ptr seems inappropriate here since the pointers aren't unique. shared_ptr doesn't even work because it looks like this data structure is representing a graph and would expect to have cycles. Perhaps you could use some sort of weak pointer that gets nulled out when the target object is destroyed, but that would not have fixed the bug here, just replaced segfaults with some more controlled exception or panic. In…

The good news is that since C++ containers aren't special to the language, you can just implement your own wrapper classes that disable the copy ctor (and provide an explicit `.clone()` instead). Coupled with `#pragma GCC poison` it is pretty easy to blacklist legacy footguns in source files at least (though not in headers without some aggressive work).

... and yet, almost all vulnerabilities in C++ code are still written in C style, not even legacy C++.

Re: Mysterious Moving Pointers

#17

Post by wakawaka28 »

This is a noob mistake, not a huge mystery. It's not always wrong to store raw pointers to STL container elements, but if you do then you must take care of reallocations.

If you find storing pointers to elements too perilous, you should probably just make a container of pointers instead.

Re: Mysterious Moving Pointers

#18

Post by Joker_vD »

kentonv wrote: post #14
olliej wrote: post #2

I know C++ the language but not the STL (the overwhelming abundance of UB and total lack of safety make it an anathema), so my question is why the STL allows/requires non-move here copying here dependent on whether an object has a no throw move constructor? Note I’m not asking about move constructor vs memmove/cpy but rather the use of copy constructor vs move depending on exception behavior? Is it something like pre…

I think the other replies may have misunderstood your question. I think you are asking: Why does std::vector require T's move constructor to be noexcept (or else it falls back to copying instead)? The reason goes something like this: When std::vector grows, it needs to move or copy all of its elements into a new, larger-capacity array. It would prefer to move them, since that's a lot more efficient than copying (for…

Honestly, trying to move the elements back and calling std::abort if that fails seems fine. It is indeed an exceptional happenstance, and how quickly you can recover from it is probably not as important as being able to recover correctly. And who catches exceptions around resize()/push_back() anyway?

Re: Mysterious Moving Pointers

#20

Post by limaoscarjuliet »

userbinator wrote: post #12

IMHO this is another case where C++'s hidden layers of complexity hides bugs that would've been obvious in plain C. In fact for this particular use-case I'd probably use indices instead of pointers.

I remember the C++ ca. 1994 year when I started my career. It was C with Objects back then. And it was great! C++ was better C, it was easy for any C dev to convince him/her to jump to it.

I recently had to work with C++ code and... it is not a happy story anymore:

- Lots of magic like described here

- F**ing templates - for those who like them, did you ever see a C++ core file? Or tried to understand a single symbol?

- Standarization that feels like pulling more Boost into the language, which means more templates. Which makes core files incomprehensible.

It used the be that average dev who knew C could read and work with simple C++. This is no longer true. C++ is no longer a better C.

P.S. Example from recent core file, one line in the stack trace: boost::asio::asio_handler_invoke::waitForSignal:: >(XServer::m_accept_loop(tsr::Data&)::::&&)::, boost::system::error_code, int> > (function=...)

Post reply on HN