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

[Richard Smith]

On Wed, Mar 26, 2014 at 5:20 PM, Reid Kleckner <rnk at google.com> wrote:

> 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/
>

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