On 26/04/19 14:36 +0100, Jonathan Wakely wrote:
On 26/04/19 14:30 +0200, Jakub Jelinek wrote:
On Fri, Apr 26, 2019 at 01:25:37PM +0100, Jonathan Wakely wrote:
On 26/04/19 12:48 +0200, Jakub Jelinek wrote:
Hi!

The following patch updates the baseline symbols files from April 18th
Fedora rpm build.  I've verified the only added lines are for the
GLIBCXX_3.4.26 or CXXABI_1.3.12 symvers and I don't see any new long double
symbol on powerpc64 or s390x, except I had to manually remove
FUNC:_ZNSbIwSt11char_traitsIwESaIwEE19_M_replace_dispatchIPKcEERS2_N9__gnu_cxx17__normal_iteratorIPwS2_EESA_T_SB_St12__false_type@@GLIBCXX_3.4
lines that started to appear on all but s390x builds in Fedora rpm builds
(but they don't show up e.g. on my workstation).  Guess we need to make the
wildcards more careful.

The attached patch would do that. The symbol above is a function
template, so we don't need to export it from the lib (because user
code that needs it will instantiate it anyway). It's only called from
the basic_string<C,T,A>::replace<Iter>(iterator, iterator, Iter, Iter)
function template, which isn't exported from the lib.

Thanks, LGTM.
grep _ZNSbIwSt11char_traitsIwESaIwEE.*_M_replace 
libstdc++-v3/config/abi/post/*/{,*/}*.txt | grep -v 
_ZNSbIwSt11char_traitsIwESaIwEE14_M_replace_aux | grep -v 
_ZNSbIwSt11char_traitsIwESaIwEE15_M_replace_safe
prints nothing, so the patch looks correct and safe.

Ok for 9.1.

I tracked down where that symbol was being instantiated, and it's not
needed anyway, and can be suppressed by using if-constexpr. Here's
what I'm going to commit for trunk (after testing finishes).

This should be safe for the branch too, but we can just make the
linker script change for now and backport this for 9.2 once it's been
on trunk for a while.

Here's the final patch I tested and committed to trunk (I had to
replace std::-si_same_v with std::is_same because the
<bits/locale_conv.h> header is used in C++11 and C++14 too).


commit 7ef6887dab4368b82b1cda06775dcd5810e3c479
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Fri Apr 26 14:23:09 2019 +0100

    Reduce code instantiated by filesystem::path::_S_convert_loc
    
    Jakub noted in https://gcc.gnu.org/ml/libstdc++/2019-04/msg00140.html
    that an unwanted std::wstring::_M_replace_dispatch symbol has started to
    be exported from the Fedora shared library. This symbol is triggered by
    the instantiation of std::wstring::assign(const char*, const char*) from
    std::__str_codecvt_in which is called from path::_S_convert_loc. The
    branch that triggers that instantiation can't actually happen in that
    case, because codecvt facets will only return noconv when the input and
    output types are the same. Guarding the assign call with an if-constexpr
    check that the types are the same avoids instantiating template
    specializations that will never actually be needed.
    
            * config/abi/pre/gnu.ver (GLIBCXX_3.4): Replace wildcard that matches
            wstring::_M_replace_dispatch with more specific patterns.
            * include/bits/fs_path.h (path::_S_convert_loc<_InputIterator>):
            Create const std::string to avoid redundant call to _S_convert_loc
            with non-const pointers.
            * include/bits/locale_conv.h (__do_str_codecvt): Use if-constexpr to
            avoid unnecessary basic_string::assign instantiations.

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index e8cf6f0a4a9..7eff4e983f3 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -299,7 +299,8 @@ GLIBCXX_3.4 {
     _ZNSbIwSt11char_traitsIwESaIwEE12_S_constructE[jmy]wRKS1_;
     _ZNSbIwSt11char_traitsIwESaIwEE12_S_empty_repEv;
     _ZNSbIwSt11char_traitsIwESaIwEE13_S_copy_chars*;
-    _ZNSbIwSt11char_traitsIwESaIwEE[0-9][0-9]_M_replace*;
+    _ZNSbIwSt11char_traitsIwESaIwEE14_M_replace_aux*;
+    _ZNSbIwSt11char_traitsIwESaIwEE15_M_replace_safe*;
     _ZNSbIwSt11char_traitsIwESaIwEE4_Rep10_M_destroy*;
     _ZNSbIwSt11char_traitsIwESaIwEE4_Rep10_M_dispose*;
     _ZNSbIwSt11char_traitsIwESaIwEE4_Rep10_M_refcopyEv;
diff --git a/libstdc++-v3/include/bits/fs_path.h b/libstdc++-v3/include/bits/fs_path.h
index 3674b4391f8..28878c627a7 100644
--- a/libstdc++-v3/include/bits/fs_path.h
+++ b/libstdc++-v3/include/bits/fs_path.h
@@ -539,7 +539,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       _S_convert_loc(_InputIterator __src, __null_terminated,
 		     const std::locale& __loc)
       {
-	std::string __s = _S_string_from_iter(__src);
+	const std::string __s = _S_string_from_iter(__src);
 	return _S_convert_loc(__s.data(), __s.data() + __s.size(), __loc);
       }
 
diff --git a/libstdc++-v3/include/bits/locale_conv.h b/libstdc++-v3/include/bits/locale_conv.h
index d7510dff80e..4cb9c39ebb4 100644
--- a/libstdc++-v3/include/bits/locale_conv.h
+++ b/libstdc++-v3/include/bits/locale_conv.h
@@ -88,8 +88,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       if (__result == codecvt_base::noconv)
 	{
-	  __outstr.assign(__first, __last);
-	  __count = __last - __first;
+	  // The codecvt facet will only return noconv when the types are
+	  // the same, so avoid instantiating basic_string::assign otherwise
+	  if _GLIBCXX17_CONSTEXPR (is_same<typename _Codecvt::intern_type,
+					   typename _Codecvt::extern_type>())
+	    {
+	      __outstr.assign(__first, __last);
+	      __count = __last - __first;
+	    }
 	}
       else
 	{

Reply via email to