Bug 24660 - versioning weak symbols in libstdc++
versioning weak symbols in libstdc++
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: libstdc++
4.1.0
: P3 enhancement
: ---
Assigned To: Benjamin Kosnik
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-03 19:06 UTC by Benjamin Kosnik
Modified: 2006-08-22 14:05 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-12-25 01:17:16


Attachments
libstdc++ patch (15.41 KB, patch)
2005-11-03 19:07 UTC, Benjamin Kosnik
Details | Diff
mainline chokes on this (100.66 KB, application/x-bzip2)
2005-11-03 19:08 UTC, Benjamin Kosnik
Details
mainline chokes on this (100.66 KB, application/x-bzip2)
2005-11-03 19:09 UTC, Benjamin Kosnik
Details
mainline output (4.70 KB, text/plain)
2005-11-03 19:10 UTC, Benjamin Kosnik
Details
Kona paper, as revised but not in post-Kona mailing (12.66 KB, text/plain)
2005-11-17 21:53 UTC, Benjamin Kosnik
Details
Outline for Kona presentation (2.96 KB, text/plain)
2005-11-17 21:54 UTC, Benjamin Kosnik
Details
Patch to require nesting (1.83 KB, patch)
2005-11-18 18:34 UTC, Jason Merrill
Details | Diff
test case for first fail (402 bytes, text/plain)
2005-12-13 20:38 UTC, Benjamin Kosnik
Details
test case for second fail (693 bytes, text/plain)
2005-12-13 20:39 UTC, Benjamin Kosnik
Details
libstdc++ patch v5 (25.78 KB, patch)
2005-12-14 07:23 UTC, Benjamin Kosnik
Details | Diff
libsupc++ issue 1 (14.03 KB, application/x-bzip2)
2005-12-18 01:50 UTC, Benjamin Kosnik
Details
libsupc++ issue 2 (1.18 KB, application/x-bzip2)
2005-12-18 01:50 UTC, Benjamin Kosnik
Details
libsupc++ issue 3 (7.21 KB, application/x-bzip2)
2005-12-18 01:51 UTC, Benjamin Kosnik
Details
files from kona meeting (27.42 KB, application/x-bzip2)
2005-12-30 03:48 UTC, Benjamin Kosnik
Details
mail surrounding namespace association development (404.07 KB, text/plain)
2005-12-30 03:49 UTC, Benjamin Kosnik
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Kosnik 2005-11-03 19:06:47 UTC
This is an experiment in versioning the vague parts of libstdc++. It is apparent that some kind of control over weak symbols is necessary for accurate versioning of C++ constructs WRT shared libraries.

To that end, the obvious point of influence is the namespace name itself, since it is part of the mangled symbol name. 

When trying to version with namespace names, there are a couple of problems. See Sutter, N1344 for more details and a general problem statement. There appear to be two general solutions.

One, relax specialization rules WRT declared namespace. Motivations for the current rules appear somewhat vague, but are best described here:

http://gcc.gnu.org/ml/libstdc++/2005-09/msg00081.html

Two, invent a mechanism to allow lookup and specialization. This was done for g++ as part of the debug mode work, and is called namespace association.

This patch uses approach two. This was anticipated and discussed as part of the Kona discussion, and was also subject of a paper at last year's GCC summit.

There are a couple of outstanding issues with this patch.

1) --disable-symvers for the time being

2) ext, "C" headers, and libsupc++ bits need to be thought through

3) longstanding issues with things like std::swap, see other PR's

4) new issues with mainline. See attached for problem statement.

This bug report is really just a convenience for me.
Comment 1 Benjamin Kosnik 2005-11-03 19:07:30 UTC
Created attachment 10130 [details]
libstdc++ patch
Comment 2 Benjamin Kosnik 2005-11-03 19:08:30 UTC
Created attachment 10131 [details]
mainline chokes on this
Comment 3 Benjamin Kosnik 2005-11-03 19:09:13 UTC
Created attachment 10132 [details]
mainline chokes on this


