<p dir="ltr">And I'm out on vacation again this week. :). Thanks, will commit when I am back Mon Jul 5.<br>
Teresa</p>
<br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 29, 2015, 5:04 PM Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Whereas I just missed this go by and have no excuse :/. LGTM too, FWIW.<br>
<br>
> On 2015-Jun-29, at 16:54, Reid Kleckner <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>> wrote:<br>
><br>
> Back from vacation myself. It's that time of year. :)<br>
><br>
> lgtm<br>
><br>
> On Tue, Jun 23, 2015 at 11:39 AM, Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>> wrote:<br>
><br>
><br>
> On Thu, Jun 11, 2015 at 8:17 PM, Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>> wrote:<br>
> Hi Nico,<br>
><br>
> Sorry about that. Since I am heading out on vacation for a week tomorrow I went ahead and reverted for now.<br>
><br>
> Teresa<br>
><br>
> On Thu, Jun 11, 2015 at 6:07 PM, Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:<br>
> Hi Teresa,<br>
><br>
> this (well, 239480 really) seems to break building dynamic libraries pretty decisively: <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__code.google.com_p_chromium_issues_detail-3Fid-3D499508-23c3&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=RSwmGgUK1DFkWLfzg_yGWA8FrYnWcmIhX3K1GnCT5PQ&s=S13Jd2tgg3PKwl0IEFuRUPSWaNtSp3jQbMz5HmBpQBo&e=" rel="noreferrer" target="_blank">https://code.google.com/p/chromium/issues/detail?id=499508#c3</a> Can you take a look, and if it takes a while to investigate, revert this for now?<br>
><br>
> Thanks,<br>
> Nico<br>
><br>
> I am back from vacation and found what was happening here. The attached patches are largely the same as the original ones but contain a fix for this issue (llvm patch) and a new test created from Nico's reduced test (clang patch).<br>
><br>
> The broken code was compiled -fvisibility=hidden, and this caused the thunk to SyncMessageFilter::Send (_ZThn16_N17SyncMessageFilter4SendEP7Message), which is available_externally, to also be marked hidden.<br>
><br>
> With my patch, we eliminated the function's body and turned it into a declaration, which was still marked hidden as I wasn't modifying visibility. During AsmPrinter::doFinalization, EmitVisibility is called on all function declarations in the module, which caused this symbol to get hidden visibility (via a resulting call to MCSymbolELF::setVisibility). The linker then complained because it was undefined and hidden.<br>
><br>
> Before my patch, code gen suppressed the emission of this function since it was available externally, and as a result, EmitVisibility, and thus MCSymbolELF::setVisibility, were simply never called. The undefined symbol then ended up with the default visibility. It seems to me that this essentially worked by luck.<br>
><br>
> I've fixed this by changing the visibility on globals to DefaultVisibility when we eliminate their definitions in the new EliminateAvailableExternally pass.<br>
><br>
> Patches attached. Tests (including the new one) all pass. Ok to commit?<br>
><br>
> Thanks,<br>
> Teresa<br>
><br>
><br>
> On Wed, Jun 10, 2015 at 10:49 AM, Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>> wrote:<br>
> Author: tejohnson<br>
> Date: Wed Jun 10 12:49:45 2015<br>
> New Revision: 239481<br>
><br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D239481-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=RSwmGgUK1DFkWLfzg_yGWA8FrYnWcmIhX3K1GnCT5PQ&s=ToYM6xXvuwJArMDIbifhoPJmYDv4goUkuCgWOlt7Kuw&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=239481&view=rev</a><br>
> Log:<br>
> Pass down the -flto option to the -cc1 job, and from there into the<br>
> CodeGenOptions and onto the PassManagerBuilder. This enables gating<br>
> the new EliminateAvailableExternally module pass on whether we are<br>
> preparing for LTO.<br>
><br>
> If we are preparing for LTO (e.g. a -flto -c compile), the new pass is not<br>
> included as we want to preserve available externally functions for possible<br>
> link time inlining.<br>
><br>
> --<br>
> Teresa Johnson | Software Engineer | <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> | 408-460-2413<br>
><br>
<br>
</blockquote></div>