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

Fix some more code sanitizer errors #644

Merged
merged 2 commits into from
Feb 22, 2018

Conversation

aautushka
Copy link

Here is the list of addressed errors:

The function handle_block not returning a value

create_committee_member' test: invalid database index used

/tests/tests/operation_tests.cpp:513:140: runtime error: member call on address 0x60c0000a5700 which does not point to an object of type 'primary_index'
0x60c0000a5700: note: object is of type 'graphene::db::primary_index<graphene::chain::generic_index<graphene::chain::committee_member_object, boost::multi_index::multi_index_container<graphene::chain::committee_member_object, boost::multi_index::indexed_by<boost::multi_index::ordered_unique<boost::multi_index::tag<graphene::chain::by_id, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::member<graphene::db::object, graphene::db::object_id_type, &graphene::db::object::id>, mpl_::na>, boost::multi_index::ordered_unique<boost::multi_index::tag<graphene::chain::by_account, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::member<graphene::chain::committee_member_object, graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object>, &graphene::chain::committee_member_object::committee_member_account>, mpl_::na>, boost::multi_index::ordered_unique<boost::multi_index::tag<graphene::chain::by_vote_id, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::member<graphene::chain::committee_member_object, graphene::chain::vote_id_type, &graphene::chain::committee_member_object::vote_id>, mpl_::na>, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, std::allocator<graphene::chain::committee_member_object> > > >'
 73 00 80 64  f0 26 e6 06 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  be be be be
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'graphene::db::primary_index<graphene::chain::generic_index<graphene::chain::committee_member_object, boost::multi_index::multi_index_container<graphene::chain::committee_member_object, boost::multi_index::indexed_by<boost::multi_index::ordered_unique<boost::multi_index::tag<graphene::chain::by_id, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::member<graphene::db::object, graphene::db::object_id_type, &graphene::db::object::id>, mpl_::na>, boost::multi_index::ordered_unique<boost::multi_index::tag<graphene::chain::by_account, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::member<graphene::chain::committee_member_object, graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object>, &graphene::chain::committee_member_object::committee_member_account>, mpl_::na>, boost::multi_index::ordered_unique<boost::multi_index::tag<graphene::chain::by_vote_id, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, boost::multi_index::member<graphene::chain::committee_member_object, graphene::chain::vote_id_type, &graphene::chain::committee_member_object::vote_id>, mpl_::na>, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, std::allocator<graphene::chain::committee_member_object> > > >'

verify_authority: stack use after scope

==23394==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffdfdf1ad00 at pc 0x000001ed0f9a bp 0x7ffdfdf17d80 sp 0x7ffdfdf17d78
READ of size 8 at 0x7ffdfdf1ad00 thread T0
    #0 0x1ed0f99 in graphene::chain::sign_state::check_authority(graphene::chain::authority const*, unsigned int)
    #1 0x1e8411f in graphene::chain::verify_authority(std::vector<fc::static_variant<graphene::chain::transfer_operation, graphene::chain::account_create_operation, graphene::chain::account_update_operation, graphene::chain::account_transfer_operation, graphene::chain::asset_publish_feed_operation, graphene::chain::witness_create_operation, graphene::chain::witness_update_operation, graphene::chain::proposal_create_operation, graphene::chain::proposal_update_operation, graphene::chain::proposal_delete_operation, graphene::chain::committee_member_create_operation, graphene::chain::committee_member_update_operation, graphene::chain::committee_member_update_global_parameters_operation, graphene::chain::vesting_balance_withdraw_operation, graphene::chain::assert_operation, graphene::chain::balance_claim_operation, graphene::chain::pk_update_operation>, std::allocator<fc::static_variant<graphene::chain::transfer_operation, graphene::chain::account_create_operation, graphene::chain::account_update_operation, graphene::chain::account_transfer_operation, graphene::chain::asset_publish_feed_operation, graphene::chain::witness_create_operation, graphene::chain::witness_update_operation, graphene::chain::proposal_create_operation, graphene::chain::proposal_update_operation, graphene::chain::proposal_delete_operation, graphene::chain::committee_member_create_operation, graphene::chain::committee_member_update_operation, graphene::chain::committee_member_update_global_parameters_operation, graphene::chain::vesting_balance_withdraw_operation, graphene::chain::assert_operation, graphene::chain::balance_claim_operation, graphene::chain::pk_update_operation> > > const&, boost::container::flat_set<graphene::chain::public_key_type, std::less<graphene::chain::public_key_type>, boost::container::new_allocator<graphene::chain::public_key_type> > const&, std::function<graphene::chain::authority const* (graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object>)> const&, std::function<graphene::chain::authority const* (graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object>)> const&, unsigned int, bool, boost::container::flat_set<graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object>, std::less<graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object> >, boost::container::new_allocator<graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object> > > const&, boost::container::flat_set<graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object>, std::less<graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object> >, boost::container::new_allocator<graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object> > > const&)
    #2 0x1e92a51 in graphene::chain::signed_transaction::verify_authority(fc::sha256 const&, std::function<graphene::chain::authority const* (graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object>)> const&, std::function<graphene::chain::authority const* (graphene::db::object_id<(unsigned char)1, (unsigned char)2, graphene::chain::account_object>)> const&, unsigned int) const
    #3 0x19209af in graphene::chain::database::_apply_transaction(graphene::chain::signed_transaction const&)
    #4 0x192ad39 in graphene::chain::database::_push_transaction(graphene::chain::signed_transaction const&)
    #5 0x192e0ee in graphene::chain::database::push_transaction(graphene::chain::signed_transaction const&, unsigned int)

@@ -103,6 +103,7 @@ void transaction::get_required_authorities( flat_set<account_id_type>& active, f



const fc::flat_set<public_key_type> empty_keyset;
Copy link
Member

Choose a reason for hiding this comment

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

I thought it's boost::flat_set. It's actually a fc wrapper? Perhaps just remove fc:: here, since everywhere else doesn't have it.

Copy link
Author

Choose a reason for hiding this comment

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

fc::flat_set is a typedef. Will remove fc:: here

@@ -537,7 +537,7 @@ namespace detail {
_is_finished_syncing = true;
Copy link
Member

Choose a reason for hiding this comment

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

Actually this code block can be removed because it will never execute?
Either already returned, or an exception was thrown out.

Copy link
Author

Choose a reason for hiding this comment

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

I don't have enough background to answer your question.

At this point we are only concerned about stability and code sanitizers. Cleaning up the code is a different matter.

@abitmore abitmore added this to the Next Non-Consensus-Changing Release milestone Feb 8, 2018
@oxarbitrage oxarbitrage self-requested a review February 20, 2018 23:12
Copy link
Member

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

looks good, i think we should add this. i guess AddressSanitizer was used in the entire project and only this 3 problems were found. if this is the case, great, if more were found please post them up as issue so we can work on them. thanks.

@oxarbitrage oxarbitrage merged commit 0da4ff2 into bitshares:develop Feb 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants