This is the mail archive of the mailing list for the libstdc++ project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR libstdc++/88782 avoid ODR problems in std::make_shared

On 18/01/19 21:28 +0000, Jonathan Wakely wrote:
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/ (_Sp_make_shared_tag::_S_eq): Define.

Tested powerpc64le-linux, committed to trunk.

I'll backport this to gcc-8-branch without the new symbol in the library.

Here's the backport. It's the same, except that it doesn't add the new
_Sp_make_shared_tag::_S_eq function, and so there are no changes to
_Sp_counted_ptr_inplace::_M_get_deleter. This means that -fno-rtti
instantiations of _M_get_deleter will still return null to -frtti
callers. That can be fixed by recompiling the callers with GCC 8.3 or
later (so they don't use _M_get_deleter at all), or just by relinking
so that a -frtti instantiation of _M_get_deleter is kept by the
linker. I'll document that as requested by Richi.

Tested x86_64-linux, committed to gcc-8-branch as r268114.

I've also tested the reproducers from 87520 and 88782 with every
combination of GCC 8.2.0, 8.2.1 (after r266380 but before r268114),
8.2.1 (r268114) and 9.0.0 (r268086) and only the expected cases fail
(i.e.  when both objects use 8.2.0, because obviously the fix isn't
present, or when _M_get_deleter is compiled by 8.x with -fno-rtti and
a caller is compiled by 8.2.0 with -fno-rtti, as described above).

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]