public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
[not found] <bug-22592-183@http.gcc.gnu.org/bugzilla/>
@ 2006-06-14 0:32 ` jason at gcc dot gnu dot org
2006-06-20 18:28 ` jason at gcc dot gnu dot org
1 sibling, 0 replies; 11+ messages in thread
From: jason at gcc dot gnu dot org @ 2006-06-14 0:32 UTC (permalink / raw)
To: gcc-bugs
------- Comment #8 from jason at gcc dot gnu dot org 2006-06-13 23:28 -------
Either 20218 is a bug or this is. It seems to me that 20218 is the bug.
If you declare a function to be hidden, you are asserting that it will be
defined in the current DSO. From the GCC documentation:
"Two declarations of an object with hidden linkage refer to the same object
if they are in the same shared object."
Calling this function directly is a correct optimization, the bug is that you
fail to define it (by defining the key method) in the same DSO.
If this class is imported from a library, it shouldn't have hidden linkage; the
library's namespace should have explicit default linkage.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
[not found] <bug-22592-183@http.gcc.gnu.org/bugzilla/>
2006-06-14 0:32 ` [Bug c++/22592] -fvisibility-inlines-hidden broken differently jason at gcc dot gnu dot org
@ 2006-06-20 18:28 ` jason at gcc dot gnu dot org
1 sibling, 0 replies; 11+ messages in thread
From: jason at gcc dot gnu dot org @ 2006-06-20 18:28 UTC (permalink / raw)
To: gcc-bugs
------- Comment #9 from jason at gcc dot gnu dot org 2006-06-20 18:21 -------
Not a bug; see my earlier comment.
--
jason at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution| |INVALID
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
` (7 preceding siblings ...)
2005-09-05 7:35 ` matz at suse dot de
@ 2005-09-05 13:09 ` pluto at agmk dot net
8 siblings, 0 replies; 11+ messages in thread
From: pluto at agmk dot net @ 2005-09-05 13:09 UTC (permalink / raw)
To: gcc-bugs
--
What |Removed |Added
----------------------------------------------------------------------------
CC| |pluto at agmk dot net
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
` (6 preceding siblings ...)
2005-09-03 10:52 ` jh at suse dot cz
@ 2005-09-05 7:35 ` matz at suse dot de
2005-09-05 13:09 ` pluto at agmk dot net
8 siblings, 0 replies; 11+ messages in thread
From: matz at suse dot de @ 2005-09-05 7:35 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From matz at suse dot de 2005-09-05 07:35 -------
I want to add a comment to Mark's comment #5: I also thought about multiple
solutions to this problem, and in the end came to the conclusion that
disabling devirtualization in the current state of the compiler is the best.
My reasons for different options:
* don't support -fv-i-h anymore: that would trivially solve the problem,
but at the expense of deactivating a sometimes quite effective
optimization (especially with template heavy code, in the light of
non-class-member inlines), so I would like to retain the option itself
and support it as best as possible, even when this means to stretch
the C++ standard a bit in some corner, as this is just an option for
those who know what they are doing
* emit all referenced inlines in the compilation unit of reference,
as linkonce. This also would solve the problem of the direct call,
as now the implicit condition is met, that the function is indeed
forced local. But with the current infrastructure in 4.0 (and even 4.1),
this means that we would have to emit all _potentially_ referenced
inlines, because devirtualization can make random functions called
directly which weren't before. This would increase the memory
footprint of GCC again and as such is suboptimal.
* only localize (and hide) inline functions which are not class members.
This would solve this problem, because only class member inlines are
in danger of suddenly being called directly, when they formerly were
only called over the vtable. I haven't thought much about this solution,
but I think it's very feasible also. At least it should retain
most of the meaning of the -fv-i-h option, in that it reduces the
exporting of many out-of-class inline functions, particularly templates.
* disable devirtualization. Solves the problem, and has the least
effects regarding code quality. AFAIK currently devirtualization
is not used for very many interesting things. Inlining happens
before, so we don't get more inlining opportunities. Attributes
like constness are available before, so optimizations relying on them
are also possible on the indirect vtable-call. And what finally
convinced me was the fact that calls over the vtable are actually
faster than calls over the PLT. The call over PLT includes the call
to the PLT stub, the fetch of the address and the jump to the final
address (at first call also the symbol resolution). The vtable call
includes the fetch of the address and the call to it.
So all in all I think the best trade off is the last option. At the expense
of a missed optimization (which doesn't happen very often, as we can see
how often this bug happens, and if it happens then it's IMHO not very
effective currently) he get's a program working with this option, and with
the effects this option should have (less exported "fake" symbols).
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
` (5 preceding siblings ...)
2005-09-03 1:03 ` mmitchel at gcc dot gnu dot org
@ 2005-09-03 10:52 ` jh at suse dot cz
2005-09-05 7:35 ` matz at suse dot de
2005-09-05 13:09 ` pluto at agmk dot net
8 siblings, 0 replies; 11+ messages in thread
From: jh at suse dot cz @ 2005-09-03 10:52 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From jh at suse dot cz 2005-09-03 10:51 -------
Subject: Re: -fvisibility-inlines-hidden broken differently
>
> ------- Additional Comments From mmitchel at gcc dot gnu dot org 2005-09-03 01:03 -------
> Frankly, I think -fvisibility-inlines-hidden is a bad idea. I don't feel that
> way about -fvisibility, but giving inline functions special linkage in this way
> is a very fragile concept, and awards something with minimal C++ semantics
> ("inline") substantial observable semantics.
>
> The documentation for -fv-i-h is not clear about this case. All it really says
> is that any inlines emitted will be hidden. The compiler's meeting that
> requirement, trivially, because it's not emitting any inlines.
Is that even correct that we don't emit any inline? (ie see bellow)
>
> I don't think that we should do anything different in *this* source file because
> -fv-i-h is present. The thing we actually want to know is how the file
> containing the vtable is going to be compiled, and we can't know that here.
> Therefore, it seems to me that the mistake in Michael's chain of events is:
>
> 4) the call is emitted such that it expects a local (hidden) definition
> of the function in the DSO (i.e. not over PLT but direct call)
>
> Nothing in the semantics of -fv-i-h says that all inline functions in the
> program will be in this DSO. If we were to try to believe that, other things
> (like "extern template") would probably break too. Instead, the linker should
> have the responsibility of binding the relocation within the DSO at DSO
> link-time, if there happens to be a satisfying symbol at link time.
>
> Jan, I don't think your patch is correct, as I don't think anything about the
> optimization of this program should change based on -fv-i-h.
Hmm, OK, there are two problems I see.
One problem is that this testcase breaks. We can declare it invalid or
one other possibility I run across is to teach locally_bound predicate
in varasm to not believe that this hidden inline function is going to be
linked locally. This is very similar to way -fvisibility works - ie
function is hidden only if defined in current unit and would result in
DSO to build and even link if the other DSO will export the inline
function (that won't happen with -fv-i-h on the other DSO, right?)
This is not quite trivial, becuase -fv-i-h is hanled in the frontend and
backend already don't see whether the particular inline was actually
declared hidden (where user should provide it in within same DSO) or
whether the hidden implied this way, but I guess we can simply be
conservative here and when flag is active, thread this way all comdats
with inline flag and hidden visibility.
But the more general problem however is that I think it is quite
incorrect to call comdat linkonce function directly without actually
outputting it into current compilation unit, at least from the
definition of comdat flag:
/* Used in a DECL to indicate that, even if it TREE_PUBLIC, it need
not be put out unless it is needed in this translation unit.
Entities like this are shared across translation units (like weak
entities), but are guaranteed to be generated by any translation
unit that needs them, and therefore need not be put out anywhere
where they are not needed. DECL_COMDAT is just a hint to the
back-end; it is up to front-ends which set this flag to ensure
that there will never be any harm, other than bloat, in putting out
something which is DECL_COMDAT. */
Making direct call the function probably makes function "needed in this
translation unit" even tought it wasn't originally. This can happen
with and without the patch in this special case (COMDAT functio being
originally referenced by vtable not going to be output here). Is there
something behind scenes making this safe? (like is the argument that
the function will be provided by unit defining vtable anyway safe and if
so, why is not frontend emitting it as extern inline in all other units
saving object file sizes?)
If not, we probably should prevent the folding...
Honza
>
> --
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
>
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
` (4 preceding siblings ...)
2005-08-17 1:52 ` trapni at gentoo dot org
@ 2005-09-03 1:03 ` mmitchel at gcc dot gnu dot org
2005-09-03 10:52 ` jh at suse dot cz
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: mmitchel at gcc dot gnu dot org @ 2005-09-03 1:03 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From mmitchel at gcc dot gnu dot org 2005-09-03 01:03 -------
Frankly, I think -fvisibility-inlines-hidden is a bad idea. I don't feel that
way about -fvisibility, but giving inline functions special linkage in this way
is a very fragile concept, and awards something with minimal C++ semantics
("inline") substantial observable semantics.
The documentation for -fv-i-h is not clear about this case. All it really says
is that any inlines emitted will be hidden. The compiler's meeting that
requirement, trivially, because it's not emitting any inlines.
I don't think that we should do anything different in *this* source file because
-fv-i-h is present. The thing we actually want to know is how the file
containing the vtable is going to be compiled, and we can't know that here.
Therefore, it seems to me that the mistake in Michael's chain of events is:
4) the call is emitted such that it expects a local (hidden) definition
of the function in the DSO (i.e. not over PLT but direct call)
Nothing in the semantics of -fv-i-h says that all inline functions in the
program will be in this DSO. If we were to try to believe that, other things
(like "extern template") would probably break too. Instead, the linker should
have the responsibility of binding the relocation within the DSO at DSO
link-time, if there happens to be a satisfying symbol at link time.
Jan, I don't think your patch is correct, as I don't think anything about the
optimization of this program should change based on -fv-i-h.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
` (3 preceding siblings ...)
2005-07-27 13:56 ` matz at suse dot de
@ 2005-08-17 1:52 ` trapni at gentoo dot org
2005-09-03 1:03 ` mmitchel at gcc dot gnu dot org
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: trapni at gentoo dot org @ 2005-08-17 1:52 UTC (permalink / raw)
To: gcc-bugs
--
What |Removed |Added
----------------------------------------------------------------------------
CC| |trapni at gentoo dot org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
` (2 preceding siblings ...)
2005-07-26 20:54 ` pinskia at gcc dot gnu dot org
@ 2005-07-27 13:56 ` matz at suse dot de
2005-08-17 1:52 ` trapni at gentoo dot org
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: matz at suse dot de @ 2005-07-27 13:56 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From matz at suse dot de 2005-07-27 13:46 -------
Because these symbols indeed are not defined anywhere. On linux this happens
to work, but on darwin you need to link against something which provides them.
So you would need to create a library which implements both operators
out-of-line (and hence also the vtable), and us that to link _this_ library
against.
But that's not the issue at hand.
This is the bigger picture, how this error can be seen in the real world:
First, there is a base library (let's call it libbase) implementing the
whole class, all methods, the vtable, everything.
Then there's another library (libtwo), using that class in implementing some
of it's functionality (breakme in our case). It does so by including the
header for that class (defining the inline operator !=, besides declaring
the class), and linking against libbase. Hence no unresolved symbols will
occur.
The libtwo exports only those symbols and class it wants exported, hence
it switches the default visibility to hidden (including inlines), because all
these are already defined in libbase, no need to export them too.
This is all perfectly valid usage of shared libs. But it doesn't work because
libtwo can't be created due to the invalid call emitted to a method not
defined in the same DSO.
Perhaps I should have made more clear the bigger picture to not sidetrack
others by the undefinedness of operator==. In the real world it _will_ be
defined, in a different shared lib. So just for reference a little bit
reformatted:
--------------- libbase.ii --------------------
struct A {
virtual bool operator== (const A &a) const;
virtual bool operator!= (const A &a) const;
};
inline bool A::operator!= ( const A &a) const
{ return !(*this == a); }
bool A::operator== (const A&a) const
{ return true; }
-----------------------------------------------
Compile this with just "g++ -fPIC -shared -o libbase.so libbase.ii", and
you have a shared lib you can use to link against when creating the
second shared lib, from the source of the initial report here. Note that
the first few lines (including definition of operator !=) reflect a
header file which declares class A, which is included in libbase.ii and
testcase.ii.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
2005-07-21 16:44 ` [Bug c++/22592] " pinskia at gcc dot gnu dot org
2005-07-22 12:53 ` matz at suse dot de
@ 2005-07-26 20:54 ` pinskia at gcc dot gnu dot org
2005-07-27 13:56 ` matz at suse dot de
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu dot org @ 2005-07-26 20:54 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From pinskia at gcc dot gnu dot org 2005-07-26 20:51 -------
I still don't think this is a bug as if I compile the library on ppc-darwin, we get the following link error
even without -fvisibility=hidden/-fvisibility-inlines-hidden:
ld: Undefined symbols:
__ZNK1AneERKS_
__ZTV1A
this is at -O2.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
2005-07-21 16:44 ` [Bug c++/22592] " pinskia at gcc dot gnu dot org
@ 2005-07-22 12:53 ` matz at suse dot de
2005-07-26 20:54 ` pinskia at gcc dot gnu dot org
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: matz at suse dot de @ 2005-07-22 12:53 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From matz at suse dot de 2005-07-22 12:46 -------
I don't understand. The code itself is perfectly valid C++, I don't think
you mean that it's invalid, right? Yes, operator== is also hidden, but
there is no definition for it in this unit, hence GCC generates the correct
call type (over PLT). (It should also be noted that because of the other bugs
GCC can't emit the .hidden directives for undefined symbols, except when
using HJs patches, but that's tangential and wouldn't make a difference,
the crucial point is, that the correct call is emitted).
And irrespective of that the error also happens without -fvisibility=hidden,
i.e. when _only_ the inlines are hidden. I still think this is a bug,
which should be corrected by making GCC just emit an out-of-line copy of
the inline function (in a linkonce section).
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug c++/22592] -fvisibility-inlines-hidden broken differently
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
@ 2005-07-21 16:44 ` pinskia at gcc dot gnu dot org
2005-07-22 12:53 ` matz at suse dot de
` (7 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu dot org @ 2005-07-21 16:44 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From pinskia at gcc dot gnu dot org 2005-07-21 16:39 -------
No I think this code is in fact invalid and should error out like this. Note you also declared operator==
as being hidden too. So if you call that, it would break too.
--
What |Removed |Added
----------------------------------------------------------------------------
Component|middle-end |c++
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22592
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2006-06-20 18:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <bug-22592-183@http.gcc.gnu.org/bugzilla/>
2006-06-14 0:32 ` [Bug c++/22592] -fvisibility-inlines-hidden broken differently jason at gcc dot gnu dot org
2006-06-20 18:28 ` jason at gcc dot gnu dot org
2005-07-21 16:20 [Bug middle-end/22592] New: " matz at suse dot de
2005-07-21 16:44 ` [Bug c++/22592] " pinskia at gcc dot gnu dot org
2005-07-22 12:53 ` matz at suse dot de
2005-07-26 20:54 ` pinskia at gcc dot gnu dot org
2005-07-27 13:56 ` matz at suse dot de
2005-08-17 1:52 ` trapni at gentoo dot org
2005-09-03 1:03 ` mmitchel at gcc dot gnu dot org
2005-09-03 10:52 ` jh at suse dot cz
2005-09-05 7:35 ` matz at suse dot de
2005-09-05 13:09 ` pluto at agmk dot net
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).