<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Mar 26, 2014 at 5:20 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">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></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></div></blockquote>
<div><br></div><div>Also, diagnostics start with a lowercase letter. I don't think this diagnostic is very clear about the problem, how about something like:</div><div><br></div><div>"alias to will always resolve to %1 even if weak definition of alias %2 is overridden"</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><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>
</blockquote></div><br></div></div>