netcan/asyncio

wait_for控制超时导致coredump

Closed this issue · 7 comments

awen162 commented

在测试echo_client时，使用wait_for为read增加超时机制。在很快收到echo_server响应后，打印data.data()数据时，程序coredump了。请帮忙看看是什么问题，谢谢！

运行环境：
Ubuntu 18.04.4 LTS
gcc version 11.1.0

测试代码如下：

#include <iostream>
#include <asyncio/open_connection.h>
#include <asyncio/runner.h>
#include <asyncio/sleep.h>
#include <asyncio/schedule_task.h>
#include <asyncio/wait_for.h>
using asyncio::Stream;
using asyncio::Task;

using namespace std::chrono;

Task<> tcp_echo_client(std::string_view message) {
        auto stream = co_await asyncio::open_connection("127.0.0.1", 8888);

        fmt::print("Send: '{}'\n", message);
        co_await stream.write(Stream::Buffer(message.begin(), message.end()));

        fmt::print("Send: ok\n");

        auto data = co_await asyncio::wait_for(stream.read(100), 300ms);

        fmt::print("wait_for: ok data len {}\n", data.size());
        fmt::print("Received: '{}'\n", data.data());

        fmt::print("Close the connection\n");
        stream.close();
}

int main(int argc, char** argv) {
    asyncio::run(tcp_echo_client("hello world!"));
    return 0;
}

输出和报错信息如下：

