Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

fix: multiple asan errors #715

Merged
merged 2 commits into from
Jan 11, 2021
Merged

fix: multiple asan errors #715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
43366f8
fix: multiple asan warnings
neverchanje Dec 28, 2020
33f9951
fix clang-format
neverchanje Dec 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion include/dsn/utility/TokenBucket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,11 @@ class BasicDynamicTokenBucket
assert(burstSize > 0);

if (burstSize < toConsume) {
return boost::none;
// boost::none
// if we use boost::none here, some compilers will generate warning
// that's actually a false positive of "-Wmaybe-uninitialized".
// https://www.boost.org/doc/libs/1_65_1/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html
return boost::make_optional(false, double());
}

while (toConsume > 0) {
Expand Down
3 changes: 3 additions & 0 deletions include/dsn/utils/time_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ static struct tm *get_localtime(uint64_t ts_ms, struct tm *tm_buf)
}

// get time string, which format is yyyy-MM-dd hh:mm:ss.SSS
// NOTE: using char* as output is usually unsafe. Please use std::string as the output argument
// as long as it's possible.
extern void time_ms_to_string(uint64_t ts_ms, char *str);
extern void time_ms_to_string(uint64_t ts_ms, std::string &str);

// get date string with format of 'yyyy-MM-dd' from given timestamp
inline void time_ms_to_date(uint64_t ts_ms, char *str, int len)
Expand Down
7 changes: 4 additions & 3 deletions src/aio/test/aio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include <dsn/tool-api/async_calls.h>
#include <dsn/utility/filesystem.h>
#include <dsn/utility/smart_pointers.h>

#include <gtest/gtest.h>

Expand Down Expand Up @@ -145,9 +146,9 @@ TEST(core, operation_failed)
auto fp = file::open("tmp_test_file", O_WRONLY, 0600);
EXPECT_TRUE(fp == nullptr);

