Message ID | 20180621232851.GA6885@redhat.com |
---|---|
State | New |
Headers | show |
Series | PR libstdc++/86138 prevent implicit instantiation of COW empty rep | expand |
On 22/06/18 00:28 +0100, Jonathan Wakely wrote: >The explicit instantiation declarations for std::basic_string are >disabled for C++17 (and later) so that basic_string symbols get >implicitly instantiated in every translation unit that needs them. On >targets that don't support STB_GNU_UNIQUE this leads to multiple copies >of the empty rep symbol for COW strings. In order to detect whether a >COW string needs to deallocate its storage it compares the address with >the empty rep. When there are multiple copies of the empty rep object >the address is not unique, and so string destructors try to delete the >empty rep, which crashes. > >In order to guarantee uniqueness of the _S_empty_rep_storage symbol this >patch adds an explicit instantiation declaration for just that symbol. >This means the other symbols are still implicitly instantiated in C++17 >code, but for the empty rep the definition in the library gets used. > >Separately, there is no need for C++17 code to implicitly instantiate >the I/O functions for strings, so this also restores the explicit >instantiation declarations for those functions. > > PR libstdc++/86138 > * include/bits/basic_string.tcc: > [__cplusplus > 201402 && !_GLIBCXX_USE_CXX11_ABI] > (basic_string<char>::_Rep::_S_empty_rep_storage) > (basic_string<wchar_t>::_Rep::_S_empty_rep_storage): Add explicit > instantiation declarations. > [__cplusplus > 201402] (operator>>, operator<<, getline): Re-enable > explicit instantiation declarations. > * testsuite/21_strings/basic_string/cons/char/86138.cc: New. > * testsuite/21_strings/basic_string/cons/wchar_t/86138.cc: New. > >Tested x86_64-linux, committed to trunk. > >If this passes testing on Cygwin I'll also backport it to gcc-7 and >gcc-8, as the explicit instantiation declarations are disabled for >C++17 on those branches. The new tests are failing with _GLIBCXX_ASSERTIONS or _GLIBCXX_DEBUG defined, because enabling assertions suppresses the explicit instantiation declarations. However the COW empty reps and the I/O functions don't contain any assertions, so don't need to be implicitly instantiated. Tested x86_64-linux, committed to trunk. commit 22bd831a1bc38ba3e114c6166e96817fe03f006b Author: Jonathan Wakely <jwakely@redhat.com> Date: Tue Jun 26 22:14:29 2018 +0100 Declare some explicit instantiations for strings in Debug Mode The empty reps and the I/O functions do not need to be implicitly instantiated to enable assertions, so declare the explicit instantiations when _GLIBCXX_EXTERN_TEMPLATE == -1 (i.e. when _GLIBCXX_ASSERTIONS is defined). PR libstdc++/86138 * include/bits/basic_string.tcc: [_GLIBCXX_EXTERN_TEMPLATE < 0] Declare explicit instantiations of COW empty reps and I/O functions. diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc index 9fbea84c4af..04b68ca0202 100644 --- a/libstdc++-v3/include/bits/basic_string.tcc +++ b/libstdc++-v3/include/bits/basic_string.tcc @@ -1597,13 +1597,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Inhibit implicit instantiations for required instantiations, // which are defined via explicit instantiations elsewhere. -#if _GLIBCXX_EXTERN_TEMPLATE > 0 +#if _GLIBCXX_EXTERN_TEMPLATE // The explicit instantiations definitions in src/c++11/string-inst.cc // are compiled as C++14, so the new C++17 members aren't instantiated. // Until those definitions are compiled as C++17 suppress the declaration, // so C++17 code will implicitly instantiate std::string and std::wstring // as needed. -# if __cplusplus <= 201402L +# if __cplusplus <= 201402L && _GLIBCXX_EXTERN_TEMPLATE > 0 extern template class basic_string<char>; # elif ! _GLIBCXX_USE_CXX11_ABI // Still need to prevent implicit instantiation of the COW empty rep, @@ -1626,7 +1626,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION getline(basic_istream<char>&, string&); #ifdef _GLIBCXX_USE_WCHAR_T -# if __cplusplus <= 201402L +# if __cplusplus <= 201402L && _GLIBCXX_EXTERN_TEMPLATE > 0 extern template class basic_string<wchar_t>; # elif ! _GLIBCXX_USE_CXX11_ABI extern template basic_string<wchar_t>::size_type @@ -1646,7 +1646,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION basic_istream<wchar_t>& getline(basic_istream<wchar_t>&, wstring&); #endif // _GLIBCXX_USE_WCHAR_T -#endif // _GLIBCXX_EXTERN_TEMPLATE > 0 +#endif // _GLIBCXX_EXTERN_TEMPLATE _GLIBCXX_END_NAMESPACE_VERSION } // namespace std
diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc index be8815c711b..9fbea84c4af 100644 --- a/libstdc++-v3/include/bits/basic_string.tcc +++ b/libstdc++-v3/include/bits/basic_string.tcc @@ -1597,8 +1597,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Inhibit implicit instantiations for required instantiations, // which are defined via explicit instantiations elsewhere. -#if _GLIBCXX_EXTERN_TEMPLATE > 0 && __cplusplus <= 201402L +#if _GLIBCXX_EXTERN_TEMPLATE > 0 + // The explicit instantiations definitions in src/c++11/string-inst.cc + // are compiled as C++14, so the new C++17 members aren't instantiated. + // Until those definitions are compiled as C++17 suppress the declaration, + // so C++17 code will implicitly instantiate std::string and std::wstring + // as needed. +# if __cplusplus <= 201402L extern template class basic_string<char>; +# elif ! _GLIBCXX_USE_CXX11_ABI + // Still need to prevent implicit instantiation of the COW empty rep, + // to ensure the definition in libstdc++.so is unique (PR 86138). + extern template basic_string<char>::size_type + basic_string<char>::_Rep::_S_empty_rep_storage[]; +# endif + extern template basic_istream<char>& operator>>(basic_istream<char>&, string&); @@ -1613,7 +1626,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION getline(basic_istream<char>&, string&); #ifdef _GLIBCXX_USE_WCHAR_T +# if __cplusplus <= 201402L extern template class basic_string<wchar_t>; +# elif ! _GLIBCXX_USE_CXX11_ABI + extern template basic_string<wchar_t>::size_type + basic_string<wchar_t>::_Rep::_S_empty_rep_storage[]; +# endif + extern template basic_istream<wchar_t>& operator>>(basic_istream<wchar_t>&, wstring&); @@ -1626,8 +1645,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION extern template basic_istream<wchar_t>& getline(basic_istream<wchar_t>&, wstring&); -#endif -#endif +#endif // _GLIBCXX_USE_WCHAR_T +#endif // _GLIBCXX_EXTERN_TEMPLATE > 0 _GLIBCXX_END_NAMESPACE_VERSION } // namespace std diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/86138.cc b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/86138.cc new file mode 100644 index 00000000000..224ea42a3c7 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/86138.cc @@ -0,0 +1,30 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17" } +// { dg-do compile { target c++17 } } +// { dg-final { scan-assembler-not "_ZNSs4_Rep20_S_empty_rep_storageE:" } } + +#undef _GLIBCXX_USE_CXX11_ABI +#define _GLIBCXX_USE_CXX11_ABI 0 +#include <string> + +void +test01(std::string* s) +{ + s->~basic_string(); +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/86138.cc b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/86138.cc new file mode 100644 index 00000000000..88f136bc7bc --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/86138.cc @@ -0,0 +1,30 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17" } +// { dg-do compile { target c++17 } } +// { dg-final { scan-assembler-not "_ZNSbIwSt11char_traitsIwESaIwEE4_Rep20_S_empty_rep_storageE:" } } + +#undef _GLIBCXX_USE_CXX11_ABI +#define _GLIBCXX_USE_CXX11_ABI 0 +#include <string> + +void +test01(std::wstring* s) +{ + s->~basic_string(); +}