PATCH: Fix linkage in libsupc++

[Gabriel Dos Reis]

Mark Mitchell <mark@codesourcery.com> writes:

| Gabriel Dos Reis wrote:
| 
| >Mark Mitchell <mark@codesourcery.com> writes:
| >
| >| With the new symbol visibility patches, we have to be careful not to
| >| get incorrect linkage in the libraries; if the default visibility is
| >| not "default" when we include the headers, things get confused.
| >| Things are even worse for ports that use "hidden" visibility by
| >| default; the ARM/Symbian OS port will be one such.
| > | | A tricky problem here was that this pattern was present in a lot
| > of
| >| the libsupc++ files:
| > | |   // cxxabi.h
| > | |   namespace __cxxabiv1 { |      extern "C" void f();
| >|   }
| > | |   // f.c
| > | |   #include <cxxabi.h>
| > |   |   extern "C" void f() {}
| > | | This is *not* a definition of the function declared in the
| > header;
| >| it's just an overloaded function.  You must write "__cxxabiv1::f" or
| >| put the definition inside the namespace.
| >
| >If the "visibility" patch does something like that, then the
| >
| It's not the visibility patch that causes this; these declarations
| were never being matched up before, either.  It just didn't matter
| before.

I think they did matter and we got many "ambiguous" declarations bug
report in the past.  At some point, someone (Roger, when working on
built-ins?) checked in a patch; I thought that patch addressed the
issue, but apparently it did not.

| I have looked at this part of the standard before, and despite the
| language that you cite, I am not convinced that in my example above
| the compiler should check that the two declarations of "f" match.  For
| example, if the first were "void f(int)" must the compiler issue an
| error?  I think that program is ill-formed, because of the language
| you cite, but I see nothing that would cause name lookup for the
| definition to find the version in the namespace.
| 
| If the committee disagrees with me, then we have a compiler bug.

I think we have a compiler bug.  But a definitive word from the
committe cannot hurt. Do you want me to raise this on -core? 

| Still, the change I made to the library is at worst harmless.

I agree with the explicit setting of the visibility attribute that
your patch made.
I'm just very worried about the overload bits.

| >  At most one function with a particular name can have C language
| >  linkage. Two declarations for a function with C language linkage
| >  with the same function name (ignoring the namespace names that
| >  qualify it) that appear in different namespace scopes refer to the
| >  same function.  | I did not attempt to fix all of the V3 headers;
| > I'm only concerned
| >| with libsupc++ at the moment.  However, similar changes should
| >| probably be made throughout V3.  Otherwise, linkage will be wrong if
| >| people #include (say) <iostream> with a non-default symbol visibility.
| >
| >I'm not convinced by your example and the rationale you gave.  What am
| >I missing?  Most certainly, in your example ::f and __cxxabiv1::f
| >refer to the same function and we should have &::f == &__cxxabiv1::f.
| >
| That's a separate issue.  You still want the visibility #pragmas in
| the V3 headers; otherwise,  for example, copies of the type info for
| iostream in user programs may not be merged with that in the library,
| if the user does:
| 
|   #pragma GCC visibility push(hidden)
|   #include <iostream>

As I said in the previous message, I agree with your patch setting
explicitly the visibility.  
My disagreement is about the "overload" bits.


-- Gaby