diff options
author | Jonathan Wakely <jwakely@redhat.com> | 2019-01-18 21:28:48 +0000 |
---|---|---|
committer | Jonathan Wakely <redi@gcc.gnu.org> | 2019-01-18 21:28:48 +0000 |
commit | fb54aa5915bcea13e23b34f54a5364e01c8185af (patch) | |
tree | 59213495b3b36387ea8e49c7cd7ccee5a4cd5b83 | |
parent | dea93fc60d293b08d137f08d9129de6f4fc772db (diff) | |
download | gcc-fb54aa5915bcea13e23b34f54a5364e01c8185af.zip gcc-fb54aa5915bcea13e23b34f54a5364e01c8185af.tar.gz gcc-fb54aa5915bcea13e23b34f54a5364e01c8185af.tar.bz2 |
PR libstdc++/88782 avoid ODR problems in std::make_shared
The old version of _Sp_counted_ptr_inplace::_M_get_deleter (up to GCC
8.2.0) expects to be passed a real std::typeinfo object, so mixing that
with the new definition of the __shared_ptr constructor (which always
passes the fake tag) leads to accessing the fake object as a real
std::typeinfo. Instead of trying to make it safe to mix the old and new
definitions, just stop using that function. By passing a reference to
__shared_ptr::_M_ptr to the __shared_count constructor it can be set
directly, without needing to obtain the pointer via the _M_get_deleter
back-channel. This avoids a virtual dispatch (which fixes PR 87514).
This means that code built against new libstdc++ headers doesn't use
_M_get_deleter at all, and so make_shared works the same whether RTTI is
enabled or not.
Also change _M_get_deleter so that it checks for a real type_info object
even when RTTI is disabled, by calling a library function. Unless
libstdc++ itself is built without RTTI that library function will be
able to test if it's the right type_info. This means the new definition
of _M_get_deleter can handle both the fake type_info tag and a real
type_info object, even if built without RTTI.
If linking to objects built against older versions of libstdc++ then if
all objects use -frtti or all use -fno-rtti, then the caller of
_M_get_deleter and the definition of _M_get_deleter will be consistent
and it will work. If mixing -frtti with -fno-rtti it can still fail if
the linker picks an old definition of _M_get_deleter and an old
__shared_ptr constructor that are incompatible. In that some or all
objects might need to be recompiled.
PR libstdc++/87514
PR libstdc++/87520
PR libstdc++/88782
* config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Export new symbol.
* include/bits/shared_ptr.h
(shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...))
(allocate_shared): Change to use new tag type.
* include/bits/shared_ptr_base.h (_Sp_make_shared_tag::_S_eq):
Declare new member function.
(_Sp_alloc_shared_tag): Define new type.
(_Sp_counted_ptr_inplace): Declare __shared_count<_Lp> as a friend.
(_Sp_counted_ptr_inplace::_M_get_deleter) [!__cpp_rtti]: Use
_Sp_make_shared_tag::_S_eq to check type_info.
(__shared_count(Ptr, Deleter),__shared_count(Ptr, Deleter, Alloc)):
Constrain to prevent being called with _Sp_alloc_shared_tag.
(__shared_count(_Sp_make_shared_tag, const _Alloc&, Args&&...)):
Replace constructor with ...
(__shared_count(Tp*&, _Sp_alloc_shared_tag<_Alloc>, Args&&...)): Use
reference parameter so address of the new object can be returned to
the caller. Obtain the allocator from the tag type.
(__shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)): Replace
constructor with ...
(__shared_ptr(_Sp_alloc_shared_tag<Alloc>, Args&&...)): Pass _M_ptr
to the __shared_count constructor.
(__allocate_shared): Change to use new tag type.
* src/c++11/shared_ptr.cc (_Sp_make_shared_tag::_S_eq): Define.
From-SVN: r268086
-rw-r--r-- | libstdc++-v3/ChangeLog | 29 | ||||
-rw-r--r-- | libstdc++-v3/config/abi/pre/gnu.ver | 3 | ||||
-rw-r--r-- | libstdc++-v3/include/bits/shared_ptr.h | 7 | ||||
-rw-r--r-- | libstdc++-v3/include/bits/shared_ptr_base.h | 74 | ||||
-rw-r--r-- | libstdc++-v3/src/c++11/shared_ptr.cc | 12 |
5 files changed, 91 insertions, 34 deletions
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 3ea3168..4ca096d 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,32 @@ +2019-01-18 Jonathan Wakely <jwakely@redhat.com> + + PR libstdc++/87514 + PR libstdc++/87520 + PR libstdc++/88782 + * config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Export new symbol. + * include/bits/shared_ptr.h + (shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)) + (allocate_shared): Change to use new tag type. + * include/bits/shared_ptr_base.h (_Sp_make_shared_tag::_S_eq): + Declare new member function. + (_Sp_alloc_shared_tag): Define new type. + (_Sp_counted_ptr_inplace): Declare __shared_count<_Lp> as a friend. + (_Sp_counted_ptr_inplace::_M_get_deleter) [!__cpp_rtti]: Use + _Sp_make_shared_tag::_S_eq to check type_info. + (__shared_count(Ptr, Deleter),__shared_count(Ptr, Deleter, Alloc)): + Constrain to prevent being called with _Sp_alloc_shared_tag. + (__shared_count(_Sp_make_shared_tag, const _Alloc&, Args&&...)): + Replace constructor with ... + (__shared_count(Tp*&, _Sp_alloc_shared_tag<_Alloc>, Args&&...)): Use + reference parameter so address of the new object can be returned to + the caller. Obtain the allocator from the tag type. + (__shared_ptr(_Sp_make_shared_tag, const Alloc&, Args&&...)): Replace + constructor with ... + (__shared_ptr(_Sp_alloc_shared_tag<Alloc>, Args&&...)): Pass _M_ptr + to the __shared_count constructor. + (__allocate_shared): Change to use new tag type. + * src/c++11/shared_ptr.cc (_Sp_make_shared_tag::_S_eq): Define. + 2019-01-17 Jonathan Wakely <jwakely@redhat.com> * src/c++17/fs_ops.cc diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index 3b254b2..34c70b6 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -2235,6 +2235,9 @@ GLIBCXX_3.4.26 { _ZNSolsEDn; _ZNSt13basic_ostreamIwSt11char_traitsIwEElsEDn; + # _Sp_make_shared_tag::_S_eq + _ZNSt19_Sp_make_shared_tag5_S_eqERKSt9type_info; + } GLIBCXX_3.4.25; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/include/bits/shared_ptr.h b/libstdc++-v3/include/bits/shared_ptr.h index d504627..ee815f0 100644 --- a/libstdc++-v3/include/bits/shared_ptr.h +++ b/libstdc++-v3/include/bits/shared_ptr.h @@ -355,9 +355,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION private: // This constructor is non-standard, it is used by allocate_shared. template<typename _Alloc, typename... _Args> - shared_ptr(_Sp_make_shared_tag __tag, const _Alloc& __a, - _Args&&... __args) - : __shared_ptr<_Tp>(__tag, __a, std::forward<_Args>(__args)...) + shared_ptr(_Sp_alloc_shared_tag<_Alloc> __tag, _Args&&... __args) + : __shared_ptr<_Tp>(__tag, std::forward<_Args>(__args)...) { } template<typename _Yp, typename _Alloc, typename... _Args> @@ -699,7 +698,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION inline shared_ptr<_Tp> allocate_shared(const _Alloc& __a, _Args&&... __args) { - return shared_ptr<_Tp>(_Sp_make_shared_tag(), __a, + return shared_ptr<_Tp>(_Sp_alloc_shared_tag<_Alloc>{__a}, std::forward<_Args>(__args)...); } diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h index b45cbf7..0367c2d 100644 --- a/libstdc++-v3/include/bits/shared_ptr_base.h +++ b/libstdc++-v3/include/bits/shared_ptr_base.h @@ -501,8 +501,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION struct _Sp_make_shared_tag { private: - template<typename _Tp, _Lock_policy _Lp> - friend class __shared_ptr; template<typename _Tp, typename _Alloc, _Lock_policy _Lp> friend class _Sp_counted_ptr_inplace; @@ -512,8 +510,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION alignas(type_info) static constexpr char __tag[sizeof(type_info)] = { }; return reinterpret_cast<const type_info&>(__tag); } + + static bool _S_eq(const type_info&) noexcept; }; + template<typename _Alloc> + struct _Sp_alloc_shared_tag + { + const _Alloc& _M_a; + }; + template<typename _Tp, typename _Alloc, _Lock_policy _Lp> class _Sp_counted_ptr_inplace final : public _Sp_counted_base<_Lp> { @@ -560,24 +566,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION this->~_Sp_counted_ptr_inplace(); } - // Sneaky trick so __shared_ptr can get the managed pointer. + private: + friend class __shared_count<_Lp>; // To be able to call _M_ptr(). + + // No longer used, but code compiled against old libstdc++ headers + // might still call it from __shared_ptr ctor to get the pointer out. virtual void* _M_get_deleter(const std::type_info& __ti) noexcept override { + auto __ptr = const_cast<typename remove_cv<_Tp>::type*>(_M_ptr()); // Check for the fake type_info first, so we don't try to access it - // as a real type_info object. - if (&__ti == &_Sp_make_shared_tag::_S_ti()) - return const_cast<typename remove_cv<_Tp>::type*>(_M_ptr()); + // as a real type_info object. Otherwise, check if it's the real + // type_info for this class. With RTTI enabled we can check directly, + // or call a library function to do it. + if (&__ti == &_Sp_make_shared_tag::_S_ti() + || #if __cpp_rtti - // Callers compiled with old libstdc++ headers and RTTI enabled - // might pass this instead: - else if (__ti == typeid(_Sp_make_shared_tag)) - return const_cast<typename remove_cv<_Tp>::type*>(_M_ptr()); + __ti == typeid(_Sp_make_shared_tag) +#else + _Sp_make_shared_tag::_S_eq(__ti) #endif + ) + return __ptr; return nullptr; } - private: _Tp* _M_ptr() noexcept { return _M_impl._M_storage._M_ptr(); } _Impl _M_impl; @@ -593,6 +606,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template<_Lock_policy _Lp> class __shared_count { + template<typename _Tp> + struct __not_alloc_shared_tag { using type = void; }; + + template<typename _Tp> + struct __not_alloc_shared_tag<_Sp_alloc_shared_tag<_Tp>> { }; + public: constexpr __shared_count() noexcept : _M_pi(0) { } @@ -622,12 +641,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : __shared_count(__p, __sp_array_delete{}, allocator<void>()) { } - template<typename _Ptr, typename _Deleter> + template<typename _Ptr, typename _Deleter, + typename = typename __not_alloc_shared_tag<_Deleter>::type> __shared_count(_Ptr __p, _Deleter __d) : __shared_count(__p, std::move(__d), allocator<void>()) { } - template<typename _Ptr, typename _Deleter, typename _Alloc> + template<typename _Ptr, typename _Deleter, typename _Alloc, + typename = typename __not_alloc_shared_tag<_Deleter>::type> __shared_count(_Ptr __p, _Deleter __d, _Alloc __a) : _M_pi(0) { typedef _Sp_counted_deleter<_Ptr, _Deleter, _Alloc, _Lp> _Sp_cd_type; @@ -648,17 +669,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } template<typename _Tp, typename _Alloc, typename... _Args> - __shared_count(_Sp_make_shared_tag, _Tp*, const _Alloc& __a, + __shared_count(_Tp*& __p, _Sp_alloc_shared_tag<_Alloc> __a, _Args&&... __args) - : _M_pi(0) { typedef _Sp_counted_ptr_inplace<_Tp, _Alloc, _Lp> _Sp_cp_type; - typename _Sp_cp_type::__allocator_type __a2(__a); + typename _Sp_cp_type::__allocator_type __a2(__a._M_a); auto __guard = std::__allocate_guarded(__a2); _Sp_cp_type* __mem = __guard.get(); - ::new (__mem) _Sp_cp_type(__a, std::forward<_Args>(__args)...); - _M_pi = __mem; + auto __pi = ::new (__mem) + _Sp_cp_type(__a._M_a, std::forward<_Args>(__args)...); __guard = nullptr; + _M_pi = __pi; + __p = __pi->_M_ptr(); } #if _GLIBCXX_USE_DEPRECATED @@ -1318,17 +1340,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION protected: // This constructor is non-standard, it is used by allocate_shared. template<typename _Alloc, typename... _Args> - __shared_ptr(_Sp_make_shared_tag __tag, const _Alloc& __a, - _Args&&... __args) - : _M_ptr(), _M_refcount(__tag, (_Tp*)0, __a, - std::forward<_Args>(__args)...) - { - // _M_ptr needs to point to the newly constructed object. - // This relies on _Sp_counted_ptr_inplace::_M_get_deleter. - void* __p = _M_refcount._M_get_deleter(_Sp_make_shared_tag::_S_ti()); - _M_ptr = static_cast<_Tp*>(__p); - _M_enable_shared_from_this_with(_M_ptr); - } + __shared_ptr(_Sp_alloc_shared_tag<_Alloc> __tag, _Args&&... __args) + : _M_ptr(), _M_refcount(_M_ptr, __tag, std::forward<_Args>(__args)...) + { _M_enable_shared_from_this_with(_M_ptr); } template<typename _Tp1, _Lock_policy _Lp1, typename _Alloc, typename... _Args> @@ -1808,7 +1822,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION inline __shared_ptr<_Tp, _Lp> __allocate_shared(const _Alloc& __a, _Args&&... __args) { - return __shared_ptr<_Tp, _Lp>(_Sp_make_shared_tag(), __a, + return __shared_ptr<_Tp, _Lp>(_Sp_alloc_shared_tag<_Alloc>{__a}, std::forward<_Args>(__args)...); } diff --git a/libstdc++-v3/src/c++11/shared_ptr.cc b/libstdc++-v3/src/c++11/shared_ptr.cc index 6e838de..1f1323e 100644 --- a/libstdc++-v3/src/c++11/shared_ptr.cc +++ b/libstdc++-v3/src/c++11/shared_ptr.cc @@ -94,5 +94,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } #endif + bool + _Sp_make_shared_tag::_S_eq(const type_info& ti) noexcept + { +#if __cpp_rtti + return ti == typeid(_Sp_make_shared_tag); +#else + // If libstdc++ itself is built with -fno-rtti then just assume that + // make_shared and allocate_shared will never be used with -frtti. + return false; +#endif + } + _GLIBCXX_END_NAMESPACE_VERSION } // namespace |