::dsn::error_code *err = new ::dsn::error_code;
size_t *count = new size_t;
auto io_callback = [err, count](::dsn::error_code e, size_t n) {
auto err = dsn::make_unique<dsn::error_code>();
auto count = dsn::make_unique<size_t>();
auto io_callback = [&err, &count](::dsn::error_code e, size_t n) {
*err = e;
*count = n;
};
Expand Down
9 changes: 0 additions & 9 deletions src/runtime/rpc/rpc_message.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,6 @@
* THE SOFTWARE.
*/

/*
* Description:
* What is this file about?
*
* Revision history:
* xxxx-xx-xx, author, first version
* xxxx-xx-xx, author, fix bug about xxx
*/

#include <dsn/utility/ports.h>
#include <dsn/utility/crc.h>
#include <dsn/utility/transient_memory.h>
Expand Down
13 changes: 1 addition & 12 deletions src/utils/logging.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,7 @@ using namespace tools;
DSN_REGISTER_COMPONENT_PROVIDER(screen_logger, "dsn::tools::screen_logger");
DSN_REGISTER_COMPONENT_PROVIDER(simple_logger, "dsn::tools::simple_logger");

std::function<std::string()> log_prefixed_message_func = []() {
static thread_local std::string prefixed_message;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of memory leak? Can you describe it in detail?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

==3425012==ERROR: AddressSanitizer: heap-use-after-free on address 0x6030000093d0 at pc 0x7fd06fed3480 bp 0x7ffe607c0e40 sp 0x7ffe607c05e8
READ of size 23 at 0x6030000093d0 thread T0
    #0 0x7fd06fed347f  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x9b47f)
    #1 0x559307aa3d56 in std::char_traits<char>::copy(char*, char const*, unsigned long) /usr/include/c++/9/bits/char_traits.h:365
    #2 0x559307aa3d56 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy(char*, char const*, unsigned long) /usr/include/c++/9/bits/basic_string.h:351
    #3 0x559307aa3d56 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy(char*, char const*, unsigned long) /usr/include/c++/9/bits/basic_string.h:346
    #4 0x559307aa3d56 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy_chars(char*, char*, char*) /usr/include/c++/9/bits/basic_string.h:393
    #5 0x559307aa3d56 in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) /usr/include/c++/9/bits/basic_string.tcc:225
    #6 0x7fd06faa586d in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct_aux<char*>(char*, char*, std::__false_type) /usr/include/c++/9/bits/basic_string.h:247
    #7 0x7fd06faa586d in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*) /usr/include/c++/9/bits/basic_string.h:266
    #8 0x7fd06faa586d in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /usr/include/c++/9/bits/basic_string.h:451
    #9 0x7fd06faa586d in operator() /home/wutao1/git/pegasus/rdsn/src/utils/logging.cpp:57
    #10 0x7fd06faa586d in _M_invoke /usr/include/c++/9/bits/std_function.h:286
    #11 0x7fd06face772 in std::function<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > ()>::operator()() const /usr/include/c++/9/bits/std_function.h:688
    #12 0x7fd06face772 in print_header /home/wutao1/git/pegasus/rdsn/src/utils/simple_logger.cpp:72
    #13 0x7fd06facfd25 in dsn::tools::screen_logger::dsn_logv(char const*, char const*, int, dsn_log_level_t, char const*, __va_list_tag*) /home/wutao1/git/pegasus/rdsn/src/utils/simple_logger.cpp:100
    #14 0x7fd06faa2a8e in dsn_logv(char const*, char const*, int, dsn_log_level_t, char const*, __va_list_tag*) /home/wutao1/git/pegasus/rdsn/src/utils/logging.cpp:139
    #15 0x7fd06faa2b9b in dsn_logf(char const*, char const*, int, dsn_log_level_t, char const*, ...) /home/wutao1/git/pegasus/rdsn/src/utils/logging.cpp:151
    #16 0x559307cda30f in dsn::perf_counters::remove_counter(char const*) /home/wutao1/git/pegasus/rdsn/src/perf_counter/perf_counters.cpp:159
    #17 0x559307eac56c in dsn::perf_counter_wrapper::clear() /home/wutao1/git/pegasus/rdsn/include/dsn/perf_counter/perf_counter_wrapper.h:73
    #18 0x559307eac56c in dsn::perf_counter_wrapper::clear() /home/wutao1/git/pegasus/rdsn/include/dsn/perf_counter/perf_counter_wrapper.h:70
    #19 0x559307eac56c in dsn::perf_counter_wrapper::~perf_counter_wrapper() /home/wutao1/git/pegasus/rdsn/include/dsn/perf_counter/perf_counter_wrapper.h:66
    #20 0x559307eac56c in dsn::builtin_counters::~builtin_counters() /home/wutao1/git/pegasus/rdsn/src/perf_counter/builtin_counters.cpp:25
    #21 0x7fd06f482a26  (/lib/x86_64-linux-gnu/libc.so.6+0x49a26)
    #22 0x7fd06f482bdf in exit (/lib/x86_64-linux-gnu/libc.so.6+0x49bdf)
    #23 0x7fd06f4600b9 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b9)
    #24 0x559307a6a92d in _start (/home/wutao1/git/pegasus/rdsn/builder/src/perf_counter/test/dsn_perf_counter_test+0x23092d)

0x6030000093d0 is located 0 bytes inside of 31-byte region [0x6030000093d0,0x6030000093ef)
freed by thread T0 here:
    #0 0x7fd06ff488df in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x1108df)
    #1 0x7fd06f48343e in __call_tls_dtors (/lib/x86_64-linux-gnu/libc.so.6+0x4a43e)

previously allocated by thread T0 here:
    #0 0x7fd06ff47947 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10f947)
    #1 0x7fd06f788e7d in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x142e7d)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"__call_tls_dtors" means the threadlocal vars are freed. But print_header is still reading those vars. It causes an asan error. I have no idea why this happens.


static thread_local std::once_flag flag;
std::call_once(flag, [&]() {
prefixed_message.resize(23);
int tid = dsn::utils::get_current_tid();
sprintf(const_cast<char *>(prefixed_message.c_str()), "unknown.io-thrd.%05d: ", tid);
});

return prefixed_message;
};
std::function<std::string()> log_prefixed_message_func = []() -> std::string { return ": "; };

void set_log_prefixed_message_func(std::function<std::string()> func)
{
Expand Down
19 changes: 10 additions & 9 deletions src/utils/simple_logger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <dsn/utility/filesystem.h>
#include <dsn/utility/flags.h>
#include <dsn/utils/time_utils.h>
#include <fmt/format.h>

namespace dsn {
namespace tools {
Expand Down Expand Up @@ -58,17 +59,17 @@ static void print_header(FILE *fp, dsn_log_level_t log_level)
static char s_level_char[] = "IDWEF";

uint64_t ts = dsn_now_ns();
char str[24];
dsn::utils::time_ms_to_string(ts / 1000000, str);
std::string time_str;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

==3428602==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffec075928 at pc 0x7f1002a58a6d bp 0x7fffec0740e0 sp 0x7fffec073888
READ of size 47 at 0x7fffec075928 thread T0
    #0 0x7f1002a58a6c  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x67a6c)
    #1 0x559a236c7123 in std::char_traits<char>::length(char const*) /usr/include/c++/9/bits/char_traits.h:335
    #2 0x559a236c7123 in fmt::v5::internal::arg_formatter_base<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >::write(char const*) /home/wutao1/git/pegasus/rdsn/thirdparty/output/include/fmt/format.h:1353
    #3 0x559a2371c3ff in fmt::v5::internal::arg_formatter_base<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >::operator()(char const*) /home/wutao1/git/pegasus/rdsn/thirdparty/output/include/fmt/format.h:1421
    #4 0x559a2371c3ff in fmt::v5::internal::result_of<fmt::v5::arg_formatter<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > > (int)>::type fmt::v5::visit_format_arg<fmt::v5::arg_formatter<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >, fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> >(fmt::v5::arg_formatter<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >&&, fmt::v5::basic_format_arg<fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> > const&) /home/wutao1/git/pegasus/rdsn/thirdparty/output/include/fmt/core.h:856
    #5 0x559a237200f9 in fmt::v5::format_handler<fmt::v5::arg_formatter<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >, char, fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> >::on_replacement_field(char const*) /home/wutao1/git/pegasus/rdsn/thirdparty/output/include/fmt/format.h:3097
    #6 0x559a237200f9 in void fmt::v5::internal::parse_format_string<false, char, fmt::v5::format_handler<fmt::v5::arg_formatter<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >, char, fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> >&>(fmt::v5::basic_string_view<char>, fmt::v5::format_handler<fmt::v5::arg_formatter<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >, char, fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> >&) /home/wutao1/git/pegasus/rdsn/thirdparty/output/include/fmt/format.h:2031
    #7 0x7f10029363bc in fmt::v5::buffer_context<char>::type::iterator fmt::v5::internal::vformat_to<char>(fmt::v5::internal::basic_buffer<char>&, fmt::v5::basic_string_view<char>, fmt::v5::basic_format_args<fmt::v5::buffer_context<char>::type>) (/home/wutao1/git/pegasus/rdsn/builder/src/utils/libdsn_utils.so+0x21b3bc)
    #8 0x7f100291400b in fmt::v5::vprint(_IO_FILE*, fmt::v5::basic_string_view<char>, fmt::v5::format_args) (/home/wutao1/git/pegasus/rdsn/builder/src/utils/libdsn_utils.so+0x1f900b)
    #9 0x7f10028c4a7b in std::enable_if<fmt::v5::internal::is_string<char [16]>::value, void>::type fmt::v5::print<char [16], char, char [24], unsigned long, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(_IO_FILE*, char const (&) [16], char const&, char const (&) [24], unsigned long const&, int const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/wutao1/git/pegasus/rdsn/thirdparty/output/include/fmt/core.h:1478
    #10 0x7f10028c4a7b in print_header /home/wutao1/git/pegasus/rdsn/src/utils/simple_logger.cpp:66
    #11 0x7f10028c9b06 in dsn::tools::simple_logger::dsn_log(char const*, char const*, int, dsn_log_level_t, char const*) /home/wutao1/git/pegasus/rdsn/src/utils/simple_logger.cpp:234
    #12 0x559a237dbdb4 in core_dlog_f_Test::TestBody() /home/wutao1/git/pegasus/rdsn/src/utils/test/logging.cpp:78
    #13 0x559a23cafdad in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0xa2bdad)
    #14 0x559a23ca93cc in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0xa253cc)
    #15 0x559a23c8b337 in testing::Test::Run() (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0xa07337)
    #16 0x559a23c8bcbb in testing::TestInfo::Run() (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0xa07cbb)
    #17 0x559a23c8c375 in testing::TestCase::Run() (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0xa08375)
    #18 0x559a23c93624 in testing::internal::UnitTestImpl::RunAllTests() (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0xa0f624)
    #19 0x559a23cb11a9 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0xa2d1a9)
    #20 0x559a23caa44e in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0xa2644e)
    #21 0x559a23c92175 in testing::UnitTest::Run() (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0xa0e175)
    #22 0x559a23546b60 in RUN_ALL_TESTS() /home/wutao1/git/pegasus/rdsn/thirdparty/output/include/gtest/gtest.h:2233
    #23 0x559a23546b60 in main /home/wutao1/git/pegasus/rdsn/src/utils/test/main.cpp:18
    #24 0x7f10020190b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #25 0x559a23592c7d in _start (/home/wutao1/git/pegasus/rdsn/builder/src/utils/test/dsn_utils_tests+0x30ec7d)

Address 0x7fffec075928 is located in stack of thread T0 at offset 760 in frame
    #0 0x7f10028c40bf in print_header /home/wutao1/git/pegasus/rdsn/src/utils/simple_logger.cpp:58

dsn::utils::time_ms_to_string(ts / 1000000, time_str);

int tid = dsn::utils::get_current_tid();
fprintf(fp,
"%c%s (%" PRIu64 " %04x) %s",
s_level_char[log_level],
str,
ts,
tid,
log_prefixed_message_func().c_str());
fmt::print(fp,
"{}{} ({} {}) {}",
s_level_char[log_level],
time_str,
ts,
tid,
log_prefixed_message_func());
}

screen_logger::screen_logger(bool short_header) : logging_provider("./")
Expand Down
1 change: 1 addition & 0 deletions src/utils/test/priority_queue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ TEST(core, blocking_priority_queue)
ASSERT_EQ(0, ct);
ASSERT_EQ(0, d->priority);
ASSERT_EQ(10, d->queue_index);
delete d;

bool flag = false;

Expand Down
11 changes: 11 additions & 0 deletions src/utils/time_utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,16 @@ namespace utils {
fmt::format_to(str, "{:%Y-%m-%d %H:%M:%S}.{}", *ret, static_cast<uint32_t>(ts_ms % 1000));
}

/*extern*/ void time_ms_to_string(uint64_t ts_ms, std::string &str)
{
str.clear();
struct tm tmp;
auto ret = get_localtime(ts_ms, &tmp);
fmt::format_to(std::back_inserter(str),
"{:%Y-%m-%d %H:%M:%S}.{}",
*ret,
static_cast<uint32_t>(ts_ms % 1000));
}

} // namespace utils
} // namespace dsn