<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Mar 26, 2014 at 4:28 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">The llvm side of this was LGTMed, but unfortunately I failed to<br>


noticed that clang could produce aliases to weak aliases, so the patch<br>
got reverted pending changes to clang.<br>
<br>
With llvm rejecting aliases to weak aliases, we have to decide what to<br>
do with it in clang.<br>
<br>
One option is to error. That is fairly reasonable since the user can<br>
easily fix the code (see r204823<br>
in compiler-rt for example). The issue is gcc incompatibility.<br>
<br>
What this patch does instead is skip past the weak alias in clang and<br>
warn the user that it is probably not the expected semantics.<br></blockquote><div><br></div><div>Sounds good.  I do recall writing a bunch of code like this in DynamoRIO and it'd be nice if kept behaving the same way.</div>

</div></div><div class="gmail_extra"><br></div><div class="gmail_extra"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td<br>index 13ad527..54082bc 100644<br>--- a/include/clang/Basic/DiagnosticSemaKinds.td<br>+++ b/include/clang/Basic/DiagnosticSemaKinds.td<br>

@@ -2086,6 +2086,8 @@ def err_alias_not_supported_on_darwin : Error <<br>   "only weak aliases are supported on darwin">;<br> def err_alias_to_undefined : Error<<br>   "alias must point to a defined variable or function">;<br>

+def warn_alias_to_weak_alias : Warning<<br>+  "An alias to an weak alias is not weak">, InGroup<IgnoredAttributes>;<br></blockquote><div><br></div><div>s/an weak/a weak/</div><div><br></div><div>IgnoredAttributes seems right.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
 def err_duplicate_mangled_name : Error<<br>   "definition with same mangled name as another definition">;<br> def err_cyclic_alias : Error<<br>diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp<br>

index c0fbdbe..f0deefe 100644<br>--- a/lib/CodeGen/CodeGenModule.cpp<br>+++ b/lib/CodeGen/CodeGenModule.cpp</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
@@ -217,7 +217,7 @@ void CodeGenModule::checkAliases() {<br>     StringRef MangledName = getMangledName(GD);<br>
     llvm::GlobalValue *Entry = GetGlobalValue(MangledName);<br>     llvm::GlobalAlias *Alias = cast<llvm::GlobalAlias>(Entry);<br>-    llvm::GlobalValue *GV = Alias->resolveAliasedGlobal(/*stopOnWeak*/ false);<br>

+    llvm::GlobalValue *GV = Alias->getAliasedGlobal();<br>     if (!GV) {<br>       Error = true;<br>       getDiags().Report(AA->getLocation(), diag::err_cyclic_alias);<br>@@ -225,6 +225,30 @@ void CodeGenModule::checkAliases() {<br>

       Error = true;<br>       getDiags().Report(AA->getLocation(), diag::err_alias_to_undefined);<br>     }<br></blockquote><div><br></div><div>Having this in CodeGen is really unfortunate.  I'm assuming this is because we don't know mangled names until CodeGen time.  IMO that's probably worth a big doc comment on top of checkAliases().</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">+    // We have to handle alias to weak aliases in here. LLVM itself disallows<br>
+    // this since the object semantics would not match the IL one. For<br>
+    // compatibility with gcc we implement it by just pointing the alias<br>+    // to its aliasee's aliasee. We also warn, since the user is probably<br>+    // expecting the link to be weak.<br>+    llvm::Constant *Aliasee = Alias->getAliasee();<br>

+    llvm::GlobalValue *AliaseeGV;<br>+    if (auto CE = dyn_cast<llvm::ConstantExpr>(Aliasee)) {<br>+      assert((CE->getOpcode() == llvm::Instruction::BitCast ||<br>+              CE->getOpcode() == llvm::Instruction::AddrSpaceCast) &&<br>

+             "Unsupported aliasee");<br>+      AliaseeGV = cast<llvm::GlobalValue>(CE->getOperand(0));<br>+    } else {<br>+      AliaseeGV = cast<llvm::GlobalValue>(Aliasee);<br>+    }<br>+    if (auto GA = dyn_cast<llvm::GlobalAlias>(AliaseeGV)) {<br>

+      if (GA->mayBeOverridden()) {<br>+        getDiags().Report(AA->getLocation(), diag::warn_alias_to_weak_alias);</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

+        Aliasee = llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast(<br>+            GA->getAliasee(), Alias->getType());<br>+        Alias->setAliasee(Aliasee);<br>+      }<br>+    }</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

   }<br>   if (!Error)<br>     return;<br>diff --git a/test/CodeGen/alias.c b/test/CodeGen/alias.c<br>index efa94b3..4a89b13 100644<br>--- a/test/CodeGen/alias.c<br>+++ b/test/CodeGen/alias.c<br>@@ -14,6 +14,8 @@ void f0(void) { }<br>

 extern void f1(void);<br> extern void f1(void) __attribute((alias("f0")));<br> // CHECKBASIC-DAG: @f1 = alias void ()* @f0<br>+// CHECKBASIC-DAG: @test8_foo = alias weak bitcast (void ()* @test8_bar to void (...)*)<br>

+// CHECKBASIC-DAG: @test8_zed = alias bitcast (void ()* @test8_bar to void (...)*)<br> // CHECKBASIC: define void @f0() [[NUW:#[0-9]+]] {<br> <br> // Make sure that aliases cause referenced values to be emitted.<br>@@ -48,3 +50,7 @@ int outer_weak(int a) { return inner_weak_a(a); }<br>

 // CHECKBASIC: attributes [[NUW]] = { nounwind{{.*}} }<br> <br> // CHECKCC: attributes [[NUW]] = { nounwind{{.*}} }<br>+<br>+void test8_bar() {}<br>+void test8_foo() __attribute__((weak, alias("test8_bar")));<br>

+void test8_zed() __attribute__((alias("test8_foo")));<br>diff --git a/test/Sema/attr-alias-elf.c b/test/Sema/attr-alias-elf.c<br>index 88bd7b7..2bec38a 100644<br>--- a/test/Sema/attr-alias-elf.c<br>+++ b/test/Sema/attr-alias-elf.c<br>

@@ -52,3 +52,7 @@ extern int b3;<br> <br> extern int a4 __attribute__((alias("b4"))); // expected-error {{alias must point to a defined variable or function}}<br> typedef int b4;<br>+<br>+void test2_bar() {}<br>

+void test2_foo() __attribute__((weak, alias("test2_bar")));<br>+void test2_zed() __attribute__((alias("test2_foo"))); // expected-warning {{An alias to an weak alias is not weak}}</blockquote><div><br>

</div></div></div>