[PATCH] D13825: [ELF2] Don't create RelativeReloc for weak undef symbols

[Rafael Espíndola via llvm-commits]

The test is OK, but would the attached patch fix the problem for you?

I think it is a better spot to put the test.

On 16 October 2015 at 16:24, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> The test I think is correct. Give me a minute to see if there is a
> better place in the code to put the check.
>
> On 16 October 2015 at 15:30, hfinkel at anl.gov <hfinkel at anl.gov> wrote:
>> hfinkel created this revision.
>> hfinkel added reviewers: rafael, ruiu.
>> hfinkel added a subscriber: llvm-commits.
>> hfinkel added a project: lld.
>>
>> When we have a R_PPC64_ADDR64 for a weak undef symbol, which thus resolves to 0, and we're creating a shared library, we need to make sure that it stays 0 (because code that conditionally calls the weak function tests for this). Unfortunately, we were creating a R_PPC64_RELATIVE for these relocation targets, making the address of the undefined weak symbol equal to the base address of the shared library (which is non-zero). It seems like, in general, we should not be creating RelativeReloc relocs for undef weak symbols.
>>
>>
>> http://reviews.llvm.org/D13825
>>
>> Files:
>>   ELF/Writer.cpp
>>   test/elf2/ppc64-weak-undef-call-shared.s
>>
>> Index: test/elf2/ppc64-weak-undef-call-shared.s
>> ===================================================================
>> --- /dev/null
>> +++ test/elf2/ppc64-weak-undef-call-shared.s
>> @@ -0,0 +1,11 @@
>> +# RUN: llvm-mc -filetype=obj -triple=powerpc64-unknown-linux %s -o %t.o
>> +# RUN: ld.lld2 -shared %t.o -o %t.so
>> +# RUN: llvm-readobj -t -r -dyn-symbols %t.so | FileCheck %s
>> +# REQUIRES: ppc
>> +
>> +.section        ".toc","aw"
>> +.quad weakfunc
>> +// CHECK-NOT: R_PPC64_RELATIVE
>> +
>> +.weak weakfunc
>> +
>> Index: ELF/Writer.cpp
>> ===================================================================
>> --- ELF/Writer.cpp
>> +++ ELF/Writer.cpp
>> @@ -201,7 +201,8 @@
>>      }
>>
>>      bool CBP = canBePreempted(Body, NeedsGot);
>> -    if (!CBP && (!Config->Shared || Target->isRelRelative(Type)))
>> +    if (!CBP && (!Config->Shared || Target->isRelRelative(Type) ||
>> +                 (Body && Body->isWeak() && Body->isUndefined())))
>>        continue;
>>      if (CBP)
>>        Body->setUsedInDynamicReloc();
>>
>>
-------------- next part --------------
diff --git a/ELF/OutputSections.cpp b/ELF/OutputSections.cpp
index 7f7b970..3d53f2c 100644
--- a/ELF/OutputSections.cpp
+++ b/ELF/OutputSections.cpp
@@ -473,7 +473,10 @@ bool lld::elf2::canBePreempted(const SymbolBody *Body, bool NeedsGot) {
     //   the plt entry would have a non zero address.
     // Since we cannot do anything better, we just resolve the symbol to 0 and
     // don't produce a dynamic relocation.
-    return NeedsGot;
+    //
+    // As an extra hack, assume that if we are producing a shared library
+    // the user knows what it is doing and can handle a dynamic relocation.
+    return Config->Shared || NeedsGot;
   }
   if (!Config->Shared)
     return false;