From 3b1622d63a9c2e0a63a5796f94a7f4218a9a4cad Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Fri, 24 Apr 2020 11:23:23 +0100 Subject: [PATCH] [LLD][ELF][ARM] recommit Fix ARM Exidx order for non monotonic section order Fixed error detected by msan. The size field of the .ARM.exidx synthetic section needs to be initialized to at least estimation level before calling assignAddresses as that will use the size field. This was previously reverted in 1ca16fc4f5146b90512d4740cfcc4d4c34640853. Differential Revision: https://reviews.llvm.org/D78422 --- lld/ELF/SyntheticSections.cpp | 11 +++++-- lld/ELF/SyntheticSections.h | 2 +- lld/ELF/Writer.cpp | 16 ++++++---- lld/test/ELF/arm-exidx-script-order.s | 57 +++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 lld/test/ELF/arm-exidx-script-order.s diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp index 8cf813c..e1322cd 100644 --- a/lld/ELF/SyntheticSections.cpp +++ b/lld/ELF/SyntheticSections.cpp @@ -3276,8 +3276,13 @@ static bool isValidExidxSectionDep(InputSection *isec) { bool ARMExidxSyntheticSection::addSection(InputSection *isec) { if (isec->type == SHT_ARM_EXIDX) { if (InputSection *dep = isec->getLinkOrderDep()) - if (isValidExidxSectionDep(dep)) + if (isValidExidxSectionDep(dep)) { exidxSections.push_back(isec); + // Every exidxSection is 8 bytes, we need an estimate of + // size before assignAddresses can be called. Final size + // will only be known after finalize is called. + size += 8; + } return true; } @@ -3363,14 +3368,14 @@ void ARMExidxSyntheticSection::finalizeContents() { // Sort the executable sections that may or may not have associated // .ARM.exidx sections by order of ascending address. This requires the - // relative positions of InputSections to be known. + // relative positions of InputSections and OutputSections to be known. auto compareByFilePosition = [](const InputSection *a, const InputSection *b) { OutputSection *aOut = a->getParent(); OutputSection *bOut = b->getParent(); if (aOut != bOut) - return aOut->sectionIndex < bOut->sectionIndex; + return aOut->addr < bOut->addr; return a->outSecOff < b->outSecOff; }; llvm::stable_sort(executableSections, compareByFilePosition); diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h index 05eee24..8ed82ba 100644 --- a/lld/ELF/SyntheticSections.h +++ b/lld/ELF/SyntheticSections.h @@ -1046,7 +1046,7 @@ public: std::vector exidxSections; private: - size_t size; + size_t size = 0; // Instead of storing pointers to the .ARM.exidx InputSections from // InputObjects, we store pointers to the executable sections that need diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp index 91fdff1..a9dd643 100644 --- a/lld/ELF/Writer.cpp +++ b/lld/ELF/Writer.cpp @@ -1604,6 +1604,11 @@ template void Writer::resolveShfLinkOrder() { } } +static void finalizeSynthetic(SyntheticSection *sec) { + if (sec && sec->isNeeded() && sec->getParent()) + sec->finalizeContents(); +} + // We need to generate and finalize the content that depends on the address of // InputSections. As the generation of the content may also alter InputSection // addresses we must converge to a fixed point. We do that here. See the comment @@ -1613,6 +1618,11 @@ template void Writer::finalizeAddressDependentContent() { AArch64Err843419Patcher a64p; ARMErr657417Patcher a32p; script->assignAddresses(); + // .ARM.exidx does not require precise addresses, but it does require the + // relative addresses of OutputSections because linker scripts can assign + // Virtual Addresses to OutputSections that are not monotonically increasing. + for (Partition &part : partitions) + finalizeSynthetic(part.armExidx); // Converts call x@GDPLT to call __tls_get_addr if (config->emachine == EM_HEXAGON) @@ -1762,11 +1772,6 @@ template void Writer::optimizeBasicBlockJumps() { } } -static void finalizeSynthetic(SyntheticSection *sec) { - if (sec && sec->isNeeded() && sec->getParent()) - sec->finalizeContents(); -} - // In order to allow users to manipulate linker-synthesized sections, // we had to add synthetic sections to the input section list early, // even before we make decisions whether they are needed. This allows @@ -2035,7 +2040,6 @@ template void Writer::finalizeSections() { // Dynamic section must be the last one in this list and dynamic // symbol table section (dynSymTab) must be the first one. for (Partition &part : partitions) { - finalizeSynthetic(part.armExidx); finalizeSynthetic(part.dynSymTab); finalizeSynthetic(part.gnuHashTab); finalizeSynthetic(part.hashTab); diff --git a/lld/test/ELF/arm-exidx-script-order.s b/lld/test/ELF/arm-exidx-script-order.s new file mode 100644 index 0000000..809278b --- /dev/null +++ b/lld/test/ELF/arm-exidx-script-order.s @@ -0,0 +1,57 @@ +// REQUIRES: arm +// RUN: llvm-mc --arm-add-build-attributes --triple=armv7a-linux-gnueabihf -filetype=obj %s -o %t.o +// RUN: echo "SECTIONS { \ +// RUN: . = 0x80000000; \ +// RUN: .ARM.exidx : { *(.ARM.exidx) } \ +// RUN: .text : { *(.text) } \ +// RUN: .text.1 0x80000200 : AT(0x1000) { *(.text.1) } \ +// RUN: .text.2 0x80000100 : AT(0x2000) { *(.text.2) } \ +// RUN: } " > %t.script +// RUN: ld.lld --script %t.script %t.o -o %t +// RUN: llvm-readobj -x .ARM.exidx %t | FileCheck %s + +/// When a linker script does not have monotonically increasing addresses +/// the .ARM.exidx table should still be in monotonically increasing order. + +// CHECK: Hex dump of section '.ARM.exidx': +// 0x80000000 + 0x28 = 0x80000028, 0x80000008 + 0xf8 = 0x80000100 +// CHECK-NEXT: 0x80000000 24000000 08849780 f8000000 20849980 +// 0x80000010 + 0x1f0 = 0x8000200, 0x80000018 + 0x1ec = 0x8000204 +// CHECK-NEXT: 0x80000010 f0010000 10849880 ec010000 01000000 + + .text + .global _start + .type _start, %function +_start: + .fnstart + bx lr + .save {r7, lr} + .setfp r7, sp, #0 + .fnend + + .section .text.1, "ax", %progbits + .global fn1 + .type fn1, %function +fn1: + .fnstart + bx lr + .save {r8, lr} + .setfp r8, sp, #0 + .fnend + + .section .text.2, "ax", %progbits + .global fn2 + .type fn2, %function +fn2: + .fnstart + bx lr + .save {r9, lr} + .setfp r9, sp, #0 + .fnend + +/// Dummy definition for a reference from the personality routine created by +/// the assembler, use .data to avoid generating a cantunwind table. + .section .rodata + .global __aeabi_unwind_cpp_pr0 +__aeabi_unwind_cpp_pr0: + .word 0 -- 2.7.4