Will attach the compiler output as a separate piece. It looks recent, as gcc-3.4.x, gcc-4.0.x are ok.
Comment 4 Benjamin Kosnik 2005-11-03 19:10:51 UTC
Created attachment 10133 [details]
mainline output
Comment 5 Andrew Pinski 2005-11-03 19:25:12 UTC
Isn't in a way strong using what you want here?
Comment 6 Benjamin Kosnik 2005-11-03 19:28:38 UTC
namespace association is the correct name for "strong using."

Indeed, you'll find that this is what I am using. 

-benjamin
Comment 7 Jason Merrill 2005-11-17 18:57:17 UTC
Here's a reduced testcase:

namespace N { }
namespace std
{
  using namespace N __attribute__ ((strong));
}
namespace N
{
  using namespace std;
  template<typename T>
  struct A
  {
    void f();
  };
}
namespace std
{
  template<>
  struct A<int>
  {
    void f() { }
  };
}

This code, like the testcase for c++/16021, works fine if the implementation namespace is nested within std.  I'm thinking of enforcing that requirement in parse_using_directive; that would allow us to avoid adding more special lookup rules.  Do you see any problem with that requirement?
Comment 8 Jason Merrill 2005-11-17 19:01:51 UTC
The nesting also means you don't need the reciprocal using-directive which you added to avoid the problem from 16021.
Comment 9 Gabriel Dos Reis 2005-11-17 20:46:09 UTC
Subject: Re:  versioning weak symbols in libstdc++

"jason at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| This code, like the testcase for c++/16021, works fine if the implementation
| namespace is nested within std.  I'm thinking of enforcing that
| requirement in parse_using_directive; that would allow us to avoid
| adding more special lookup rules.  Do you see any problem with that
| requirement? 

I'm for minimal invasion in the name lookup rules; so if that takes
care of issues raised by Benjamin, I'm all for it.

-- Gaby
Comment 10 Benjamin Kosnik 2005-11-17 21:53:40 UTC
Created attachment 10271 [details]
Kona paper, as revised but not in post-Kona mailing
Comment 11 Benjamin Kosnik 2005-11-17 21:54:22 UTC
Created attachment 10272 [details]
Outline for Kona presentation


I cannot believe I found this stuff.
Comment 12 Benjamin Kosnik 2005-11-17 22:15:28 UTC
Here you go Jason. Actually, it looks like I have a complete archive of most parts of this discussion, the related material, code samples, issues, etc. If you would like, I will tar it up and put it in this pr. Or, if you are interested, I can send it to you (or others) via private email. 

I should have just put this stuff in post-Kona: sorry. I need to get better about this kind of thing.

Anyway. Jason, I think making the namespace nested is fine. Interestingly, I reviewed these discussions, and it looks like at the time of the debug mode design discussion, we were wondering about the pros and cons of nested vs. separate namespaces.

Looks like now we know more about these tradeoffs.

The only drawback that I can see is trying to reduce the size penalty for mangled names. This is going to be an issue for some people, and we might as well admit it from the start. 

I'd thought of nested solutions, including the obvious ones:

