Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues detect by valgrind #654

Closed
mingodad opened this issue Feb 20, 2018 · 4 comments

Comments

5 participants
@mingodad
Copy link

commented Feb 20, 2018

Here is the output of running valgrind on a debug build of a fresh git clone on ubuntu 14.04 64bits.

valgrind ./rai_node --debug_block_count
==1104== Memcheck, a memory error detector
==1104== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==1104== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==1104== Command: ./rai_node --debug_block_count
==1104== 
==1104== Invalid read of size 4
==1104==    at 0x4E41404: pthread_mutex_lock (pthread_mutex_lock.c:66)
==1104==    by 0xB31959: mdb_txn_renew0 (mdb.c:2743)
==1104==    by 0xB3221D: mdb_txn_begin (mdb.c:2944)
==1104==    by 0xB2E7F2: rai::transaction::transaction(rai::mdb_env&, MDB_txn*, bool) (utility.cpp:117)
==1104==    by 0x9573E3: rai::node::node(rai::node_init&, boost::asio::io_context&, boost::filesystem::path const&, rai::alarm&, rai::node_config const&, rai::work_pool&) (node.cpp:1531)
==1104==    by 0x9545F1: rai::node::node(rai::node_init&, boost::asio::io_context&, unsigned short, boost::filesystem::path const&, rai::alarm&, rai::logging const&, rai::work_pool&) (node.cpp:1344)
==1104==    by 0xA0960B: void __gnu_cxx::new_allocator<rai::node>::construct<rai::node, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(rai::node*, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (in /home/mingo/dev/c/A_frameworks/raiblocks/build/rai_node)
==1104==    by 0xA0378C: void std::allocator_traits<std::allocator<rai::node> >::construct<rai::node, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::allocator<rai::node>&, rai::node*, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (alloc_traits.h:475)
==1104==    by 0x9FC93C: std::_Sp_counted_ptr_inplace<rai::node, std::allocator<rai::node>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::allocator<rai::node>, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (shared_ptr_base.h:526)
==1104==    by 0x9F3D0D: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<rai::node, std::allocator<rai::node>, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::_Sp_make_shared_tag, rai::node*, std::allocator<rai::node> const&, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (shared_ptr_base.h:637)
==1104==    by 0x9E7E0D: std::__shared_ptr<rai::node, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<rai::node>, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::_Sp_make_shared_tag, std::allocator<rai::node> const&, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (shared_ptr_base.h:1295)
==1104==    by 0x9DA222: std::shared_ptr<rai::node>::shared_ptr<std::allocator<rai::node>, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::_Sp_make_shared_tag, std::allocator<rai::node> const&, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (shared_ptr.h:344)
==1104==  Address 0x4024018 is not stack'd, malloc'd or (recently) free'd
==1104== 
==1104== 
==1104== Process terminating with default action of signal 11 (SIGSEGV)
==1104==  Access not within mapped region at address 0x4024018
==1104==    at 0x4E41404: pthread_mutex_lock (pthread_mutex_lock.c:66)
==1104==    by 0xB31959: mdb_txn_renew0 (mdb.c:2743)
==1104==    by 0xB3221D: mdb_txn_begin (mdb.c:2944)
==1104==    by 0xB2E7F2: rai::transaction::transaction(rai::mdb_env&, MDB_txn*, bool) (utility.cpp:117)
==1104==    by 0x9573E3: rai::node::node(rai::node_init&, boost::asio::io_context&, boost::filesystem::path const&, rai::alarm&, rai::node_config const&, rai::work_pool&) (node.cpp:1531)
==1104==    by 0x9545F1: rai::node::node(rai::node_init&, boost::asio::io_context&, unsigned short, boost::filesystem::path const&, rai::alarm&, rai::logging const&, rai::work_pool&) (node.cpp:1344)
==1104==    by 0xA0960B: void __gnu_cxx::new_allocator<rai::node>::construct<rai::node, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(rai::node*, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (in /home/mingo/dev/c/A_frameworks/raiblocks/build/rai_node)
==1104==    by 0xA0378C: void std::allocator_traits<std::allocator<rai::node> >::construct<rai::node, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::allocator<rai::node>&, rai::node*, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (alloc_traits.h:475)
==1104==    by 0x9FC93C: std::_Sp_counted_ptr_inplace<rai::node, std::allocator<rai::node>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::allocator<rai::node>, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (shared_ptr_base.h:526)
==1104==    by 0x9F3D0D: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<rai::node, std::allocator<rai::node>, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::_Sp_make_shared_tag, rai::node*, std::allocator<rai::node> const&, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (shared_ptr_base.h:637)
==1104==    by 0x9E7E0D: std::__shared_ptr<rai::node, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<rai::node>, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::_Sp_make_shared_tag, std::allocator<rai::node> const&, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (shared_ptr_base.h:1295)
==1104==    by 0x9DA222: std::shared_ptr<rai::node>::shared_ptr<std::allocator<rai::node>, rai::node_init&, boost::asio::io_context&, int, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&>(std::_Sp_make_shared_tag, std::allocator<rai::node> const&, rai::node_init&, boost::asio::io_context&, int&&, boost::filesystem::path const&, rai::alarm&, rai::logging&, rai::work_pool&) (shared_ptr.h:344)
==1104==  If you believe this happened as a result of a stack
==1104==  overflow in your program's main thread (unlikely but
==1104==  possible), you can try to increase the size of the
==1104==  main thread stack using the --main-stacksize= flag.
==1104==  The main thread stack size used in this run was 8388608.
==1104== 
==1104== HEAP SUMMARY:
==1104==     in use at exit: 163,075 bytes in 674 blocks
==1104==   total heap usage: 1,783 allocs, 1,109 frees, 3,425,639 bytes allocated
==1104== 
==1104== LEAK SUMMARY:
==1104==    definitely lost: 0 bytes in 0 blocks
==1104==    indirectly lost: 0 bytes in 0 blocks
==1104==      possibly lost: 35,078 bytes in 250 blocks
==1104==    still reachable: 127,997 bytes in 424 blocks
==1104==         suppressed: 0 bytes in 0 blocks
==1104== Rerun with --leak-check=full to see details of leaked memory
==1104== 
==1104== For counts of detected and suppressed errors, rerun with: -v
==1104== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Killed

@smukil

This comment has been minimized.

Copy link

commented Feb 23, 2018

I read the code around this failure a bit and it seems like this could be due to a lack of proper error checking.
We pass the 'environment_a' object here which is of type 'rai::mdb_env':
https://github.com/nanocurrency/raiblocks/blob/e9592e5ad42859e30275fd365049931f45726178/rai/node/utility.cpp#L117

That gets its value from:
https://github.com/nanocurrency/raiblocks/blob/e9592e5ad42859e30275fd365049931f45726178/rai/node/node.cpp#L1358

If you look at how 'block_store' initializes its 'environment' variable:
https://github.com/nanocurrency/raiblocks/blob/702da39639241d748cf31c82c8c2bc5bbfb73be9/rai/blockstore.cpp#L236

It gets 'error_a' back as 'true' if it failed to initialize the 'environment' object. However, we never check if the 'environment' is valid before using it. I'm guessing when we pass that all the way up to rai::transaction::transaction(...), it goes into LMDB (via mdb_txn_begin()) and crashes inside, since the contract expects a pre-allocated 'mdb_env' object.

@rkeene rkeene added this to the V18.0 milestone Aug 23, 2018

@rkeene rkeene self-assigned this Aug 23, 2018

@rkeene rkeene added the bug label Aug 23, 2018

@zhyatt zhyatt added this to Unscheduled in V18 Dec 27, 2018

@rkeene rkeene moved this from Unscheduled to CP 1 (2018-01-09) in V18 Dec 31, 2018

@zhyatt zhyatt moved this from CP 1 (2018-01-09) to CP 2 (2018-01-16) in V18 Jan 8, 2019

@zhyatt zhyatt moved this from CP 2 (2018-01-16) to CP 3 (2018-01-23) in V18 Jan 17, 2019

@zhyatt zhyatt moved this from CP 3 (2018-01-27) to During RC in V18 Jan 30, 2019

@zhyatt zhyatt removed this from During RC in V18 Feb 6, 2019

@zhyatt zhyatt modified the milestones: V18.0, V19.0 Feb 6, 2019

@zhyatt zhyatt assigned SergiySW and unassigned rkeene Feb 6, 2019

@SergiySW SergiySW added this to CP1 in V19 Feb 15, 2019

@SergiySW

This comment has been minimized.

Copy link
Collaborator

commented Mar 2, 2019

Old issue seems fixed.

In order to run LMDB under Valgrind, the maximum mapsize must be smaller than half your available RAM.

By default for node it's 128GB, so running valgring requires small changes

Valgrind detected a few more issues, so leave it open

==6860== Conditional jump or move depends on uninitialised value(s)
==6860==    at 0x6AFAC0: boost::filesystem::detail::directory_iterator_increment(boost::filesystem::directory_iterator&, boost::system::error_code*) (in /root/rai_test/nano_node)
==6860==    by 0x6B2649: boost::filesystem::detail::directory_iterator_construct(boost::filesystem::directory_iterator&, boost::filesystem::path const&, boost::system::error_code*) (in /root/rai_test/nano_node)
==6860==    by 0x6996CE: boost::log::v2s_mt_posix::sinks::(anonymous namespace)::file_collector::scan_for_files(boost::log::v2s_mt_posix::sinks::file::scan_method, boost::filesystem::path const&, unsigned int*) (in /root/rai_test/nano_node)
==6860==    by 0x277F40: boost::shared_ptr<boost::log::v2s_mt_posix::sinks::synchronous_sink<boost::log::v2s_mt_posix::sinks::text_file_backend> > boost::log::v2s_mt_posix::aux::add_file_log<boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::format, char const [25]>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::max_size, unsigned long>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::scan_method, boost::log::v2s_mt_posix::sinks::file::scan_method const>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::auto_flush, bool>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::rotation_size, unsigned long>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::target, boost::filesystem::path>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::file_name, boost::filesystem::path const>, boost::parameter::aux::empty_arg_list> > > > > > > >(boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::format, char const [25]>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::max_size, unsigned long>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::scan_method, boost::log::v2s_mt_posix::sinks::file::scan_method const>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::auto_flush, bool>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::rotation_size, unsigned long>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::target, boost::filesystem::path>, boost::parameter::aux::arg_list<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::file_name, boost::filesystem::path const>, boost::parameter::aux::empty_arg_list> > > > > > > const&) (file.hpp:82)
==6860==    by 0x276FA4: boost::shared_ptr<boost::log::v2s_mt_posix::sinks::synchronous_sink<boost::log::v2s_mt_posix::sinks::text_file_backend> > boost::log::v2s_mt_posix::add_file_log<boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::target, boost::filesystem::path>, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::file_name, boost::filesystem::path const>, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::rotation_size, unsigned long>, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::auto_flush, bool>, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::scan_method, boost::log::v2s_mt_posix::sinks::file::scan_method const>, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::max_size, unsigned long>, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::format, char const [25]> >(boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::target, boost::filesystem::path> const&, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::file_name, boost::filesystem::path const> const&, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::rotation_size, unsigned long> const&, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::auto_flush, bool> const&, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::scan_method, boost::log::v2s_mt_posix::sinks::file::scan_method const> const&, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::max_size, unsigned long> const&, boost::parameter::aux::tagged_argument<boost::log::v2s_mt_posix::keywords::tag::format, char const [25]> const&) (file.hpp:127)
==6860==    by 0x271E08: nano::logging::init(boost::filesystem::path const&) (logging.cpp:19)
==6860==    by 0x1BCB29: nano_daemon::daemon::run(boost::filesystem::path const&, nano::node_flags const&) (daemon.cpp:21)
==6860==    by 0x1CBEEE: main (entry.cpp:107)
@SergiySW

This comment has been minimized.

Copy link
Collaborator

commented Mar 15, 2019

Fixed via #1814

@SergiySW SergiySW closed this Mar 15, 2019

@SergiySW

This comment has been minimized.

Copy link
Collaborator

commented Apr 27, 2019

Last warning will be fixed in Boost 1.71 boostorg/circular_buffer#18
boostorg/circular_buffer@03a5c84

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.