<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 13, 2017 at 4:53 PM, Teresa Johnson <span dir="ltr"><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Thanks, David and Peter. Some responses to Peter's email below. Teresa<div class="gmail_extra"><br><div class="gmail_quote"><span>On Fri, Jan 13, 2017 at 3:21 PM, Peter Collingbourne <span dir="ltr"><<a href="mailto:peter@pcc.me.uk" target="_blank">peter@pcc.me.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Teresa,<div><br></div><div>I think that to answer your question correctly it is helpful to consider what is going on at the object file level. For your test1.c we conceptually have a .text section containing the body of f, and then three symbols:</div><div><br></div><div>.weak f</div><div>f = .text</div><div>.globl strongalias</div><div>strongalias = .text</div><div>.weak weakalias</div><div>weakalias = .text</div><div><br></div><div>Note that f, strongalias and weakalias are not related at all, except that they happen to point to the same place. If f is overridden by a symbol in another object file, it does not affect the symbols strongalias and weakalias, so we still need to make them point to .text. I don't think it would be right to make strongalias and weakalias into copies of f, as that would be observable through function pointer equality. Most likely all you need to do is to internalize f and keep strongalias and weakalias as aliases of f.</div></div></blockquote><div><br></div></span><div>Good point on wanting function pointer equality.  However, we can't simply internalize f(). We'll also need to rename the internalized copy. The reason is that we want the original f() references to resolve to the prevailing copy in the other module.Summarizing what we just talked about on IRC, when we have a non-prevailing weak/linkonce symbol f() that has an alias point to it, we need to:</div><div>1) Rename and internalize f()</div><div>2) Create a new external decl f()</div><div>3) RAUW existing references (other than from the aliases) with the new local created in 1)</div><div><br></div><div>I think if it is however a weak_odr/linkonce_odr we can simplify the process since all copies will be the same. We can make f() available_externally (to enable inlining), and simply convert references to aliases of f() into direct references to f() and drop the aliases - does that sound right?  </div></div></div></div></blockquote><div><br></div><div>I think you are right about the _odr -- we should assume that even if the symbol as we see it is an alias it will be replaced with something with the same semantics. I think we can also take that into account in the logic for replacing the alias with its aliasee, as follows:</div><div><br></div><div>- if the alias is internal, strong external or odr (i.e. not isInterposableLinkage): may replace the global with an available_externally copy of its aliasee</div><div>- if both the alias and aliasee are not isInterposableLinkage: may replace the global reference with a reference to the aliasee</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> Another tricky thing is if the weak symbol was a variable that is initialized via a __cxx_global_var_init function in the global_ctors list. If we have an alias to that symbol, presumably we'll want the new internalized/renamed version to get initialized instead? </div></div></div></div></blockquote><div><br></div><div>I believe that the purpose of the global reference in global_ctors is to control which comdat the .init_array entry appears in. So yes, we will need to rewrite the reference in global_ctors to point to the renamed global. However, I think we also need to think about whether to remove the init_array entry entirely. There are a couple of cases:</div><div><br></div><div>1) All symbols in the comdat are weak and have been overridden by strong symbols in another object file, which may not necessarily be in a comdat. In that case we need to keep the init_array entry so that we call the initializer function.</div><div>2) The linker has selected another comdat entirely. That means that this object file's init_array entry has not been chosen and we need to drop it.</div><div><br></div><div>The interesting thing about these two cases is that they are indistinguishable at the LTO API level because the linker will report all of the comdat symbols as non-prevailing.</div><div><br></div><div>I don't think it is possible to arrive at case 1 with regular C++, so maybe we'd be fine assuming case 2 if all comdat symbols are non-prevailing. But then again it would seem to make the implementation simpler if we communicate which comdat has prevailed at the LTO API level.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Now in the case where we have an alias that is itself a weak non-prevailing symbol, how we handle will I think depend on what it is aliased to:</div><div>a) aliased to a weak/linkonce non-prevailing symbol -> handle as described earlier</div><div>b) aliased to a weak_odr/linkonce_odr non-prevailing symbol -> handle as described earlier</div><div>c) aliased to a strong symbol or a prevailing symbol -> convert to external decl (I think this case is only possible if the alias is a non-odr weak/linkonce)</div><div><br></div><div>Does that sound right?</div></div></div></div></blockquote><div><br></div><div>I think the logic just needs to depend on the linkage of the alias itself, as I described above.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div>If we're resolving strongalias to f at -O2, that seems like a bug to me. We can probably only resolve an alias to the symbol it references if we are guaranteed that both symbols will have the same resolution, i.e. we must check at least that both symbols have strong or internal linkage. If we cared about symbol interposition, we might also want to check that both symbols have non-default visibility, but I think that our support for that is still a little fuzzy at the moment.<br></div></div></blockquote><div><br></div></span><div>Per your and David's analysis it sounds like this is a bug then - I can file a bug to track it with the example.</div><div><br></div><div>Regarding the comdat case I mentioned - Peter and I discussed on IRC and he pointed out that my case was illegal since aliases are by definition in the same comdat group as the symbol they alias. So in effect I had an incomplete comdat group.</div><div><br></div><div>Thanks,</div><div>Teresa</div><div><div class="m_6431447535946325881m_-8957994097528057329m_5978050163523393214gmail-m_-5936775844221289801h5"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div><br></div><div>Thanks,</div><div>Peter</div></div><div class="gmail_extra"><br><div class="gmail_quote"><span>On Fri, Jan 13, 2017 at 2:33 PM, Teresa Johnson <span dir="ltr"><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>></span> wrote:<br></span><div><div class="m_6431447535946325881m_-8957994097528057329m_5978050163523393214gmail-m_-5936775844221289801m_5701581641049668504gmail-m_-421348620981757362h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Mehdi, Peter and David (and anyone else who sees this),<div><br></div><div>I've been playing with some examples to handle the weak symbol cases we discussed in IRC earlier this week in the context of D28523. I was going to implement the support for turning aliases into copies in order to enable performing thinLTOResolveWeakF<wbr>orLinkerGUID on both aliases and aliasees, as a first step to being able to drop non-prevailing weak symbols in ThinLTO backends.</div><div><br></div><div>I was wondering though what happens if we have an alias, which may or may not be weak itself, to a non-odr weak symbol that isn't prevailing. In that case, do we eventually want references via the alias to go to the prevailing copy (in another module), or to the original copy in the alias's module? I looked at some examples without ThinLTO, and am a little confused. Current (non-ThinLTO) behavior in some cases seems to depend on opt level.</div><div><br></div><div>Example:</div><div><br></div><div><div>$ cat weak12main.c</div><div>extern void test2();</div><div>int main() {</div><div>  test2();</div><div>}</div><div><br></div><div>$ cat weak1.c</div><div>#include <stdio.h></div><div><br></div><div>void weakalias() __attribute__((weak, alias ("f")));</div><div>void strongalias() __attribute__((alias ("f")));</div><div><br></div><div>void f () __attribute__ ((weak));</div><div>void f()</div><div>{</div><div>  printf("In weak1.c:f\n");</div><div>}</div><div>void test1() {</div><div>  printf("Call f() from weak1.c:\n");</div><div>  f();</div><div>  printf("Call weakalias() from weak1.c:\n");</div><div>  weakalias();</div><div>  printf("Call strongalias() from weak1.c:\n");</div><div>  strongalias();</div><div>}</div><div><br></div><div>$ cat weak2.c</div><div>#include <stdio.h></div><div><br></div><div>void f () __attribute__ ((weak));</div><div>void f()</div><div>{</div><div>  printf("In weak2.c:f\n");</div><div>}</div><div>extern void test1();</div><div>void test2()</div><div>{</div><div>  test1();</div><div>  printf("Call f() from weak2.c\n");</div><div>  f();</div><div>}</div></div><div><br></div><div>If I link weak1.c before weak2.c, nothing is surprising (we always invoke weak1.c:f at both -O0 and -O2):</div><div><br></div><div><div><div>$ clang weak12main.c weak1.c weak2.c -O0</div><div>$ a.out</div><div>Call f() from weak1.c:</div><div>In weak1.c:f</div><div>Call weakalias() from weak1.c:</div><div>In weak1.c:f</div><div>Call strongalias() from weak1.c:</div><div>In weak1.c:f</div><div>Call f() from weak2.c</div><div>In weak1.c:f</div><div><br></div><div>$ clang weak12main.c weak1.c weak2.c -O2</div><div>$ a.out</div><div>Call f() from weak1.c:</div><div>In weak1.c:f</div><div>Call weakalias() from weak1.c:</div><div>In weak1.c:f</div><div>Call strongalias() from weak1.c:</div><div>In weak1.c:f</div><div>Call f() from weak2.c</div><div>In weak1.c:f</div><div><br></div><div>If I instead link weak2.c first, so it's copy of f() is prevailing, I still get weak1.c:f for the call via weakalias() (both opt levels), and for strongalias() when building at -O0. At -O2 the compiler replaces the call to strongalias() with a call to f(), so it get's the weak2 copy in that case.</div><div><br></div><div>$ clang weak12main.c weak2.c weak1.c -O2</div><div>$ a.out</div><div>Call f() from weak1.c:</div><div>In weak2.c:f</div><div>Call weakalias() from weak1.c:</div><div>In weak1.c:f</div><div>Call strongalias() from weak1.c:</div><div>In weak2.c:f</div><div>Call f() from weak2.c</div><div>In weak2.c:f</div><div><br></div><div>$ clang weak12main.c weak2.c weak1.c -O0</div><div>$ a.out</div><div>Call f() from weak1.c:</div><div>In weak2.c:f</div><div>Call weakalias() from weak1.c:</div><div>In weak1.c:f</div><div>Call strongalias() from weak1.c:</div><div>In weak1.c:f</div><div>Call f() from weak2.c</div><div>In weak2.c:f</div></div><div><br></div><div>I'm wondering what the expected/correct behavior is? Depending on what is correct, we need to handle this differently in ThinLTO mode. Let's say weak1.c's copy of f() is not prevailing and I am going to drop it (it needs to be removed completely, not turned into available_externally to ensure it isn't inlined since weak isInterposable). If we want the aliases in weak1.c to reference the original version, then copying is correct (e.g. weakalias and strong alias would each become a copy of weak1.c's f()). If we however want them to resolve to the prevailing copy of f(), then we need to turn the aliases into declarations (external linkage in the case of strongalias and external weak in the case of weakalias?). </div><div><br></div><div>I also tried the case where f() was in a comdat, because I also need to handle that case in ThinLTO (when f() is not prevailing, drop it from the comdat and remove the comdat from that module). Interestingly, in this case when weak2.c is prevailing, I get the following warning when linking and get a seg fault at runtime:</div><div><br></div><div><div>weak1.o:weak1.o:function test1: warning: relocation refers to discarded section</div></div><div><br></div><div>Presumably the aliases still refer to the copy in weak1.c, which is in the comdat that gets dropped by the linker. So is it not legal to have an alias to a weak symbol in a comdat (i.e. alias from outside the comdat)? We don't complain in the compiler.</div><div><br></div><div>Thanks,</div><div>Teresa</div><span class="m_6431447535946325881m_-8957994097528057329m_5978050163523393214gmail-m_-5936775844221289801m_5701581641049668504gmail-m_-421348620981757362m_1534376695556713550HOEnZb"><font color="#888888">-- <br><div class="m_6431447535946325881m_-8957994097528057329m_5978050163523393214gmail-m_-5936775844221289801m_5701581641049668504gmail-m_-421348620981757362m_1534376695556713550m_1265033782169285673gmail_signature"><span style="font-family:times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"> <a href="tel:(408)%20460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</font></span></div></div>
</blockquote></div></div></div><span class="m_6431447535946325881m_-8957994097528057329m_5978050163523393214gmail-m_-5936775844221289801m_5701581641049668504gmail-m_-421348620981757362HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div class="m_6431447535946325881m_-8957994097528057329m_5978050163523393214gmail-m_-5936775844221289801m_5701581641049668504gmail-m_-421348620981757362m_1534376695556713550gmail_signature"><div dir="ltr">-- <div>Peter</div></div></div>
</font></span></div>
</blockquote></div></div></div><div><div class="m_6431447535946325881m_-8957994097528057329m_5978050163523393214gmail-m_-5936775844221289801h5"><br><br clear="all"><div><br></div>-- <br><div class="m_6431447535946325881m_-8957994097528057329m_5978050163523393214gmail-m_-5936775844221289801m_5701581641049668504gmail-m_-421348620981757362gmail_signature"><span style="font-family:times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"> <a href="tel:(408)%20460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</div></div></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="m_6431447535946325881m_-8957994097528057329m_5978050163523393214gmail-m_-5936775844221289801gmail_signature"><div dir="ltr">-- <div>Peter</div></div></div>
</div></div>