[patch][pr17530] Prevent alias from pointing to weak aliases

[Reid Kleckner]

On Wed, Mar 26, 2014 at 4:28 PM, Rafael Espíndola <
rafael.espindola at gmail.com> 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/

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}}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140326/029b38fd/attachment.html>