[LLVMdev] Review Request: Fix for a bug found in applying R_AARCH64_ADR_GOT_PAGE relocation

[Suprateeka R Hegde]

Hi

While working on lld, I observed that applying R_AARCH64_ADR_GOT_PAGE 
relocation causes a hello-world program to crash with SIGSEGV when 
compiled/linked with the trunk versions of clang/lld on AArch64.

As per my understanding, this is due to a bug in the implementation of 
relocR_AARCH64_ADR_GOT_PAGE() in file AArch64RelocationHandler.cpp. This 
seems to be a regression compared to the stable version RELEASE_360 of lld.

The generated code sequence with the bug is:
---
00000000004003b8 <call_weak_fn>:
   4003b8:       90000001        adrp    x1, 400000 
<__preinit_array_end+0x400000>
   4003bc:       f9409000        ldr     x0, [x0,#288]
   4003c0:       b4000040        cbz     x0, 4003c8 <call_weak_fn+0x10>
---

The bad relocation fix-up has changed the adrp instruction to use 
register x1 instead of x0. The address fix-up is also wrong.

Attached is a patch that fixes the above issue, and the generated code 
sequence with the fixed lld is:
---
00000000004003b8 <call_weak_fn>:
   4003b8:       b0000000        adrp    x0, 401000 <__init_array_start>
   4003bc:       f9409000        ldr     x0, [x0,#288]
   4003c0:       b4000040        cbz     x0, 4003c8 <call_weak_fn+0x10>
---

While fixing this, I observed that implementation of other relocations 
may also have similar issues. I am yet to hit the problem if any because 
of them. I shall post a patch in that case.

I have tested this patch with lld test suite. In case this looks good, 
can someone please commit this to the lld trunk (I do not have write 
privilege).

In addition, I need help on the following:
1. What is the process to update the page - 
http://lld.llvm.org/open_projects.html#elf-aarch64 ? I would say, as and 
when we progress on AArch64, we need to update this too.
2. I am planning to add a test case under test/elf/AArch64. What is the 
right place for a test case for this problem?


Thanks a lot
--
Supra
-------------- next part --------------
Index: lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp
===================================================================
--- lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp	(revision 233917)
+++ lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp	(working copy)
@@ -223,7 +223,7 @@
         llvm::dbgs() << " immhi: " << Twine::utohexstr(immhi);
         llvm::dbgs() << " immlo: " << Twine::utohexstr(immlo);
         llvm::dbgs() << " result: " << Twine::utohexstr(result) << "\n");
-  write32le(location, result | read32le(location));
+  write32le(location, immhi | immlo | read32le(location));
 }
 
 // R_AARCH64_LD64_GOT_LO12_NC