Send: 'hello world!'
Send: ok
wait_for: ok data len 12
=================================================================
==16717==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000019c at pc 0x55555557d747 bp 0x7fffffffcfa0 sp 0x7fffffffc748
READ of size 13 at 0x60200000019c thread T0
    #0 0x55555557d746 in __interceptor_strlen.part.0 (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0x29746)
    #1 0x5555556906bd in fmt::v8::basic_string_view<char>::basic_string_view(char const*) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/core.h:440
    #2 0x5555556906bd in fmt::v8::appender fmt::v8::detail::write<char, fmt::v8::appender>(fmt::v8::appender, char const*) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/format.h:2122
    #3 0x55555568a2f5 in fmt::v8::appender fmt::v8::detail::default_arg_formatter<char>::operator()<char const*>(char const*) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/format.h:2162
    #4 0x55555569254b in decltype ({parm#1}(0)) fmt::v8::visit_format_arg<fmt::v8::detail::default_arg_formatter<char>, fmt::v8::basic_format_context<fmt::v8::appender, char> >(fmt::v8::detail::default_arg_formatter<char>&&, fmt::v8::basic_format_arg<fmt::v8::basic_format_context<fmt::v8::appender, char> > const&) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/core.h:1560
    #5 0x55555569254b in fmt::v8::detail::vformat_to<char>(fmt::v8::detail::buffer<char>&, fmt::v8::basic_string_view<char>, fmt::v8::basic_format_args<fmt::v8::basic_format_context<std::conditional<std::is_same<fmt::v8::type_identity<char>::type, char>::value, fmt::v8::appender, std::back_insert_iterator<fmt::v8::detail::buffer<fmt::v8::type_identity<char>::type> > >::type, fmt::v8::type_identity<char>::type> >, fmt::v8::detail::locale_ref)::format_handler::on_replacement_field(int, char const*) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/format.h:2919
    #6 0x55555569254b in char const* fmt::v8::detail::parse_replacement_field<char, fmt::v8::detail::vformat_to<char>(fmt::v8::detail::buffer<char>&, fmt::v8::basic_string_view<char>, fmt::v8::basic_format_args<fmt::v8::basic_format_context<std::conditional<std::is_same<fmt::v8::type_identity<char>::type, char>::value, fmt::v8::appender, std::back_insert_iterator<fmt::v8::detail::buffer<fmt::v8::type_identity<char>::type> > >::type, fmt::v8::type_identity<char>::type> >, fmt::v8::detail::locale_ref)::format_handler&>(char const*, char const*, fmt::v8::detail::vformat_to<char>(fmt::v8::detail::buffer<char>&, fmt::v8::basic_string_view<char>, fmt::v8::basic_format_args<fmt::v8::basic_format_context<std::conditional<std::is_same<fmt::v8::type_identity<char>::type, char>::value, fmt::v8::appender, std::back_insert_iterator<fmt::v8::detail::buffer<fmt::v8::type_identity<char>::type> > >::type, fmt::v8::type_identity<char>::type> >, fmt::v8::detail::locale_ref)::format_handler&) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/core.h:2536
    #7 0x55555567b74c in void fmt::v8::detail::parse_format_string<false, char, fmt::v8::detail::vformat_to<char>(fmt::v8::detail::buffer<char>&, fmt::v8::basic_string_view<char>, fmt::v8::basic_format_args<fmt::v8::basic_format_context<std::conditional<std::is_same<fmt::v8::type_identity<char>::type, char>::value, fmt::v8::appender, std::back_insert_iterator<fmt::v8::detail::buffer<fmt::v8::type_identity<char>::type> > >::type, fmt::v8::type_identity<char>::type> >, fmt::v8::detail::locale_ref)::format_handler>(fmt::v8::basic_string_view<char>, fmt::v8::detail::vformat_to<char>(fmt::v8::detail::buffer<char>&, fmt::v8::basic_string_view<char>, fmt::v8::basic_format_args<fmt::v8::basic_format_context<std::conditional<std::is_same<fmt::v8::type_identity<char>::type, char>::value, fmt::v8::appender, std::back_insert_iterator<fmt::v8::detail::buffer<fmt::v8::type_identity<char>::type> > >::type, fmt::v8::type_identity<char>::type> >, fmt::v8::detail::locale_ref)::format_handler&&) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/core.h:2571
    #8 0x55555567b74c in void fmt::v8::detail::vformat_to<char>(fmt::v8::detail::buffer<char>&, fmt::v8::basic_string_view<char>, fmt::v8::basic_format_args<fmt::v8::basic_format_context<std::conditional<std::is_same<fmt::v8::type_identity<char>::type, char>::value, fmt::v8::appender, std::back_insert_iterator<fmt::v8::detail::buffer<fmt::v8::type_identity<char>::type> > >::type, fmt::v8::type_identity<char>::type> >, fmt::v8::detail::locale_ref) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/format.h:2945
    #9 0x55555566dc10 in fmt::v8::vprint(_IO_FILE*, fmt::v8::basic_string_view<char>, fmt::v8::basic_format_args<fmt::v8::basic_format_context<fmt::v8::appender, char> >) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/format-inl.h:2611
    #10 0x55555566de4a in fmt::v8::vprint(fmt::v8::basic_string_view<char>, fmt::v8::basic_format_args<fmt::v8::basic_format_context<fmt::v8::appender, char> >) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/format-inl.h:2627
    #11 0x555555640102 in void fmt::v8::print<char*>(fmt::v8::basic_format_string<char, fmt::v8::type_identity<char*>::type>, char*&&) /home/ubuntu/c++/asyncio/third_party/fmt/include/fmt/core.h:3150
    #12 0x555555640102 in tcp_echo_client(std::basic_string_view<char, std::char_traits<char> >) [clone .actor] /home/ubuntu/c++/asyncio/test/st/echo_client.cpp:26
    #13 0x555555662401 in std::__n4861::coroutine_handle<asyncio::Task<void>::promise_type>::resume() const (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0x10e401)
    #14 0x555555661671 in asyncio::Task<void>::promise_type::run() (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0x10d671)
    #15 0x5555556634c3 in asyncio::EventLoop::run_once() /home/ubuntu/c++/asyncio/src/event_loop.cpp:63
    #16 0x555555662805 in asyncio::EventLoop::run_until_complete() /home/ubuntu/c++/asyncio/src/event_loop.cpp:17
    #17 0x555555647d14 in decltype(auto) asyncio::run<asyncio::Task<void> >(asyncio::Task<void>&&) (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0xf3d14)
    #18 0x55555564099b in main /home/ubuntu/c++/asyncio/test/st/echo_client.cpp:33
    #19 0x7ffff6a15bf6 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21bf6)
    #20 0x555555564bb9 in _start (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0x10bb9)

