[lld] r315658 - Slightly simplify code and add comment.

[Ed Maste via llvm-commits]

On 15 December 2017 at 21:01, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> Ed Maste via llvm-commits <llvm-commits at lists.llvm.org> writes:
>
>> Hi Rui,
>>
>> I can confirm that lld does in fact not behave exactly the same way
>> with this patch :)
>>
>> I was investigating lld on FreeBSD/i386, and found that with lld 5.0.1
>> in FreeBSD-HEAD we encounter an assertion failure from libc at
>> startup, for all dynamically linked i386 binaries. Testing with lld
>> HEAD was successful. Bisection turned up r315658 - it was a bit tricky
>> because r315552 introduced a regression that caused libc to fail to
>> link, fixed in r315737. In any case, r315658 has the pleasant
>> side-effect of fixing lld for FreeBSD/i386.
>>
>> I'm investigating the differences in the output .so when linked with
>> lld with/without this patch and will follow up with my findings. I'd
>> like to try to add a test case for the behaviour that changed so that
>> we can ensure it doesn't regress, then Dimitry or I will likely merge
>> it into the lld 5.0.1 in FreeBSD.
>
> If you put the output of --reproduce somewhere it shouldn't be too hard
> to reduce it.

I emailed a link to the full reproducer to directly to Rui - it's at
https://people.freebsd.org/~emaste/lld-reproduce.txz. It's a full
32-bit FreeBSD libc and I agree it should be easy to reduce; I didn't
do so because it wasn't necessary for bisection, and now that the
commit has been identified it seems fairly straightforward.

Comparing the produced libc shard objs many of the got entries were 0
before the change, and populated after -- and after looking at the
diff it's not too surprising given this addition in the change:

+    // REL type relocations don't have addend fields unlike RELAs, and
+    // their addends are stored to the section to which they are applied.
+    // So, store addends if we need to.
+    //
+    // This is ugly -- the difference between REL and RELA should be
+    // handled in a better way. It's a TODO.
+    if (!Config->IsRela)
+      InX::Got->Relocations.push_back({R_ABS, Target->GotRel, Off, 0, &Sym});