On Wed, Mar 26, 2014 at 5:20 PM, Reid Kleckner <[email protected]> wrote:
> On Wed, Mar 26, 2014 at 4:28 PM, Rafael Espíndola < > [email protected]> wrote: > >> The llvm side of this was LGTMed, but unfortunately I failed to >> noticed that clang could produce aliases to weak aliases, so the patch >> got reverted pending changes to clang. >> >> With llvm rejecting aliases to weak aliases, we have to decide what to >> do with it in clang. >> >> One option is to error. That is fairly reasonable since the user can >> easily fix the code (see r204823 >> in compiler-rt for example). The issue is gcc incompatibility. >> >> What this patch does instead is skip past the weak alias in clang and >> warn the user that it is probably not the expected semantics. >> > > 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. > > diff --git a/include/clang/Basic/DiagnosticSemaKinds.td >> b/include/clang/Basic/DiagnosticSemaKinds.td >> index 13ad527..54082bc 100644 >> --- a/include/clang/Basic/DiagnosticSemaKinds.td >> +++ b/include/clang/Basic/DiagnosticSemaKinds.td >> @@ -2086,6 +2086,8 @@ def err_alias_not_supported_on_darwin : Error < >> "only weak aliases are supported on darwin">; >> def err_alias_to_undefined : Error< >> "alias must point to a defined variable or function">; >> +def warn_alias_to_weak_alias : Warning< >> + "An alias to an weak alias is not weak">, InGroup<IgnoredAttributes>; >> > > s/an weak/a weak/ > Also, diagnostics start with a lowercase letter. I don't think this diagnostic is very clear about the problem, how about something like: "alias to will always resolve to %1 even if weak definition of alias %2 is overridden" IgnoredAttributes seems right. > > >> def err_duplicate_mangled_name : Error< >> "definition with same mangled name as another definition">; >> def err_cyclic_alias : Error< >> diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp >> index c0fbdbe..f0deefe 100644 >> --- a/lib/CodeGen/CodeGenModule.cpp >> +++ b/lib/CodeGen/CodeGenModule.cpp > > @@ -217,7 +217,7 @@ void CodeGenModule::checkAliases() { >> StringRef MangledName = getMangledName(GD); >> llvm::GlobalValue *Entry = GetGlobalValue(MangledName); >> llvm::GlobalAlias *Alias = cast<llvm::GlobalAlias>(Entry); >> - llvm::GlobalValue *GV = Alias->resolveAliasedGlobal(/*stopOnWeak*/ >> false); >> + llvm::GlobalValue *GV = Alias->getAliasedGlobal(); >> if (!GV) { >> Error = true; >> getDiags().Report(AA->getLocation(), diag::err_cyclic_alias); >> @@ -225,6 +225,30 @@ void CodeGenModule::checkAliases() { >> Error = true; >> getDiags().Report(AA->getLocation(), diag::err_alias_to_undefined); >> } >> > > 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(). > > >> + // We have to handle alias to weak aliases in here. LLVM itself >> disallows >> + // this since the object semantics would not match the IL one. For >> + // compatibility with gcc we implement it by just pointing the alias >> + // to its aliasee's aliasee. We also warn, since the user is probably >> + // expecting the link to be weak. >> + llvm::Constant *Aliasee = Alias->getAliasee(); >> + llvm::GlobalValue *AliaseeGV; >> + if (auto CE = dyn_cast<llvm::ConstantExpr>(Aliasee)) { >> + assert((CE->getOpcode() == llvm::Instruction::BitCast || >> + CE->getOpcode() == llvm::Instruction::AddrSpaceCast) && >> + "Unsupported aliasee"); >> + AliaseeGV = cast<llvm::GlobalValue>(CE->getOperand(0)); >> + } else { >> + AliaseeGV = cast<llvm::GlobalValue>(Aliasee); >> + } >> + if (auto GA = dyn_cast<llvm::GlobalAlias>(AliaseeGV)) { >> + if (GA->mayBeOverridden()) { >> + getDiags().Report(AA->getLocation(), >> diag::warn_alias_to_weak_alias); > > + Aliasee = llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast( >> + GA->getAliasee(), Alias->getType()); >> + Alias->setAliasee(Aliasee); >> + } >> + } > > } >> if (!Error) >> return; >> diff --git a/test/CodeGen/alias.c b/test/CodeGen/alias.c >> index efa94b3..4a89b13 100644 >> --- a/test/CodeGen/alias.c >> +++ b/test/CodeGen/alias.c >> @@ -14,6 +14,8 @@ void f0(void) { } >> extern void f1(void); >> extern void f1(void) __attribute((alias("f0"))); >> // CHECKBASIC-DAG: @f1 = alias void ()* @f0 >> +// CHECKBASIC-DAG: @test8_foo = alias weak bitcast (void ()* @test8_bar >> to void (...)*) >> +// CHECKBASIC-DAG: @test8_zed = alias bitcast (void ()* @test8_bar to >> void (...)*) >> // CHECKBASIC: define void @f0() [[NUW:#[0-9]+]] { >> >> // Make sure that aliases cause referenced values to be emitted. >> @@ -48,3 +50,7 @@ int outer_weak(int a) { return inner_weak_a(a); } >> // CHECKBASIC: attributes [[NUW]] = { nounwind{{.*}} } >> >> // CHECKCC: attributes [[NUW]] = { nounwind{{.*}} } >> + >> +void test8_bar() {} >> +void test8_foo() __attribute__((weak, alias("test8_bar"))); >> +void test8_zed() __attribute__((alias("test8_foo"))); >> diff --git a/test/Sema/attr-alias-elf.c b/test/Sema/attr-alias-elf.c >> index 88bd7b7..2bec38a 100644 >> --- a/test/Sema/attr-alias-elf.c >> +++ b/test/Sema/attr-alias-elf.c >> @@ -52,3 +52,7 @@ extern int b3; >> >> extern int a4 __attribute__((alias("b4"))); // expected-error {{alias >> must point to a defined variable or function}} >> typedef int b4; >> + >> +void test2_bar() {} >> +void test2_foo() __attribute__((weak, alias("test2_bar"))); >> +void test2_zed() __attribute__((alias("test2_foo"))); // >> expected-warning {{An alias to an weak alias is not weak}} > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
