* [committed 3/4] libstdc++: Fix undefined behaviour in std::string
2021-05-04 22:17 [committed 1/4] libstdc++ Fix undefined behaviour in testsuite Jonathan Wakely
2021-05-04 22:18 ` [committed 2/4] libstdc++: Fix null dereference in pb_ds containers Jonathan Wakely
@ 2021-05-04 22:19 ` Jonathan Wakely
2021-05-04 22:20 ` [committed 4/4] libstdc++: Fix null dereferences in std::promise Jonathan Wakely
2 siblings, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2021-05-04 22:19 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 708 bytes --]
This fixes a ubsan error when constructing a string with a null pointer:
bits/basic_string.h:534:21: runtime error: applying non-zero offset 18446744073709551615 to null pointer
The _M_construct function only cares whether the second pointer is
non-null, so create a non-null value without undefined arithmetic.
We can also pass the random_access_iterator_tag directly to the
_M_construct function, to avoid going via the tag dispatching
_M_construct_aux, because we know we have pointers not integers here.
libstdc++-v3/ChangeLog:
* include/bits/basic_string.h (basic_string(const CharT*, const A&)):
Do not do arithmetic on null pointer.
Tested x86_64-linux. Committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1722 bytes --]
commit 789c57bc5fe023fc6dc72ade4afcb0916ff788d3
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue May 4 15:49:38 2021
libstdc++: Fix undefined behaviour in std::string
This fixes a ubsan error when constructing a string with a null pointer:
bits/basic_string.h:534:21: runtime error: applying non-zero offset 18446744073709551615 to null pointer
The _M_construct function only cares whether the second pointer is
non-null, so create a non-null value without undefined arithmetic.
We can also pass the random_access_iterator_tag directly to the
_M_construct function, to avoid going via the tag dispatching
_M_construct_aux, because we know we have pointers not integers here.
libstdc++-v3/ChangeLog:
* include/bits/basic_string.h (basic_string(const CharT*, const A&)):
Do not do arithmetic on null pointer.
diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index fba7c6f3354..84356adc7ae 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -531,7 +531,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
#endif
basic_string(const _CharT* __s, const _Alloc& __a = _Alloc())
: _M_dataplus(_M_local_data(), __a)
- { _M_construct(__s, __s ? __s + traits_type::length(__s) : __s+npos); }
+ {
+ const _CharT* __end = __s ? __s + traits_type::length(__s)
+ // We just need a non-null pointer here to get an exception:
+ : reinterpret_cast<const _CharT*>(__alignof__(_CharT));
+ _M_construct(__s, __end, random_access_iterator_tag());
+ }
/**
* @brief Construct string as multiple characters.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [committed 4/4] libstdc++: Fix null dereferences in std::promise
2021-05-04 22:17 [committed 1/4] libstdc++ Fix undefined behaviour in testsuite Jonathan Wakely
2021-05-04 22:18 ` [committed 2/4] libstdc++: Fix null dereference in pb_ds containers Jonathan Wakely
2021-05-04 22:19 ` [committed 3/4] libstdc++: Fix undefined behaviour in std::string Jonathan Wakely
@ 2021-05-04 22:20 ` Jonathan Wakely
2 siblings, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2021-05-04 22:20 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]
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.
Tested x86_64-linux. Committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 7515 bytes --]
commit 058d6acefe8bac4a66c8e7fb4951276db188e2d8
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue May 4 16:28:57 2021
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.
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<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 };
}
@@ -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<typename _Res>
@@ -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<void>
@@ -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<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 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<void> p1;
- int i = 0;
p1.set_value();
try {
p1.set_value_at_thread_exit();
^ permalink raw reply [flat|nested] 4+ messages in thread