[cfe-dev] Clang emits unreachable `internal linkage` constructor

[John McCall via cfe-dev]

> On Oct 20, 2016, at 11:39 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>> On Oct 20, 2016, at 10:35 AM, John McCall <rjmccall at apple.com> wrote:
>> 
>>> On Oct 13, 2016, at 10:46 PM, Mehdi Amini via cfe-dev <cfe-dev at lists.llvm.org> wrote:
>>> Hi,
>>> 
>>> This fairly simple (and valid) code does not link (on MacOS):
>>> 
>>> 
>>> #include <iostream>
>>> #include <string>
>>> namespace {
>>> 	struct VA {
>>> 	};
>>> 	struct A : virtual public VA {
>>> 		A() {
>>> 			static bool b = false;
>>> 			std::string str;
>>> 		}
>>> 	};
>>> }
>>> int main(int argc, char* argv[]) {
>>> 	A::A a;
>>> }
>>> 
>>> 
>>> 
>>> The issue is that clang emits two constructors (the complete one and the base one) for struct A because it has a virtual base class.
>>> 
>>> Because struct A is declared in an anonymous namespace, these constructors are internal linkage. Only one of them is actually called, the other is unreachable.
>>> 
>>> The “always-inliner” does not visit unreachable internal function. However this constructor is calling into a function from libc++ that are marked “available_externally” and “always inline”: the basic_string constructor.
>>> 
>>> The problem is that the linkage “available_externally" is a “lie”: there is an external template instantiation in the header, but it is marked “visibility hidden”. So the function cannot be linked to in the libc++ dylib. (I think it’d work with a static libc++).
>> 
>> Is it actually hidden in the library, or is it in practice exported?  That is, is the link failure specifically because the external definition is not defined within the current linkage unit (and thus we emit a code sequence that cannot be used to call an external symbol), or is because the symbol isn't available at all?
>> 
>> Assuming the latter, then maybe the right answer is that libc++ should only be declaring an external instantiation of the members it actually exports.  That would be the most explicit thing, because arguably what libc++ is currently doing is a lie.  Trying to work around that problem in the compiler by recognizing certain members as not "really" being externally available seems doomed to failure.
> 
> 
> Right: that’s why I suggested going through a base class that would be externally instantiated, and a derived one for the specific member intended to be hidden.
> That seems like the most “clear” (and maintainable) way to achieve it, but there might be drawbacks (or alternative) that I missed.

Well, I don't know about "maintainable".  The set of members that are guaranteed to be explicitly instantiated is fixed.  If a future version of the standard adds members to basic_string, those members will not be explicitly instantiated in existing versions of the library, and that's something the compiler needs to know about.  If you want to expand the set of explicit instantiations, you'll need to gate those explicit instantiation declarations on the target deployment version.

Enumerating all the methods that are instantiated is probably *obnoxious*, but it's the most correct thing to do.

John.

> 
> — 
> Mehdi
> 
> 
> 
>> 
>> John.
>> 
>>> 
>>> I see multiple ways to address such cases, possibly many of them can be implemented:
>>> 
>>> - Don’t emit unreferenced internal_linkage function. (We should try to do this anyway right?)
>>> - Have the CGSCC run on the unreachable part of the call-graph.
>>> - Run global-DCE at O0 (might be a good idea anyway if it speeds-up the build)
>>> - We should have the “available_externally” compatible with “hidden visibility”. Maybe we need a way to have to mark some method excluded from one  `external template` instantiation so that it would be ODR.
>>> - We could make the basic_string constructor and other method in the same situation internal linkage instead.
>>> 
>>> 
>>> Thoughts?
>>> 
>>> — 
>>> Mehdi
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> 
>