Use after move in write_op::operator()

[vladimir-mihov]

The C++ standard doesn't guarantee the order of evaluation of function arguments. In the following code, the buffers_ member of write_op is being passed as the first function argument to async_write_some, while the write_op itself is being moved into an l-value in the second argument. If the order of evaluation is from right to left, this->buffers_ would be moved before it's used for the first argument, leaving it empty.

https://github.com/boostorg/asio/blob/develop/include/boost/asio/impl/write.hpp#L333

    void operator()(boost::system::error_code ec,
        std::size_t bytes_transferred, int start = 0)
    {
      std::size_t max_size;
      switch (start_ = start)
      {
        case 1:
        max_size = this->check_for_completion(ec, buffers_.total_consumed());
        for (;;)
        {
          {
            BOOST_ASIO_HANDLER_LOCATION((__FILE__, __LINE__, "async_write"));
            stream_.async_write_some(buffers_.prepare(max_size), // Use after move <----------------------------------
                BOOST_ASIO_MOVE_CAST(write_op)(*this));
          }
          return; default:

We hit this issue with GCC9.2 and Boost 1.78. There are ~150 instances of this template in our code and GCC decides to evaluate from right to left in only 1 of them. The issue is not observed with Clang.

[vladimir-mihov]

It turns out that our custom code is at fault here. We had a custom stream that had async_write_some overridden and it was taking the handler by value.

struct OutputStream {
      template<typename ConstBufSeq, typename Handler>
      void async_write_some(const ConstBufSeq& cbs, Handler h) {
         ...
      }
}

After changing it to use a forwarding reference and forwarding the handler down the chain, we no longer see the issue.

I think that boost can be a little pickier on the functions that can be handlers because if the function takes the handler (write_op in this case) by value, problems can occur based on the environment in which the code is compiled, which is never a good thing.

[ppadevski]

I think the code in boost asio still needs to be fixed as otherwise we may end up in the same situation due to e.g. inlining.