diff --git a/include/fc/container/zeroed_array.hpp b/include/fc/container/zeroed_array.hpp new file mode 100644 index 0000000..0f91a0f --- /dev/null +++ b/include/fc/container/zeroed_array.hpp @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2019 BitShares Blockchain Foundation, and contributors + * + * The MIT License + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#pragma once +#include + +namespace fc { + + template< typename T, size_t N > + class zero_initialized_array; + + template< size_t N > + class zero_initialized_array< unsigned char, N > : public std::array< unsigned char, N > { + public: + zero_initialized_array() : std::array< unsigned char, N >() { } + }; + + template + struct get_typename< zero_initialized_array > + { + static const char* name() + { + static std::string _name = std::string("zero_initialized_array<") + + std::string(fc::get_typename::name()) + + "," + fc::to_string(N) + ">"; + return _name.c_str(); + } + }; + + class variant; + template + void to_variant( const zero_initialized_array& bi, variant& v, uint32_t max_depth = 1 ) + { + to_variant( static_cast&>( bi ), v, max_depth ); + } + template + void from_variant( const variant& v, zero_initialized_array& bi, uint32_t max_depth = 1 ) + { + from_variant( v, static_cast&>( bi ), max_depth ); + } + + namespace raw { + template + inline void pack( Stream& s, const zero_initialized_array& v, uint32_t _max_depth ) { + pack( s, static_cast&>( v ), _max_depth ); + } + template + inline void unpack( Stream& s, zero_initialized_array& v, uint32_t _max_depth ) { + try { + unpack( s, static_cast&>( v ), _max_depth ); + } FC_RETHROW_EXCEPTIONS( warn, "zero_initialized_array", ("length",N) ) + } + } +} diff --git a/include/fc/crypto/elliptic.hpp b/include/fc/crypto/elliptic.hpp index 3784299..e47dee6 100644 --- a/include/fc/crypto/elliptic.hpp +++ b/include/fc/crypto/elliptic.hpp @@ -1,4 +1,5 @@ #pragma once +#include #include #include #include @@ -17,15 +18,15 @@ namespace fc { class private_key_impl; } - typedef fc::sha256 blind_factor_type; - typedef std::array commitment_type; - typedef std::array public_key_data; - typedef fc::sha256 private_key_secret; - typedef std::array public_key_point_data; ///< the full non-compressed version of the ECC point - typedef std::array signature; - typedef std::array compact_signature; - typedef std::vector range_proof_type; - typedef std::array extended_key_data; + typedef fc::sha256 blind_factor_type; + typedef zero_initialized_array commitment_type; + typedef zero_initialized_array public_key_data; + typedef fc::sha256 private_key_secret; + typedef zero_initialized_array public_key_point_data; ///< the full non-compressed version of the ECC point + typedef zero_initialized_array signature; + typedef zero_initialized_array compact_signature; + typedef std::vector range_proof_type; + typedef zero_initialized_array extended_key_data; /** * @class public_key diff --git a/include/fc/thread/future.hpp b/include/fc/thread/future.hpp index 4e98c3d..c0db411 100644 --- a/include/fc/thread/future.hpp +++ b/include/fc/thread/future.hpp @@ -3,9 +3,9 @@ #include #include #include -#include -#include +#include +#include //#define FC_TASK_NAMES_ARE_MANDATORY 1 #ifdef FC_TASK_NAMES_ARE_MANDATORY @@ -77,23 +77,22 @@ namespace fc { void _wait( const microseconds& timeout_us ); void _wait_until( const time_point& timeout_us ); - void _enqueue_thread(); - void _dequeue_thread(); void _notify(); - void _set_timeout(); void _set_value(const void* v); void _on_complete( detail::completion_handler* c ); private: + void _enqueue_thread(); + void _dequeue_thread(); + friend class thread; friend struct context; friend class thread_d; - bool _ready; - mutable spin_yield_lock _spin_yield; - thread* _blocked_thread; - unsigned _blocked_fiber_count; + std::atomic _ready; + std::atomic _blocked_thread; + std::atomic _blocked_fiber_count; time_point _timeout; fc::exception_ptr _exceptp; bool _canceled; @@ -103,7 +102,7 @@ namespace fc { private: #endif const char* _desc; - detail::completion_handler* _compl; + std::atomic _compl; }; template diff --git a/include/fc/thread/task.hpp b/include/fc/thread/task.hpp index f7c2ec5..92e1b48 100644 --- a/include/fc/thread/task.hpp +++ b/include/fc/thread/task.hpp @@ -4,6 +4,8 @@ #include #include +#include + namespace fc { struct context; class spin_lock; diff --git a/src/crypto/elliptic_common.cpp b/src/crypto/elliptic_common.cpp index dd89c3a..2ed84a8 100644 --- a/src/crypto/elliptic_common.cpp +++ b/src/crypto/elliptic_common.cpp @@ -19,7 +19,7 @@ namespace fc { namespace ecc { namespace detail { - typedef std::array chr37; + typedef zero_initialized_array chr37; fc::sha256 _left( const fc::sha512& v ) { diff --git a/src/crypto/elliptic_secp256k1.cpp b/src/crypto/elliptic_secp256k1.cpp index b180d44..477077e 100644 --- a/src/crypto/elliptic_secp256k1.cpp +++ b/src/crypto/elliptic_secp256k1.cpp @@ -53,7 +53,7 @@ namespace fc { namespace ecc { public_key_data _key; }; - typedef std::array chr37; + typedef zero_initialized_array chr37; chr37 _derive_message( const public_key_data& key, int i ); fc::sha256 _left( const fc::sha512& v ); fc::sha256 _right( const fc::sha512& v ); diff --git a/src/thread/future.cpp b/src/thread/future.cpp index c9dbffa..b4779dd 100644 --- a/src/thread/future.cpp +++ b/src/thread/future.cpp @@ -6,7 +6,6 @@ #include - namespace fc { promise_base::promise_base( const char* desc ) @@ -22,6 +21,8 @@ namespace fc { _compl(nullptr) { } + promise_base::~promise_base() { } + const char* promise_base::get_desc()const{ return _desc; } @@ -34,16 +35,14 @@ namespace fc { #endif } bool promise_base::ready()const { - return _ready; + return _ready.load(); } bool promise_base::error()const { - { synchronized(_spin_yield) - return _exceptp != nullptr; - } + return std::atomic_load( &_exceptp ) != nullptr; } void promise_base::set_exception( const fc::exception_ptr& e ){ - _exceptp = e; + std::atomic_store( &_exceptp, e ); _set_value(nullptr); } @@ -54,16 +53,21 @@ namespace fc { _wait_until( time_point::now() + timeout_us ); } void promise_base::_wait_until( const time_point& timeout_us ){ - { synchronized(_spin_yield) - if( _ready ) { - if( _exceptp ) - _exceptp->dynamic_rethrow_exception(); - return; - } - _enqueue_thread(); + if( _ready.load() ) { + fc::exception_ptr ex = std::atomic_load( &_exceptp ); + if( ex ) + ex->dynamic_rethrow_exception(); + return; + } + _enqueue_thread(); + // Need to check _ready again to avoid a race condition. + if( _ready.load() ) + { + _dequeue_thread(); + return _wait_until( timeout_us ); // this will simply return or throw _exceptp } - std::exception_ptr e; + std::exception_ptr e; // // Create shared_ptr to take ownership of this; i.e. this will // be deleted when p_this goes out of scope. Consequently, @@ -71,9 +75,7 @@ namespace fc { // before we're done reading/writing instance variables! // See https://github.com/cryptonomex/graphene/issues/597 // - ptr p_this = shared_from_this(); - try { // @@ -94,61 +96,45 @@ namespace fc { if( e ) std::rethrow_exception(e); - if( _ready ) - { - if( _exceptp ) - _exceptp->dynamic_rethrow_exception(); - return; - } + if( _ready.load() ) return _wait_until( timeout_us ); // this will simply return or throw _exceptp + FC_THROW_EXCEPTION( timeout_exception, "" ); } void promise_base::_enqueue_thread(){ - ++_blocked_fiber_count; + _blocked_fiber_count.fetch_add( 1 ); + thread* blocked_thread = _blocked_thread.load(); // only one thread can wait on a promise at any given time - assert(!_blocked_thread || - _blocked_thread == &thread::current()); - _blocked_thread = &thread::current(); + do + assert( !blocked_thread || blocked_thread == &thread::current() ); + while( !_blocked_thread.compare_exchange_weak( blocked_thread, &thread::current() ) ); } void promise_base::_dequeue_thread(){ - synchronized(_spin_yield) - if (!--_blocked_fiber_count) - _blocked_thread = nullptr; + if( _blocked_fiber_count.fetch_add( -1 ) == 1 ) + _blocked_thread.store( nullptr ); } void promise_base::_notify(){ // copy _blocked_thread into a local so that if the thread unblocks (e.g., // because of a timeout) before we get a chance to notify it, we won't be // calling notify on a null pointer - thread* blocked_thread; - { synchronized(_spin_yield) - blocked_thread = _blocked_thread; - } + thread* blocked_thread = _blocked_thread.load(); if( blocked_thread ) blocked_thread->notify( shared_from_this() ); } - promise_base::~promise_base() { } - void promise_base::_set_timeout(){ - if( _ready ) - return; - set_exception( std::make_shared() ); - } + void promise_base::_set_value(const void* s){ - // slog( "%p == %d", &_ready, int(_ready)); -// BOOST_ASSERT( !_ready ); - { synchronized(_spin_yield) - if (_ready) //don't allow promise to be set more than once + bool ready = false; + if( !_ready.compare_exchange_strong( ready, true ) ) //don't allow promise to be set more than once return; - _ready = true; - } - _notify(); - if( nullptr != _compl ) { - _compl->on_complete(s,_exceptp); - } + _notify(); + auto* hdl = _compl.load(); + if( nullptr != hdl ) + hdl->on_complete( s, std::atomic_load( &_exceptp ) ); } + void promise_base::_on_complete( detail::completion_handler* c ) { - { synchronized(_spin_yield) - delete _compl; - _compl = c; - } + auto* hdl = _compl.load(); + while( !_compl.compare_exchange_weak( hdl, c ) ); + delete hdl; } } diff --git a/src/thread/thread_d.hpp b/src/thread/thread_d.hpp index 77e8fb7..7209485 100644 --- a/src/thread/thread_d.hpp +++ b/src/thread/thread_d.hpp @@ -1,12 +1,14 @@ #include +#include #include #include #include "context.hpp" #include #include #include + +#include #include -//#include namespace fc { struct sleep_priority_less { @@ -390,7 +392,14 @@ namespace fc { /* NB: At least on Win64, this only catches a yield while in the body of * a catch block; it fails to catch a yield while unwinding the stack, which * is probably just as likely to cause crashes */ - assert(std::current_exception() == std::exception_ptr()); + if( std::current_exception() != std::exception_ptr() ) + { + std::stringstream stacktrace; + print_stacktrace( stacktrace ); + elog( "Thread ${name} yielded in exception handler!\n${trace}", + ("name",thread::current().name())("trace",stacktrace.str()) ); + assert( std::current_exception() == std::exception_ptr() ); + } check_for_timeouts(); if( !current ) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 01cf205..6a3e474 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -26,6 +26,7 @@ target_link_libraries( ecc_test fc ) add_executable( all_tests all_tests.cpp compress/compress.cpp crypto/aes_test.cpp + crypto/array_initialization_test.cpp crypto/base_n_tests.cpp crypto/bigint_test.cpp crypto/blind.cpp diff --git a/tests/crypto/array_initialization_test.cpp b/tests/crypto/array_initialization_test.cpp new file mode 100644 index 0000000..a8d2724 --- /dev/null +++ b/tests/crypto/array_initialization_test.cpp @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2019 BitShares Blockchain Foundation, and contributors. + * + * The MIT License + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include + +#include +#include + +#include + +static void check_null_key() +{ + fc::ecc::public_key_data key1; + fc::ecc::public_key_data key2; + unsigned char zeroes[33]; + static_assert( key1.size() == sizeof(zeroes), "Wrong array size!" ); + memset( zeroes, 0, sizeof(zeroes) ); + BOOST_CHECK( !memcmp( key1.data(), zeroes, sizeof(zeroes) ) ); + BOOST_CHECK( !memcmp( key2.data(), zeroes, sizeof(zeroes) ) ); + + // now "pollute" the keys for the next invocation + key1 = fc::ecc::private_key::generate().get_public_key(); + for( unsigned char c = 0; c < key2.size(); c++ ) + { + key2[c] = c ^ 17; + zeroes[c] = c ^ 47; + } + + // ...and use them to prevent the compiler from optimizing the pollution away. + wlog( "Key1: ${k}", ("k",fc::ecc::public_key::to_base58(key1)) ); + wlog( "Key2: ${k}", ("k",fc::ecc::public_key::to_base58(key2)) ); +} + +BOOST_AUTO_TEST_SUITE(fc_crypto) + +BOOST_AUTO_TEST_CASE(array_init_test) +{ + check_null_key(); + check_null_key(); + { + char junk[128]; + fc::rand_bytes( junk, sizeof(junk) ); + } + check_null_key(); +} + +BOOST_AUTO_TEST_SUITE_END()