public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: Jonathan Wakely <redi@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r10-9941] libstdc++: Fix null dereferences in std::promise Date: Fri, 18 Jun 2021 12:56:49 +0000 (GMT) [thread overview] Message-ID: <20210618125649.789D139DC4E7@sourceware.org> (raw) https://gcc.gnu.org/g:98efaa7ea4d24355024eae7874309efd60baf97e commit r10-9941-g98efaa7ea4d24355024eae7874309efd60baf97e Author: Jonathan Wakely <jwakely@redhat.com> Date: Tue May 4 16:28:57 2021 +0100 libstdc++: Fix null dereferences in std::promise This fixes some ubsan errors in std::promise: future:1153:34: runtime error: member call on null pointer of type 'struct element_type' future:1153:34: runtime error: member access within null pointer of type 'struct element_type' The problem is that the check for a null pointer is done inside the _State::__Setter function, which is only evaluated after evaluating the _M_future->_M_set_result postfix-expression. This change adds a new promise::_M_state() helper for accessing _M_future, and moves the check for no shared state into there, instead of inside the __setter functions. The __setter functions are made always_inline, to avoid the situation where the linker selects the old version of set_value (without the _S_check call) and the new version of __setter (without the _S_check call) and so there is no check. With the always_inline attribute the old version of set_value will either inline the old __setter or call an extern definition of it, and the new set_value will do the check itself, so both versions do the check. libstdc++-v3/ChangeLog: * include/std/future (promise::set_value): Check for existence of shared state before dereferncing it. (promise::set_exception, promise::set_value_at_thread_exit) (promise::set_exception_at_thread_exit): Likewise. (promise<R&>::set_value, promise<R&>::set_exception) (promise<R&>::set_value_at_thread_exit) (promise<R&>::set_exception_at_thread_exit): Likewise. (promise<void>::set_value, promise<void>::set_exception) (promise<void>::set_value_at_thread_exit) (promise<void>::set_exception_at_thread_exit): Likewise. * testsuite/30_threads/promise/members/at_thread_exit2.cc: Remove unused variable. (cherry picked from commit 058d6acefe8bac4a66c8e7fb4951276db188e2d8) Diff: --- libstdc++-v3/include/std/future | 64 +++++++++++++++------- .../30_threads/promise/members/at_thread_exit2.cc | 1 - 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future index ceb1959e3b7..a036dc15682 100644 --- a/libstdc++-v3/include/std/future +++ b/libstdc++-v3/include/std/future @@ -533,26 +533,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; template<typename _Res, typename _Arg> + __attribute__((__always_inline__)) static _Setter<_Res, _Arg&&> - __setter(promise<_Res>* __prom, _Arg&& __arg) + __setter(promise<_Res>* __prom, _Arg&& __arg) noexcept { - _S_check(__prom->_M_future); return _Setter<_Res, _Arg&&>{ __prom, std::__addressof(__arg) }; } template<typename _Res> + __attribute__((__always_inline__)) static _Setter<_Res, __exception_ptr_tag> - __setter(exception_ptr& __ex, promise<_Res>* __prom) + __setter(exception_ptr& __ex, promise<_Res>* __prom) noexcept { - _S_check(__prom->_M_future); return _Setter<_Res, __exception_ptr_tag>{ __prom, &__ex }; } template<typename _Res> + __attribute__((__always_inline__)) static _Setter<_Res, void> - __setter(promise<_Res>* __prom) + __setter(promise<_Res>* __prom) noexcept { - _S_check(__prom->_M_future); return _Setter<_Res, void>{ __prom }; } @@ -1118,36 +1118,44 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Setting the result void set_value(const _Res& __r) - { _M_future->_M_set_result(_State::__setter(this, __r)); } + { _M_state()._M_set_result(_State::__setter(this, __r)); } void set_value(_Res&& __r) - { _M_future->_M_set_result(_State::__setter(this, std::move(__r))); } + { _M_state()._M_set_result(_State::__setter(this, std::move(__r))); } void set_exception(exception_ptr __p) - { _M_future->_M_set_result(_State::__setter(__p, this)); } + { _M_state()._M_set_result(_State::__setter(__p, this)); } void set_value_at_thread_exit(const _Res& __r) { - _M_future->_M_set_delayed_result(_State::__setter(this, __r), + _M_state()._M_set_delayed_result(_State::__setter(this, __r), _M_future); } void set_value_at_thread_exit(_Res&& __r) { - _M_future->_M_set_delayed_result( + _M_state()._M_set_delayed_result( _State::__setter(this, std::move(__r)), _M_future); } void set_exception_at_thread_exit(exception_ptr __p) { - _M_future->_M_set_delayed_result(_State::__setter(__p, this), + _M_state()._M_set_delayed_result(_State::__setter(__p, this), _M_future); } + + private: + _State& + _M_state() + { + __future_base::_State_base::_S_check(_M_future); + return *_M_future; + } }; template<typename _Res> @@ -1229,25 +1237,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Setting the result void set_value(_Res& __r) - { _M_future->_M_set_result(_State::__setter(this, __r)); } + { _M_state()._M_set_result(_State::__setter(this, __r)); } void set_exception(exception_ptr __p) - { _M_future->_M_set_result(_State::__setter(__p, this)); } + { _M_state()._M_set_result(_State::__setter(__p, this)); } void set_value_at_thread_exit(_Res& __r) { - _M_future->_M_set_delayed_result(_State::__setter(this, __r), + _M_state()._M_set_delayed_result(_State::__setter(this, __r), _M_future); } void set_exception_at_thread_exit(exception_ptr __p) { - _M_future->_M_set_delayed_result(_State::__setter(__p, this), + _M_state()._M_set_delayed_result(_State::__setter(__p, this), _M_future); } + + private: + _State& + _M_state() + { + __future_base::_State_base::_S_check(_M_future); + return *_M_future; + } }; /// Explicit specialization for promise<void> @@ -1321,22 +1337,30 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Setting the result void set_value() - { _M_future->_M_set_result(_State::__setter(this)); } + { _M_state()._M_set_result(_State::__setter(this)); } void set_exception(exception_ptr __p) - { _M_future->_M_set_result(_State::__setter(__p, this)); } + { _M_state()._M_set_result(_State::__setter(__p, this)); } void set_value_at_thread_exit() - { _M_future->_M_set_delayed_result(_State::__setter(this), _M_future); } + { _M_state()._M_set_delayed_result(_State::__setter(this), _M_future); } void set_exception_at_thread_exit(exception_ptr __p) { - _M_future->_M_set_delayed_result(_State::__setter(__p, this), + _M_state()._M_set_delayed_result(_State::__setter(__p, this), _M_future); } + + private: + _State& + _M_state() + { + __future_base::_State_base::_S_check(_M_future); + return *_M_future; + } }; template<typename _Ptr_type, typename _Fn, typename _Res> diff --git a/libstdc++-v3/testsuite/30_threads/promise/members/at_thread_exit2.cc b/libstdc++-v3/testsuite/30_threads/promise/members/at_thread_exit2.cc index 59eeae823ec..7db52a6336e 100644 --- a/libstdc++-v3/testsuite/30_threads/promise/members/at_thread_exit2.cc +++ b/libstdc++-v3/testsuite/30_threads/promise/members/at_thread_exit2.cc @@ -119,7 +119,6 @@ void test02() void test03() { std::promise<void> p1; - int i = 0; p1.set_value(); try { p1.set_value_at_thread_exit();
reply other threads:[~2021-06-18 12:56 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210618125649.789D139DC4E7@sourceware.org \ --to=redi@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ --cc=libstdc++-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).