0x60200000019c is located 0 bytes to the right of 12-byte region [0x602000000190,0x60200000019c)
allocated by thread T0 here:
    #0 0x5555555f5297 in operator new(unsigned long) (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0xa1297)
    #1 0x555555659af1 in __gnu_cxx::new_allocator<char>::allocate(unsigned long, void const*) /usr/include/c++/11/ext/new_allocator.h:121
    #2 0x5555556552dd in std::allocator<char>::allocate(unsigned long) /usr/include/c++/11/bits/allocator.h:173
    #3 0x5555556552dd in std::allocator_traits<std::allocator<char> >::allocate(std::allocator<char>&, unsigned long) /usr/include/c++/11/bits/alloc_traits.h:460
    #4 0x555555653ae9 in std::_Vector_base<char, std::allocator<char> >::_M_allocate(unsigned long) (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0xffae9)
    #5 0x55555565374c in std::_Vector_base<char, std::allocator<char> >::_M_create_storage(unsigned long) (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0xff74c)
    #6 0x55555564ee7c in std::_Vector_base<char, std::allocator<char> >::_Vector_base(unsigned long, std::allocator<char> const&) (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0xfae7c)
    #7 0x555555657ccd in std::vector<char, std::allocator<char> >::vector(std::vector<char, std::allocator<char> > const&) (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0x103ccd)
    #8 0x555555654b6e in asyncio::Result<std::vector<char, std::allocator<char> > >::result() & (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0x100b6e)
    #9 0x555555651d35 in asyncio::detail::WaitForAwaiter<std::vector<char, std::allocator<char> >, std::chrono::duration<long, std::ratio<1l, 1000l> > >::await_resume() (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0xfdd35)
    #10 0x555555641124 in asyncio::Task<decltype ((((declval<asyncio::detail::GetAwaiter<asyncio::Task<std::vector<char, std::allocator<char> > > >::type>)()).await_resume)())> asyncio::detail::wait_for<asyncio::Task<std::vector<char, std::allocator<char> > >, long, std::ratio<1l, 1000l> >(asyncio::NoWaitAtInitialSuspend, asyncio::Task<std::vector<char, std::allocator<char> > >&&, std::chrono::duration<long, std::ratio<1l, 1000l> >) [clone .actor] /home/ubuntu/c++/asyncio/include/asyncio/wait_for.h:100
    #11 0x5555556623a1 in std::__n4861::coroutine_handle<asyncio::Task<std::vector<char, std::allocator<char> > >::promise_type>::resume() const (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0x10e3a1)
    #12 0x555555660f19 in asyncio::Task<std::vector<char, std::allocator<char> > >::promise_type::run() (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0x10cf19)
    #13 0x5555556634c3 in asyncio::EventLoop::run_once() /home/ubuntu/c++/asyncio/src/event_loop.cpp:63
    #14 0x555555662805 in asyncio::EventLoop::run_until_complete() /home/ubuntu/c++/asyncio/src/event_loop.cpp:17
    #15 0x555555647d14 in decltype(auto) asyncio::run<asyncio::Task<void> >(asyncio::Task<void>&&) (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0xf3d14)
    #16 0x55555564099b in main /home/ubuntu/c++/asyncio/test/st/echo_client.cpp:33
    #17 0x7ffff6a15bf6 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21bf6)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/ubuntu/c++/asyncio/build/test/st/echo_client+0x29746) in __interceptor_strlen.part.0
Shadow bytes around the buggy address:
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff8000: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff8010: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff8020: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
=>0x0c047fff8030: fa fa 00[04]fa fa fd fd fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==16717==ABORTING
[1] + Done                       "/usr/bin/gdb" --interpreter=mi --tty=${DbgTerm} 0<"/tmp/Microsoft-MIEngine-In-qj4itxmc.b1y" 1>"/tmp/Microsoft-MIEngine-Out-gs1cjxb4.rcj"
netcan commented

看样子是发生了越界读，原因是客户端发送的字符串少发了'\0'。已经修复，可以参考这个提交。
1481361#diff-4a4b47f02ae41b2a8b7e9512e447950c24862dfe67049798ff0653c047e210caR15

awen162 commented

谢谢答复！
有个疑问：没有修改echo_client代码前，测试没有问题。那里也是没有添加结束符的。现在就改调用wait_for就有问题了，这里面有没有可能还有其它问题？
麻烦帮忙分析下，或者有什么方法进行分析，我可以尝试去分析下，谢谢！

awen162 commented