namespace _6 {
namespace std {

namespace std {
namespace _6 {

but the problem is then that all the specialized compression for narrow std:: types (basic_string, basic_istream, etc) will fall down, and all your names go through two namespace manglings. I'm not convinced this is such a bad thing, actually: having non-compressed std:: would mean post-processing of library binaries with other tools will become much more regular, without substitutions. 

Of course, 

namespace _6 {

also removes the specialized namespace std:: compression, but the fully qualified name is still within reasonable limits of the current status quo. This was judged to be the least obese of all options, in terms of symbol size explosion. (ie _ZNS vs. _ZN2_6).

I must tell you that I would rather have this feature (and a resolution to the weak symbol vs. shared object problem), and loose compression, than have either the benefits of compression or a shorter fully qualified name. If I had a magic wand, of course, I'd pick compression on generic template type name, not instance name, so that wchar_t and char types would also be compressed 
(ie compress basic_string, not basic_string<char>), plus namespace association. That might be a way to win back some of the loss if we switch to something like std::_6 for the namespace.

Anyway.

Comment 13 Jason Merrill 2005-11-17 22:39:02 UTC
Subject: Re:  versioning weak symbols in libstdc++

I think nesting _6 within std makes sense in the abstract as well, as it 
is properly part of the standard library space, not a separate entity.

The debug mode headers will need to be modified to work with this new 
requirement by moving __gnu_debug_def into std as well.  Do you plan to 
version the debug mode containers, too?

I don't think there's much compression to be done once you start 
inserting arbitrary namespaces.  Just compressing the NTBS 
"basic_string" doesn't get you much to compensate for the added complexity.

You might as well attach the tarball to the PR for posterity.  I'm 
particularly interested in the discussions you mention about the 
relative merits of nesting or not.  I note that the documentation of the 
feature uses a nested implementation namespace, so that seems to be what 
I had in mind...

Jason
Comment 14 Benjamin Kosnik 2005-11-17 23:41:08 UTC
> I think nesting _6 within std makes sense in the abstract as well, as it 
> is properly part of the standard library space, not a separate entity.

Sounds sane to me.

> The debug mode headers will need to be modified to work with this new 
> requirement by moving __gnu_debug_def into std as well.  

Yep. Current debug mode will have to be adjusted. If you put your patch in for the compiler bits to this bug report I can do the debug mode adjustments.

> Do you plan to version the debug mode containers, too?

Don't know. I've not thought that much about it, simply because we don't worry that much about debug mode and versioning, since it is not on by default. I think there are only 4 or so exports for debug mode. 

I suppose it should be.

I am thinking all things with templates should be versioned. That would mean debug, tr1, ext headers, as well as std, and some of the C++ versions of "C" headers (cmath, for instance, so we'd just do all the "C" headers as well.) 

> I don't think there's much compression to be done once you start 
> inserting arbitrary namespaces.  Just compressing the NTBS 
> "basic_string" doesn't get you much to compensate for the added complexity.

Sure, sure. I think we are on the same page here, actually. 
Comment 15 Jason Merrill 2005-11-18 18:34:48 UTC
Created attachment 10275 [details]
Patch to require nesting

I've attached my compiler patch to enforce this rule.  It can't go in until the debug mode code is fixed.
Comment 16 Benjamin Kosnik 2005-12-13 20:27:51 UTC
Two extra fails with both versioning and debug associations active.

FAIL: 20_util/memory/16505.cc (test for excess errors)
FAIL: 25_algorithms/search_n/11400.cc (test for excess errors)

For the first:

#include <memory>

// libstdc++/16505

struct S { };

template
  void
  std::uninitialized_fill_n<S*, int, S>(S*, int, const S&);

gives:
/mnt/hd/src/gcc.versioned/libstdc++-v3/testsuite/20_util/memory/16505.cc:31: error: 'void std::uninitialized_fill_n(S*, int, const S&)' should have been declared inside 'std'


and for the second:
#include <algorithm>
#include <functional>

struct Integral { operator int() const; };

namespace std
{
  template int* search_n (int*, int*, Integral, const int&);
  template int* search_n (int*, int*, Integral, const int&, greater<int>);
}


gives:
/mnt/hd/src/gcc.versioned/libstdc++-v3/testsuite/25_algorithms/search_n/11400.cc:30: error: 'search_n' is not a template function

Still testing out the other configs. I'll put reduced test cases in bugzilla.
Comment 17 Benjamin Kosnik 2005-12-13 20:38:52 UTC
Created attachment 10475 [details]
test case for first fail
Comment 18 Benjamin Kosnik 2005-12-13 20:39:22 UTC
Created attachment 10476 [details]
test case for second fail
Comment 19 Benjamin Kosnik 2005-12-14 07:23:54 UTC
Created attachment 10483 [details]
libstdc++ patch v5


Config not quite 100% correct, but good enough.

This moves all debug mode stuff to nested within std. In addition, there is a version mode too, which versions all of namespace std. 

To get to the versioned mode, configure with --enable-symvers=gnu-versioned-namespaces

In addition, the generated c++config.h has to uncomment:

// #define _GLIBCXX_NAMESPACE_ASSOCIATION_VERSION 1

Without versioned mode, there are no new fails. I'm still testing with the debug mode turned on.

With versioned mode, there are two new fails. 

Still, not so bad, not so bad. 


Kind of a mouthful at the moment.
Comment 20 Benjamin Kosnik 2005-12-14 09:12:56 UTC
This arrangement of debug mode does indeed seem to fix the longstanding swap issue. 

ie, -D_GLIBCXX_DEBUG runs are == normal runs.

Comment 21 Paolo Carlini 2005-12-14 09:48:02 UTC
(In reply to comment #20)
> This arrangement of debug mode does indeed seem to fix the longstanding swap
> issue. 
> 
> ie, -D_GLIBCXX_DEBUG runs are == normal runs.

Yeah! (about the rest of the work too ;)
Comment 22 Benjamin Kosnik 2005-12-17 09:15:14 UTC
This patch is complete with one exception, libsupc++. It looks like --disable-hosted-libstdcxx is broken right now, so I have to fix that and then make it work with this scheme.

http://people.redhat.com/bkoz/libstdc++-v3.versioned-7.tar.bz2

By default, the behavior of this patch is compatible with gcc-3.4/gcc-4.0/gcc-4.1. I remapped the new, nested symbols to the old exports. 

In addition, with --enable-symvers=gnu-versioned-namespaces, the following namespaces are versioned: std, __gnu_cxx, std::tr1. Ie, these namespaces are all  associated with nested namespaces of the type _6.

There are a lot of renames in this, so I just put up all the sources.
Comment 23 Benjamin Kosnik 2005-12-18 01:48:44 UTC
When doing the last bit, I ran into some other issues, where some of the libsupc++ bits (typeinfo, unexpected_handler, uncaught_exception, nothrow etc) are internally defined within std. 
(? Or seem to be.)

This hardcoding should be adjusted.

Here are three pre-processed examples.

Comment 24 Benjamin Kosnik 2005-12-18 01:50:14 UTC
Created attachment 10524 [details]
libsupc++ issue 1
Comment 25 Benjamin Kosnik 2005-12-18 01:50:44 UTC
Created attachment 10525 [details]
libsupc++ issue 2
Comment 26 Benjamin Kosnik 2005-12-18 01:51:05 UTC
Created attachment 10526 [details]
libsupc++ issue 3
Comment 27 Benjamin Kosnik 2005-12-30 03:48:43 UTC
Created attachment 10567 [details]
files from kona meeting
Comment 28 Benjamin Kosnik 2005-12-30 03:49:26 UTC
Created attachment 10568 [details]
mail surrounding namespace association development
Comment 29 Jason Merrill 2006-01-12 20:10:51 UTC
Subject: Bug 24660

Author: jason
Date: Thu Jan 12 20:10:47 2006
New Revision: 109647

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=109647
Log:
        PR libstdc++/24660
        * pt.c (check_explicit_specialization): Handle namespace
        association.
        * name-lookup.c (set_decl_namespace): Likewise.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/name-lookup.c
    trunk/gcc/cp/pt.c
    trunk/gcc/doc/extend.texi

Comment 30 Benjamin Kosnik 2006-01-16 20:09:58 UTC
Confirmed. Excellent, thanks. This is great, no regressions known with this on.



Comment 31 Benjamin Kosnik 2006-08-22 14:05:21 UTC
This experiment is deemed sucessful.