Boost C++ Libraries: Ticket #9578: Adapters (map_keys, map_values) cause undefined behavior (segv, etc) when applied to R-Values, especially in the context of BOOST_FOREACH

attachment set

Wed, 15 Jan 2014 20:37:51 GMT

attachment → container_test.cpp

Test to demonstrate undefined behavior of boost::range::adapter::map_keys when applied to an r-value and used with BOOST_FOREACH

cc set

Thu, 30 Jan 2014 16:59:13 GMT

cc ilubashe@… added

status changed

Neil Groves — Fri, 21 Feb 2014 20:33:14 GMT

status new → assigned

status changed; resolution set

Neil Groves — Mon, 03 Mar 2014 01:47:54 GMT

status assigned → closed
resolution → wontfix

See: https://svn.boost.org/trac/boost/ticket/5475

BOOST_FOREACH is considered to be deprecated and in maintenance mode. There are no simple solutions for C++03 that don't cause unacceptable performance overhead and enormous complexity.

status, severity changed; resolution deleted

ilubashe@… — Mon, 03 Mar 2014 04:49:48 GMT

status closed → reopened
resolution wontfix
severity Showstopper → Problem

This bug has little to do with BOOST_FOREACH. This is a boost::range problems. You'll get the same problem with C++11 "for" loops.

for(const string &s : return_map_by_value() | map_keys)

Neil Groves — Mon, 03 Mar 2014 14:33:05 GMT

Yes you would, but I have no solution for this. It seems to me to be a lifetime issue that is not fixable without enormous detrimental impact on other code. I closed the ticket not due to unwillingness to alter the code, but due to having no good solution. If we alter the semantics to require copying this has huge impact on some ranges. If we automatically move this would either require large performance overhead due to bloated adapters and would alter the semantic to have other surprising features.

I'll see if you or anyone else have any suggestions for what you actually would like done. AFAICT it simply is a general C++ issue. You have to be careful with lifetimes. Of course if there is a solution I have overlooked that can make things better with little impact on other cases I would jump at it immediately.

ilubashe@… — Mon, 03 Mar 2014 14:59:12 GMT

I called out BOOST_FOREACH in filing the bug, because BOOST_FOREACH has the solution to a very similar problem it had to deal with.

I would certainly not propose range adapter to copy values wholesale. The only values that need copying are r-values. In fact, I would be very surprised if there is ever a case when you do NOT want to copy an r-value in a range adapter. The l-values should certainly never be copied.

BOOST_FOREACH has a way to tell which values are l-values and which are r-values.

There is also a related bug: #7630

status changed; resolution set

Neil Groves — Mon, 03 Mar 2014 15:07:55 GMT

status reopened → closed
resolution → duplicate

I'm marking this as a duplicate of 7630 now since that seems to be the more general issue. I've been worrying about this lifetime issue for sometime, but haven't a good solution. I spent a little time looking at the BOOST_FOREACH implementations and I'll take a look at some of the suggestions made for ticket 7630. It might be that this can now be made to work well with the C++11 support becoming much more mature and the rvalue support that much more reliable.

I hope I don't cause any offence by collapsing this into one defect. Please feel free to make comments on the #7630 ticket.

Since there is even an example solution I should be able to find a way to make this work. I'll obviously have to take care to ensure that the fix doesn't introduce any unexpected nasty surprises.

Thanks for helping.