<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Jul 19, 2018 at 10:16 AM Louis Dionne via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><br><div><br><blockquote type="cite"><div>On Jul 19, 2018, at 09:30, Louis Dionne <<a href="mailto:ldionne@apple.com" target="_blank">ldionne@apple.com</a>> wrote:</div><br class="m_-3601286788530542602Apple-interchange-newline"><div><br class="m_-3601286788530542602Apple-interchange-newline"><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><div>On Jul 19, 2018, at 04:24, Eric Fiselier <<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>> wrote:</div><br class="m_-3601286788530542602Apple-interchange-newline"><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 18, 2018 at 11:53 AM John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><blockquote type="cite"><div>On Jul 18, 2018, at 5:40 AM, Eric Fiselier <<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>> wrote:</div><div><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Jul 18, 2018 at 1:30 AM John McCall via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><blockquote type="cite"><div>On Jul 11, 2018, at 5:29 PM, Louis Dionne via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:</div><br class="m_-3601286788530542602m_3773437658758568607m_1381249095906822725Apple-interchange-newline"><div><div style="word-wrap:break-word;line-break:after-white-space"><div>- llvm-dev (sorry Chandler, I’m not accustomed to which topics should be discussed on which lists yet)</div><br><div><blockquote type="cite"><div>On Jul 11, 2018, at 06:37, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> wrote:</div><br class="m_-3601286788530542602m_3773437658758568607m_1381249095906822725Apple-interchange-newline"><div><div dir="ltr">This is probably much more of a question for the Clang list....<br><br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 10, 2018 at 6:21 PM Hubert Tong via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jul 10, 2018 at 7:20 PM, Louis Dionne via llvm-dev<span class="m_-3601286788530542602Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span><span class="m_-3601286788530542602Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Hi,<br><br>While investigating the situation of visibility annotations and linkage in libc++ with the goal of removing uses of `__always_inline__`, Eric Fiselier and I stumbled upon the attached test case, which I don't think Clang compiles properly. Here's the gist of the test case, reduced to the important parts (see the attachment if you want to repro):<br><br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>// RUN: %cxx -shared -o %T/libtest.so %flags %compile_flags -fPIC %s<br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>// RUN: %cxx -c -o %T/main.o %flags %compile_flags %s -O2 -DBUILDING_MAIN<br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>// RUN: %cxx -o %T/test.exe -L%T/ -Wl,-rpath,%T/ -ltest %T/main.o<br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>// RUN: %T/test.exe<br><br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>template <class T><br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>struct Foo {<br>       <span class="m_-3601286788530542602Apple-converted-space"> </span>inline __attribute__((visibility("hidden")))<br>       <span class="m_-3601286788530542602Apple-converted-space"> </span>int __init(int x) { /* LOTS OF CODE */ }<br><br>       <span class="m_-3601286788530542602Apple-converted-space"> </span>inline __attribute__((visibility("default")))<br>       <span class="m_-3601286788530542602Apple-converted-space"> </span>int bar(int x) { return __init(x); }<br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>};<br><br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>extern template struct Foo<int>;<br><br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>#ifdef BUILDING_MAIN<br>       <span class="m_-3601286788530542602Apple-converted-space"> </span>int main() {<br>           <span class="m_-3601286788530542602Apple-converted-space"> </span>Foo<int> f;<br>           <span class="m_-3601286788530542602Apple-converted-space"> </span>f.bar(101);<br>       <span class="m_-3601286788530542602Apple-converted-space"> </span>}<br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>#else<br>       <span class="m_-3601286788530542602Apple-converted-space"> </span>template struct Foo<int>;<br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>#endif<br><br>When running the attached file in `lit`, we get:<br><br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>Undefined symbols for architecture x86_64:<br>     <span class="m_-3601286788530542602Apple-converted-space"> </span>"Foo<int>::__init(int)", referenced from:<br>         <span class="m_-3601286788530542602Apple-converted-space"> </span>_main in main.o<br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>ld: symbol(s) not found for architecture x86_64<br><br>The idea here is that `__init` is a pretty big function, and we're promising that an external definition of it is available through the use of the extern template declaration. With the appropriate optimization level (O2 and above), LLVM decides not to include the definition of `__init` in the executable and to use the one available externally. Unfortunately, `__init` has hidden visibility, and so the definition in the .so is not visible to the executable, and the link fails.<br><br>Where I think Clang/LLVM goes wrong is when it decides to remove the instantiation in the executable in favor of the one that is hypothetically provided externally. Indeed, the Standard says in [temp.explicit]/12 (<a href="http://eel.is/c++draft/temp.explicit#12" rel="noreferrer" target="_blank">http://eel.is/c++draft/temp.explicit#12</a>):<br><br>   <span class="m_-3601286788530542602Apple-converted-space"> </span>Except for inline functions and variables, declarations with types deduced from their initializer or return value ([dcl.spec.auto]), const variables of literal types, variables of reference types, and class template specializations, explicit instantiation declarations have the effect of suppressing the implicit instantiation of the definition of the entity to which they refer. [ Note: The intent is that an inline function that is the subject of an explicit instantiation declaration will still be implicitly instantiated when odr-used so that the body can be considered for inlining, but that no out-of-line copy of the inline function would be generated in the translation unit. — end note ]<br><br>Only reading the normative wording, it seems like LLVM should leave the instantiation there because it can't actually assume that there will be a definition provided elsewhere (yes, despite the extern template declaration, because the function is inline). Then, the non-normative note seems to be approving of what LLVM is doing, but I'm wondering whether that's really the intended behavior.<br><br>Questions:<br>1. Is what LLVM's doing there legal?<br></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>The Standard does not say anything about the instantiation producing a definition associated with object file that results from translating the current translation unit. The program is only valid if there is indeed an explicit instantiation provided elsewhere. That said, the as-if rule that allows the suppression of the definition assumes that a provided explicit instantiation can be linked against. It is up the the designers of the extension (the visibility attribute) to say whether or not the rule with templates having hidden visibility is that the explicit instantiation needs to be provided in the same “module".<br></div></div></div></div></blockquote></div></div></div></blockquote><div><br></div><div>Thanks Hubert. That makes some amount of sense. So in that case, it would either be</div><div>1. A bug that Clang is not emitting `__init` in the object file since it has hidden visibility, and it can’t tell whether we’re going to try to link dynamically or statically with the other module that provides it.</div><div>2. The intended design that visibility attributes interact very poorly with extern template declarations. This would be sad, since libc++ is a pretty big user of both, and it really isn’t working that well for us.</div></div></div></div></blockquote></div><div><blockquote type="cite"><div><div style="word-wrap:break-word;line-break:after-white-space"><div><div><br></div><div>In both cases, it’s not a conformance bug with the Standard, as Hubert points out. It would be lovely if someone more knowledgeable about the visibility attributes could shed some light on whether that’s intended and we need to invent something new, or whether it is correct to consider it a bug and we should fix it (somehow).</div></div></div></div></blockquote><div><br></div>libc++ should declare explicit instantiations of just the functions it actually intends to provide in the library instead of declaring an instantiation of the entire class-template and then trying to retroactively patch over the problem with a mess of attributes on every declaration.  You should be able to then just give the templates type_visibility("default") and visibility("hidden").  You can put visibility("default") directly on the instantiation and it should override any attributes from the template.</div></div></blockquote><div><br></div><div>That's a very good point, somehow I hadn't thought of it before.</div><div><br></div><div>One problem is that attributes like `internal_linkage` need to appear on the first declaration, and can't be added later when declaring an instantiation.</div><div>Though I'm not sure if that restriction is artificial, at least in  this particular case.</div></div></div></div></blockquote><div><br></div>Well, I can't imagine why you would declare an internal-linkage explicit instantiation, but if you have a need to, I don't see any reason we couldn't allow that.</div></div></blockquote><div><br></div><div>Sorry. I don't want to declare an explicit instantiation as having internal linkage. But I might want to declare everything except for an explicit instantiation as having internal linkage.</div><div>But since the attribute must appear on the first declaration, it's not possible to override it.</div><div><br></div><div>Even if we made Clang play nice with our design or provide magic to allow libc++ to act in a sane manner, GCC is still an issue.</div></div></div></div></blockquote><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none">There’s another option, though. We could also just ditch internal_linkage (and always_inline for that matter) entirely, since I don’t think anybody knows whether we need to enable TUs built with different libc++ versions to interoperate. It’s “supported” right now, but it’s not clear that we actually need it nor support it properly. If we did that, we’d be down to a simple visibility problem, which we can solve using what John suggested. The main benefit I see is that we’d get rid of 95% of the visibility annotations in libc++: we’d simply build with `-fvisibility=hidden` and export exactly what we want. That would be more sane.</div></div></blockquote><div><br></div>Well, never mind this. It’s apparently not an option since clients (like libfuzzer) are relying on this. Someone mentioned in IRC that libfuzzer is linked statically against a libc++ they build themselves. And then a user application using (a possibly different version of) libc++ can link statically against libfuzzer. While I think this is a brittle setup (and it only works because libc++ is currently throwing __always_inline__ really hard at the problem), it’s probably not reasonable to stop supporting the use case since people have been relying on it.</div></div></blockquote><div><br></div><div>I haven't looked at what libfuzzer does, but as you've described it, I'd say this _doesn't_ seem a reasonable use-case. If you want to link libc++ (or any other library!) hidden inside your library, you should be using a version-script to only expose symbols that you intend to expose.</div><div><br></div><div><br></div></div></div>