另外发现个问题：
auto data = co_await stream.read(100); 中，data.data()的地址，与stream.read中分配的result.data()地址一致。说明是使用了移动构造。
auto data = co_await asyncio::wait_for(stream.read(100), 300ms);中，data.data()的地址，与stream.read中分配的result.data()地址不是一致。说明是使用了拷贝或者赋值构造，这里是否可以优化下？

netcan commented

另外发现个问题： auto data = co_await stream.read(100); 中，data.data()的地址，与stream.read中分配的result.data()地址一致。说明是使用了移动构造。 auto data = co_await asyncio::wait_for(stream.read(100), 300ms);中，data.data()的地址，与stream.read中分配的result.data()地址不是一致。说明是使用了拷贝或者赋值构造，这里是否可以优化下？

你分析的对，中间应该是发生了拷贝构造。可以优化，得找出发生拷贝构造的时机点，然后通过完美转发之类的移动。暂时没时间改，你可以尝试提下PR，或者等我后续修复也行。

awen162 commented

两种情况对比测试发现，在result.h头文件中，调用如下的左值和右值函数，导致一个使用移动构造，一个使用赋值构造。
具体要如何修复，我也不是很清楚。

wait_for 调用了第一个函数进行结果赋值，将return *res，修改为return std::move(*res)测试可以，但是不清楚会不会带来其它问题。
非wait_for 调用了第二个函数进行结果赋值

constexpr T result() & {
        if (auto exception = std::get_if<std::exception_ptr>(&result_)) {
            std::rethrow_exception(*exception);
        }
        if (auto res = std::get_if<T>(&result_)) {
            return *res;
        }
        throw NoResultError{};
    }
    constexpr T result() && {
        if (auto exception = std::get_if<std::exception_ptr>(&result_)) {
            std::rethrow_exception(*exception);
        }
        if (auto res = std::get_if<T>(&result_)) {
            return std::move(*res);
        }
        throw NoResultError{};
    }
netcan commented

两种情况对比测试发现，在result.h头文件中，调用如下的左值和右值函数，导致一个使用移动构造，一个使用赋值构造。 具体要如何修复，我也不是很清楚。

wait_for 调用了第一个函数进行结果赋值，将return *res，修改为return std::move(*res)测试可以，但是不清楚会不会带来其它问题。 非wait_for 调用了第二个函数进行结果赋值

constexpr T result() & {
        if (auto exception = std::get_if<std::exception_ptr>(&result_)) {
            std::rethrow_exception(*exception);
        }
        if (auto res = std::get_if<T>(&result_)) {
            return *res;
        }
        throw NoResultError{};
    }
    constexpr T result() && {
        if (auto exception = std::get_if<std::exception_ptr>(&result_)) {
            std::rethrow_exception(*exception);
        }
        if (auto res = std::get_if<T>(&result_)) {
            return std::move(*res);
        }
        throw NoResultError{};
    }

那应该就是这一行的问题了：
https://github.com/netcan/asyncio/blob/master/include/asyncio/wait_for.h#L21

改成：

return std::move(result_).result();

然后所有测试通过的话，应该就没问题了。

awen162 commented

两种情况对比测试发现，在result.h头文件中，调用如下的左值和右值函数，导致一个使用移动构造，一个使用赋值构造。 具体要如何修复，我也不是很清楚。
wait_for 调用了第一个函数进行结果赋值，将return *res，修改为return std::move(*res)测试可以，但是不清楚会不会带来其它问题。 非wait_for 调用了第二个函数进行结果赋值

constexpr T result() & {
        if (auto exception = std::get_if<std::exception_ptr>(&result_)) {
            std::rethrow_exception(*exception);
        }
        if (auto res = std::get_if<T>(&result_)) {
            return *res;
        }
        throw NoResultError{};
    }
    constexpr T result() && {
        if (auto exception = std::get_if<std::exception_ptr>(&result_)) {
            std::rethrow_exception(*exception);
        }
        if (auto res = std::get_if<T>(&result_)) {
            return std::move(*res);
        }
        throw NoResultError{};
    }

那应该就是这一行的问题了： https://github.com/netcan/asyncio/blob/master/include/asyncio/wait_for.h#L21

改成：

return std::move(result_).result();

然后所有测试通过的话，应该就没问题了。

按照你给的修改方案，58个测试用例测试都可以了。wait_for那里也没有再使用赋值构造了，直接使用了移动构造。

  • 👍1