Suprateeka R Hegde
2015-Apr-02 18:47 UTC
[LLVMdev] Review Request: Fix for a bug found in applying R_AARCH64_ADR_GOT_PAGE relocation
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