1

I am getting segmentation fault on below line of my code with stacktrace as stated below. self is a unique_ptr here.

self->socket.async_send_to(self->frame->get_asio_buffer(), self->client_endpoint,
                           std::bind(&download_server::receiver, std::move(self), std::placeholders::_1, 
                           std::placeholders::_2));

Stacktrace. #3 is code line mentioned above

==1652==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000090 (pc 0x55e763e1bdc9 bp 0x7ffc2242d380 sp 0x7ffc2242d370 T0)
==1652==The signal is caused by a READ memory access.
==1652==Hint: address points to the zero page.
    #0 0x55e763e1bdc9 in std::__shared_ptr<tftp::frame, (__gnu_cxx::_Lock_policy)2>::get() const /usr/include/c++/10.2.0/bits/shared_ptr_base.h:1325
    #1 0x55e763e1a9d5 in std::__shared_ptr_access<tftp::frame, (__gnu_cxx::_Lock_policy)2, false, false>::_M_get() const /usr/include/c++/10.2.0/bits/shared_ptr_base.h:1024
    #2 0x55e763e1948d in std::__shared_ptr_access<tftp::frame, (__gnu_cxx::_Lock_policy)2, false, false>::operator->() const /usr/include/c++/10.2.0/bits/shared_ptr_base.h:1018
    #3 0x55e763e0d665 in tftp::download_server::sender(std::unique_ptr<tftp::download_server, std::default_delete<tftp::download_server> >&, boost::system::error_code const&, unsigned long) ../src/tftp_server.cpp:50
    #4 0x55e763e0cf56 in tftp::download_server::serve(boost::asio::io_context&, std::shared_ptr<tftp::frame const> const&, boost::asio::ip::basic_endpoint<boost::asio::ip::udp> const&) ../src/tftp_server.cpp:27
    #5 0x55e763e0da65 in spin_tftp_server(boost::asio::io_context&, std::shared_ptr<tftp::frame const> const&, boost::asio::ip::basic_endpoint<boost::asio::ip::udp> const&) ../src/tftp_server.cpp:97
    #6 0x55e763e0e3f5 in tftp::distributor::perform_distribution_cb(boost::system::error_code const&, unsigned long const&) ../src/tftp_server.cpp:142
    #7 0x55e763e273fe in void std::__invoke_impl<void, void (tftp::distributor::*&)(boost::system::error_code const&, unsigned long const&), std::shared_ptr<tftp::distributor>&, boost::system::error_code const&, unsigned long const&>(std::__invoke_memfun_deref, void (tftp::distributor::*&)(boost::system::error_code const&, unsigned long const&), std::shared_ptr<tftp::distributor>&, boost::system::error_code const&, unsigned long const&) /usr/include/c++/10.2.0/bits/invoke.h:73

In the above code self is used and moved in a single statement. Is it a valid ? If yes then what is the sequence of usage of self ?

Code on top of trace file(file /usr/include/c++/10.2.0/bits/shared_ptr_base.h)

1322       /// Return the stored pointer.
1323       element_type*
1324       get() const noexcept
1325       { return _M_ptr; }
1326

As per comment here on line 1322 it seems like above usage is invalid. At runtime it's failing to find data pointed by unique pointer. Is this reading correct ?

However if above hypothesis is correct then below sample code should also crash. Here self->t.async_wait does similar thing(moving and using in same statement) however this worked without any trouble.

#include <iostream>
#include <functional>
#include <boost/asio.hpp>

class looper {
public:
  static void create(boost::asio::io_context &io){
    std::unique_ptr<looper> self = std::make_unique<looper>(io);
    start(self, boost::system::error_code());
  }

  static void start(std::unique_ptr<looper> &self, const boost::system::error_code e){
    std::cout << "Round :" << self->count << std::endl;
    if(self->count-- == 0){
      return;
    }
    self->t.expires_after(boost::asio::chrono::seconds(1));
    self->t.async_wait(std::bind(&looper::start, std::move(self), std::placeholders::_1));
  }

  looper(boost::asio::io_context &io) : t(io) {
    std::cout << "Construction" << std::endl;
  }

  ~looper() {
    std::cout << "Destruction" << std::endl;
  }
  boost::asio::steady_timer t;
  uint32_t count = 3;
};

void f(boost::asio::io_context &io){
  looper::create(io);
}

int main() {
  boost::asio::io_context io;
  f(io);
  io.run();
  return 0;
}

Output of sample program

[root@archlinux cpp]# g++ unique_async.cpp  -lpthread -fsanitize=address -Wpedantic
[root@archlinux cpp]# ./a.out
Construction
Round :3
Round :2
Round :1
Round :0
Destruction
[root@archlinux cpp]#

If std::move is problem then why first case crashes but sample programs runs without any problem.

chandola
  • 126
  • 1
  • 10
  • 1
    You have UB (undefined behaviour). The order of the evaluation of function's parameters is unspecified. Thus, `self->socket.async_send_to(self->frame->get_asio_buffer(), ...` and `std::move(self)` is UB. – 273K May 16 '21 at 06:46
  • By UB you mean "undefined behavior" ? – chandola May 16 '21 at 06:47
  • 2
    The problem is the nested call to `bind`, which empties `self` before it is accessed by the other arguments. The `move(self)` itself is not problematic; for example, `self->foo(self->bar, move(self))` would be perfectly fine. – j6t May 16 '21 at 07:05

1 Answers1

2

The move(self) itself is not problematic. It is the combination of a nested call to bind and other arguments that access self. The call to bind empties self before it is accessed by the other arguments.

You have two show-cases:

// bad
self->socket.async_send_to(self->frame->get_asio_buffer(),
                           self->client_endpoint,
                           bind(&download_server::receiver, move(self), _1, _2));
// good
self->t.async_wait(bind(&looper::start, move(self), _1));

Before, C++17, both cases had undefined behavior because the arguments were evaluated in an unspecified order and are also unsequenced. This includes the object expression before the function call (before ().

Starting with C++17, argument expressions are indeterminately sequenced, but the object expression is sequenced before the other arguments. The consequences are:

  • In the bad case, bind can be invoked before the other arguments, and that would clear self, leading a null pointer access when the other arguments are evaluated.
  • In the good case, however, the object expression is invoked first while self is still valid, and following that bind is invoked, which clears self; but since there are no other arguments to evaluate, the call is fine.
j6t
  • 9,150
  • 1
  • 15
  • 35