From 058d6acefe8bac4a66c8e7fb4951276db188e2d8 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Tue, 4 May 2021 16:28:57 +0100 Subject: [PATCH] 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. --- libstdc++-v3/include/std/future | 64 +++++++++++++------ .../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 ef15fefa53c..09e54c3703b 100644 --- a/libstdc++-v3/include/std/future +++ b/libstdc++-v3/include/std/future @@ -532,26 +532,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 }; } @@ -1130,36 +1130,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 @@ -1241,25 +1249,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 @@ -1333,22 +1349,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 9f824efde52..679d580cee3 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 @@ -118,7 +118,6 @@ void test02() void test03() { std::promise p1; - int i = 0; p1.set_value(); try { p1.set_value_at_thread_exit();