From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2181) id 789D139DC4E7; Fri, 18 Jun 2021 12:56:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 789D139DC4E7 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jonathan Wakely To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r10-9941] libstdc++: Fix null dereferences in std::promise X-Act-Checkin: gcc X-Git-Author: Jonathan Wakely X-Git-Refname: refs/heads/releases/gcc-10 X-Git-Oldrev: 44985f6ba5d587e1c649a09ea1eb72434918597d X-Git-Newrev: 98efaa7ea4d24355024eae7874309efd60baf97e Message-Id: <20210618125649.789D139DC4E7@sourceware.org> Date: Fri, 18 Jun 2021 12:56:49 +0000 (GMT) X-BeenThere: libstdc++-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Jun 2021 12:56:49 -0000 https://gcc.gnu.org/g:98efaa7ea4d24355024eae7874309efd60baf97e commit r10-9941-g98efaa7ea4d24355024eae7874309efd60baf97e Author: Jonathan Wakely 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::set_value, promise::set_exception) (promise::set_value_at_thread_exit) (promise::set_exception_at_thread_exit): Likewise. (promise::set_value, promise::set_exception) (promise::set_value_at_thread_exit) (promise::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 + __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 + __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 + __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 @@ -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 @@ -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 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 p1; - int i = 0; p1.set_value(); try { p1.set_value_at_thread_exit();