This is another instance of the visibility stuff being broken. This time because a function isn't emitted, but called in a way as if it was. This code: --------------------------------- 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 breakme(const A& newPath, A &b) { return A(newPath) != b; } --------------------------------- Compile with (4.0.2 pre): % g++ -fPIC -DPIC -I/usr/lib/qt3/include -fvisibility=hidden \ -fvisibility-inlines-hidden -Wall -O2 -o libbla.so -shared testcase.ii ld: /tmp/ccWAn1go.o: relocation R_X86_64_PC32 against `A::operator!=(A const&) const' can not be used when making a shared object; recompile with -fPIC The chain of events goes like so: 1) the operator is virtual, hence initially the call is through the vtable 2) inlining happens before that is optimized, hence the operator call is not inlined 3) the virtual call is optimized to a direct call of the operator!=, because of the explicit copy operation GCC knows the final type of the object on which it is called (namely "A"), and hence can transform the indirect to a direct call 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) 5) But no copy of that function is emitted in this DSO, because the out-of-line copies of such inline function are generated only in the .o file which contains the key method of the class (the first virtual func in this case). So nothing can be made hidden, and it anyway wouldn't work even if, as there simply is nothing to call in this DSO. I believe this is a different error from all the other visibility problems like bug 19664 or bug 20218. I don't know what GCC should do here. It either needs to emit an out-of-line copy of this operator, or generate the call over a PLT. The first solution would be better, but could mean that we need to emit such out-of-line copies in every .o file where they are referenced. The second solution might even not work at all, as probably the main library (containing the key method and hence the out-of-line versions) is also compiled with -fvisibility-inlines-hidden, and hence wouldn't even export those function which a call over PLT could resolve to. So, it seems that cgraph needs to be changed somehow to emit this function if referenced. Hence CCing Honza.
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.
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).
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.
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.
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.
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.
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).
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.
Not a bug; see my earlier comment.