[Bug tree-optimization/52188] New: [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[Bug tree-optimization/52188] New: [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

             Bug #: 52188
           Summary: [4.7 regression] IPA-CP change broke libstdc++ symbol
                    versioning on Solaris
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: ro@gcc.gnu.org
                CC: jamborm@gcc.gnu.org, paolo.carlini@oracle.com
              Host: *-*-solaris2*
            Target: *-*-solaris2.*
             Build: *-*-solaris2.*


As already discussed to some length in

  http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01163.html

the following patch

2011-07-18  Martin Jambor  <mjambor@suse.cz>

        * ipa-prop.h: Include alloc-pool.h, all sorts of updates to general
        comments.
        (ipcp_values_pool): Declare.
        (ipcp_sources_pool): Likewise.
[...]

caused libstdc++.so symbol versioning to be broken on Solaris.  Before the
patch,
when compiling locale-const.cc only contains references and definitions of

_ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv.constprop.36

After the patch, the same source has references to

_ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv

The latter is matched by the linker map and exported at version GLIBCXX_3.4.15,
which is effectively closed with the release of gcc 4.6 where the symbol wasn't
present.

To avoid breaking symbol versioning on Solaris, the question is what to do
here:
we obviously cannot export it at 3.4.15, but could instead export it at
3.4.17 *on Solaris* instead.  Before going that route, I need to make certain
that this isn't just a code generation accident and the symbol can vanish again
later: the exported interface of a shared object *must not* depend on the 
vagaries of code generation changes.

Martin?

[Bug tree-optimization/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #1 from Rainer Orth <ro at gcc dot gnu.org> 2012-02-09 18:31:18 UTC ---
Created attachment 26629
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26629
preprocesed source

compile with

cc1plus -fpreprocessed locale-inst.ii -quiet -O2 -fPIC -o locale-inst.s

[Bug tree-optimization/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
   Target Milestone|---                         |4.7.0
           Severity|normal                      |major

[Bug tree-optimization/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #2 from Rainer Orth <ro at gcc dot gnu.org> 2012-02-09 18:33:03 UTC ---
Created attachment 26630
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26630
asembler output before patch

[Bug tree-optimization/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #3 from Rainer Orth <ro at gcc dot gnu.org> 2012-02-09 18:33:54 UTC ---
Created attachment 26631
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26631
assembler output after patch

[Bug tree-optimization/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at CeBiTec dot Uni-Bielefeld.DE]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #4 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2012-02-10 10:24:36 UTC ---
> caused libstdc++.so symbol versioning to be broken on Solaris.  Before the
> patch,
> when compiling locale-const.cc only contains references and definitions of
>
> _ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv.constprop.36
>
> After the patch, the same source has references to
>
> _ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv

The change isn't Solaris-specific, btw.  I've just compiled the same
preprocessed file (with -fpermissive added to avoid

/vol/gcc/src/hg/trunk/reghunt/libstdc++-v3/libsupc++/new:93:54: error:
'operator new' takes type 'size_t' ('long unsigned int') as first parameter
[-fpermissive]

) on x86_64-unknown-linux-gnu: with gcc 4.6 off the top of the 4.6
branch, the symbol doesn't occur in the output at all, with mainline, it
does.

    Rainer

[Bug tree-optimization/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-10 11:23:49 UTC ---
Then simply don't export it?  It's a template instantiation after all.

[Bug tree-optimization/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-02-10
     Ever Confirmed|0                           |1

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-10 11:31:00 UTC ---
So, if we'd want to be really consistent with symbols remaining in the
libstdc++ object files at different optimization levels (I guess if you
build libstdc++ with -O3 things might be different again) then we need
to avoid removing unused instantiated templates as part of the optimization
process.  Not sure on what basis we choose to be exported symbols right now.

[Bug tree-optimization/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-10 12:30:39 UTC ---
The bug was that the gnu.ver wildcards were too generic and thus matched even
something that wasn't intended to be exported and just happened to be exported
as an implementation detail.
This caused a problem on Linux several years ago (I think in 2005), and since
then these are forcefully exported from compatibility.cc:
/* gcc-3.4.4
_ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv
_ZNSt19istreambuf_iteratorIwSt11char_traitsIwEEppEv
 */

namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION

  template
    istreambuf_iterator<char>&
    istreambuf_iterator<char>::operator++();

#ifdef _GLIBCXX_USE_WCHAR_T
  template
    istreambuf_iterator<wchar_t>&
    istreambuf_iterator<wchar_t>::operator++();
#endif

_GLIBCXX_END_NAMESPACE_VERSION
} // namespace

but not on Solaris, because it either doesn't support GNU symver, or doesn't
have corresponding runtime support.
I think the bug is not on the optimization side, it is fine if the
optimizations change stuff that are implementation details, C++ isn't a very
good language for ABI stability and just requires lots of work on the libstdc++
side to preserve at least some.

So, I'd say it is time to preprocess the *.ver files and tweak them based on
target OS or its version, and additionally you'll want (for Solaris) enable the
above hunk (sans the following stuff), to make sure it doesn't go again when
the optimizers change.

[Bug tree-optimization/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[jamborm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #8 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-02-16 15:55:04 UTC ---
First and foremost, sorry for the big delay but I could not have a
look at this PR earlier.  Nevertheless, I doubt that the decision of
the new IPA-CP not to clone the function in question can be called a
bug.  Yes, if the heuristics or other early optimizations results
change, the cloning decision might change again in the future - even
in between minor versions if we are really unlucky.

[Bug libstdc++/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2012-02/msg00819.htm
                   |                            |l
          Component|tree-optimization           |libstdc++
         AssignedTo|unassigned at gcc dot       |ro at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #9 from Rainer Orth <ro at gcc dot gnu.org> 2012-02-16 19:50:23 UTC ---
Not really an optimization issue.

Patch posted.

  Rainer

[Bug libstdc++/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at CeBiTec dot Uni-Bielefeld.DE]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #11 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2012-02-16 19:58:27 UTC ---
> --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-10 12:30:39 UTC ---
> The bug was that the gnu.ver wildcards were too generic and thus matched even
> something that wasn't intended to be exported and just happened to be exported
> as an implementation detail.
> This caused a problem on Linux several years ago (I think in 2005), and since
> then these are forcefully exported from compatibility.cc:
[...]
> but not on Solaris, because it either doesn't support GNU symver, or doesn't
> have corresponding runtime support.

ld.so.1 lacks the runtime support, and likely won't get it any time soon.

> I think the bug is not on the optimization side, it is fine if the
> optimizations change stuff that are implementation details, C++ isn't a very
> good language for ABI stability and just requires lots of work on the libstdc++
> side to preserve at least some.
>
> So, I'd say it is time to preprocess the *.ver files and tweak them based on
> target OS or its version, and additionally you'll want (for Solaris) enable the
That's what my proposed patch does.  What do you think about it?

> above hunk (sans the following stuff), to make sure it doesn't go again when
> the optimizers change.

Given that they weren't present in 4.6, the first release with versioned
libstdc++.so on Solaris, I think it's easier to simply keep them
unexported, as Richard suggested.

    Rainer

[Bug libstdc++/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at CeBiTec dot Uni-Bielefeld.DE]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #12 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2012-02-16 20:01:03 UTC ---
> --- Comment #8 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-02-16 15:55:04 UTC ---
> First and foremost, sorry for the big delay but I could not have a
> look at this PR earlier.  Nevertheless, I doubt that the decision of

No worries, the other replies already pushed me in the right direction.

> the new IPA-CP not to clone the function in question can be called a
> bug.  Yes, if the heuristics or other early optimizations results
> change, the cloning decision might change again in the future - even
> in between minor versions if we are really unlucky.

Thanks.  This just means we have to be extra careful with symbol
versioning.  Handling PR libstdc++/52191 would certainly help quite a
bit, and other versions runtime libs don't use much (if any) wildcards
in their version maps, so won't be easily affected.

    Rainer

[Bug libstdc++/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at CeBiTec dot Uni-Bielefeld.DE]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #10 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2012-02-16 19:54:52 UTC ---
> --- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-10 11:23:49 UTC ---
> Then simply don't export it?  It's a template instantiation after all.

I didn't think about that, given my limited knowledge of C++.  Should
have guessed, though, given that it wasn't missed in 4.6.

    Rainer

[Bug libstdc++/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-20 11:38:41 UTC ---
(In reply to comment #8)
> First and foremost, sorry for the big delay but I could not have a
> look at this PR earlier.  Nevertheless, I doubt that the decision of
> the new IPA-CP not to clone the function in question can be called a
> bug.  Yes, if the heuristics or other early optimizations results
> change, the cloning decision might change again in the future - even
> in between minor versions if we are really unlucky.

Can/do we mark all clones having hidden visibility?  Would a matching regexp
in the linker script override that?  Isn't that a bug?

[Bug libstdc++/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[jamborm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #14 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-02-20 12:57:01 UTC ---
(In reply to comment #13)
> Can/do we mark all clones having hidden visibility?  Would a matching regexp
> in the linker script override that?  Isn't that a bug?

I believe they are made static, at least TREE_PUBLIC is cleared for
their new decls in cgraph_create_virtual_clone.  I think the problem
was that the original un-cloned function was exported (because IPA-CP
decided to ignore it) rather than the cloned one?

[Bug libstdc++/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

--- Comment #15 from Rainer Orth <ro at gcc dot gnu.org> 2012-02-27 13:51:59 UTC ---
Author: ro
Date: Mon Feb 27 13:51:50 2012
New Revision: 184598

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184598
Log:
Fix Solaris symbol versioning (PR libstdc++/52188)

    contrib:
    PR libstdc++/52188
    * make_sunver.pl: Remove #ifdef handling.

    libgomp:
    PR libstdc++/52188
    * acinclude.m4 (LIBGOMP_ENABLE_SYMVERS): Remove    symvers_renaming.
    Remove ENABLE_SYMVERS_SOL2.
    * configure: Regenerate.
    * Makefile.am [LIBGOMP_BUILD_VERSIONED_SHLIB] (comma): New variable.
    (PREPROCESS): New variable.
    (libgomp.ver): New target.
    [LIBGOMP_BUILD_VERSIONED_SHLIB &&
    LIBGOMP_BUILD_VERSIONED_SHLIB_GNU]: Remove
    LIBGOMP_BUILD_VERSIONED_SHLIB_SOL2 handling.
    Use libgomp.ver.
    [LIBGOMP_BUILD_VERSIONED_SHLIB_SUN]: Use libgomp.ver, libgomp.ver-sun.
    * Makefile.in: Regenerate.

    libstdc++-v3:
    PR libstdc++/52188
    * acinclude.m4 (GLIBCXX_ENABLE_SYMVERS): Remove symvers_renaming.
    Remove ENABLE_SYMVERS_SOL2.
    * configure: Regenerate.
    * src/Makefile.am [ENABLE_SYMVERS] (libstdc++-symbols.ver):
    Postprocess mapfile.
    [ENABLE_SYMVERS_GNU]: Remove ENABLE_SYMVERS_SOL2 handling.
    * src/Makefile.in: Regenerate.

    * config/abi/pre/gnu.ver (GLIBCXX_3.4.5) [!__sun__ && !__svr4__]:
    Don't export
    _ZNSt19istreambuf_iteratorI[cw]St11char_traitsI[cw]EEppEv.

Modified:
    trunk/contrib/ChangeLog
    trunk/contrib/make_sunver.pl
    trunk/libgomp/ChangeLog
    trunk/libgomp/Makefile.am
    trunk/libgomp/Makefile.in
    trunk/libgomp/acinclude.m4
    trunk/libgomp/configure
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/acinclude.m4
    trunk/libstdc++-v3/config/abi/pre/gnu.ver
    trunk/libstdc++-v3/configure
    trunk/libstdc++-v3/src/Makefile.am
    trunk/libstdc++-v3/src/Makefile.in

[Bug libstdc++/52188] [4.7 regression] IPA-CP change broke libstdc++ symbol versioning on Solaris

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52188

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #16 from Rainer Orth <ro at gcc dot gnu.org> 2012-02-27 14:08:34 UTC ---
Fixed for 4.7.0.