[lld] [PATCH] Properly handle relocation for LSDA in MachO eh-frames.
Rui Ueyama
ruiu at google.com
Tue May 12 13:00:38 PDT 2015
Do you mind if I ask you to upload the patch to Fabricator? It'd much
easier to review.
> Index: lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp
> ===================================================================
> --- lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp (revision
237164)
> +++ lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp (working copy)
> @@ -29,6 +29,7 @@
> #include "lld/Core/LLVM.h"
> #include "llvm/Support/Format.h"
> #include "llvm/Support/MachO.h"
> +#include "llvm/Support/LEB128.h"
>
> using namespace llvm::MachO;
> using namespace lld::mach_o::normalized;
> @@ -644,11 +645,151 @@
> return res;
> }
>
> +/// --- Augmentation String Processing ---
> +
> +class AugmentationDataInfo {
> +public:
> + AugmentationDataInfo()
> + : present(false), mayHaveLSDA(false) {}
> +
> + bool present;
> + bool mayHaveLSDA;
> +};
You can use C++ non-static member initialization here.
Also please follow the LLD coding style (I know that having
different coding style is not a good idea, but this is how
the code is formatted for LLD).
bool _present = false;
bool _mayHaveLSDA = false;
> +static std::error_code processAugmentationString(const uint8_t *augStr,
> + AugmentationDataInfo
&adi,
> + unsigned *len =
nullptr) {
> +
> + if (augStr[0] == '\0') {
> + if (len)
> + *len = 1;
> + return std::error_code();
> + }
> +
> + if (augStr[0] != 'z')
> + return make_dynamic_error_code("expected 'z' at start of
augmentation "
> + "string");
> +
> + adi.present = true;
> + uint64_t Idx = 1;
idx
> + while (augStr[Idx] != '\0') {
> + if (augStr[Idx] == 'L') {
> + adi.mayHaveLSDA = true;
> + ++Idx;
> + } else
> + ++Idx;
> + }
> +
> + if (len)
> + *len = Idx + 1;
> + return std::error_code();
> +}
> +
> +static std::error_code processCIE(const NormalizedFile &normalizedFile,
> + MachODefinedAtom *atom,
> + AugmentationDataInfo &adi) {
> + const bool isBig =
MachOLinkingContext::isBigEndian(normalizedFile.arch);
> +
> + const uint8_t *frameData = atom->rawContent().data();
> + uint32_t size = read32(frameData, isBig);
> + uint64_t cieIDField = size == 0xffffffffU
> + ? sizeof(uint32_t) + sizeof(uint64_t)
> + : sizeof(uint32_t);
sizeof(uint32_t) or sizeof(uint64_t) look a bit odd to me because
they are always 4 or 8.
> + uint64_t versionField = cieIDField + sizeof(uint32_t);
> + uint64_t augmentationStringField = versionField + sizeof(uint8_t);
> +
> + if (auto err = processAugmentationString(frameData +
augmentationStringField,
> + adi))
> + return err;
> +
> + return std::error_code();
> +}
> +
> +static std::error_code processFDE(const NormalizedFile &normalizedFile,
> + MachOFile &file,
> + mach_o::ArchHandler &handler,
> + const Section *ehFrameSection,
> + MachODefinedAtom *atom,
> + uint64_t offset,
> + const AugmentationDataInfo &adi) {
> +
> + const bool isBig =
MachOLinkingContext::isBigEndian(normalizedFile.arch);
> + const bool is64 = MachOLinkingContext::is64Bit(normalizedFile.arch);
> +
> + // Compiler wasn't lazy and actually told us what it meant.
> + if (atom->begin() != atom->end())
> + return std::error_code();
> +
> + const uint8_t *frameData = atom->rawContent().data();
> + uint32_t size = read32(frameData, isBig);
> + uint64_t cieFieldInFDE = size == 0xffffffffU
> + ? sizeof(uint32_t) + sizeof(uint64_t)
> + : sizeof(uint32_t);
> +
> + // Linker needs to fixup a reference from the FDE to its parent CIE (a
> + // 32-bit byte offset backwards in the __eh_frame section).
> + uint32_t cieDelta = read32(frameData + cieFieldInFDE, isBig);
> + uint64_t cieAddress = ehFrameSection->address + offset + cieFieldInFDE;
> + cieAddress -= cieDelta;
> +
> + Reference::Addend addend;
> + const Atom *cie =
> + findAtomCoveringAddress(normalizedFile, file, cieAddress, &addend);
> + atom->addReference(cieFieldInFDE, handler.unwindRefToCIEKind(), cie,
> + addend, handler.kindArch());
> +
> + // Linker needs to fixup reference from the FDE to the function it's
> + // describing. FIXME: there are actually different ways to do this,
and the
> + // particular method used is specified in the CIE's augmentation fields
> + // (hopefully)
> + uint64_t rangeFieldInFDE = cieFieldInFDE + sizeof(uint32_t);
> +
> + int64_t functionFromFDE = readSPtr(is64, isBig,
> + frameData + rangeFieldInFDE);
> + uint64_t rangeStart = ehFrameSection->address + offset +
rangeFieldInFDE;
> + rangeStart += functionFromFDE;
> +
> + const Atom *func =
> + findAtomCoveringAddress(normalizedFile, file, rangeStart, &addend);
> + atom->addReference(rangeFieldInFDE, handler.unwindRefToFunctionKind(),
> + func, addend, handler.kindArch());
> +
> + // Handle the augmentation data if there is any.
> + if (adi.present) {
> + // First process the augmentation data length field.
> + uint64_t AugmentationDataLengthFieldInFDE =
> + rangeFieldInFDE + 2 * (is64 ? sizeof(uint64_t) : sizeof(uint32_t));
> + unsigned LengthFieldSize = 0;
> + uint64_t AugmentationDataLength =
> + llvm::decodeULEB128(frameData + AugmentationDataLengthFieldInFDE,
> + &LengthFieldSize);
lengthFieldSize
ArgumentationDataLength
> +
> + if (adi.mayHaveLSDA && AugmentationDataLength > 0) {
> +
> + // Look at the augmentation data field.
> + uint64_t AugmentationDataFieldInFDE =
> + AugmentationDataLengthFieldInFDE + LengthFieldSize;
> +
> + int64_t lsdaFromFDE = readSPtr(is64, isBig,
> + frameData +
AugmentationDataFieldInFDE);
> + uint64_t lsdaStart =
> + ehFrameSection->address + offset + AugmentationDataFieldInFDE +
> + lsdaFromFDE;
> + const Atom *lsda =
> + findAtomCoveringAddress(normalizedFile, file, lsdaStart,
&addend);
> + atom->addReference(AugmentationDataFieldInFDE,
> + handler.unwindRefToFunctionKind(),
> + lsda, addend, handler.kindArch());
> + }
> + }
> +
> + return std::error_code();
> +}
> +
> std::error_code addEHFrameReferences(const NormalizedFile
&normalizedFile,
> MachOFile &file,
> mach_o::ArchHandler &handler) {
> - const bool isBig =
MachOLinkingContext::isBigEndian(normalizedFile.arch);
> - const bool is64 = MachOLinkingContext::is64Bit(normalizedFile.arch);
>
> const Section *ehFrameSection = nullptr;
> for (auto §ion : normalizedFile.sections)
> @@ -662,51 +803,27 @@
> if (!ehFrameSection)
> return std::error_code();
>
> + std::error_code ehFrameErr;
> + AugmentationDataInfo adi;
> +
> file.eachAtomInSection(*ehFrameSection,
> [&](MachODefinedAtom *atom, uint64_t offset) ->
void {
> assert(atom->contentType() == DefinedAtom::typeCFI);
>
> - if (ArchHandler::isDwarfCIE(isBig, atom))
> + // Bail out if we've encountered an error.
> + if (ehFrameErr)
> return;
>
> - // Compiler wasn't lazy and actually told us what it meant.
> - if (atom->begin() != atom->end())
> - return;
> + const bool isBig =
MachOLinkingContext::isBigEndian(normalizedFile.arch);
> + if (ArchHandler::isDwarfCIE(isBig, atom)) {
> + adi = AugmentationDataInfo();
> + ehFrameErr = processCIE(normalizedFile, atom, adi);
> + } else
> + ehFrameErr = processFDE(normalizedFile, file, handler,
ehFrameSection,
> + atom, offset, adi);
> + });
>
> - const uint8_t *frameData = atom->rawContent().data();
> - uint32_t size = read32(frameData, isBig);
> - uint64_t cieFieldInFDE = size == 0xffffffffU
> - ? sizeof(uint32_t) + sizeof(uint64_t)
> - : sizeof(uint32_t);
> -
> - // Linker needs to fixup a reference from the FDE to its parent CIE
(a
> - // 32-bit byte offset backwards in the __eh_frame section).
> - uint32_t cieDelta = read32(frameData + cieFieldInFDE, isBig);
> - uint64_t cieAddress = ehFrameSection->address + offset +
cieFieldInFDE;
> - cieAddress -= cieDelta;
> -
> - Reference::Addend addend;
> - const Atom *cie =
> - findAtomCoveringAddress(normalizedFile, file, cieAddress,
&addend);
> - atom->addReference(cieFieldInFDE, handler.unwindRefToCIEKind(), cie,
> - addend, handler.kindArch());
> -
> - // Linker needs to fixup reference from the FDE to the function it's
> - // describing. FIXME: there are actually different ways to do this,
and the
> - // particular method used is specified in the CIE's augmentation
fields
> - // (hopefully)
> - uint64_t rangeFieldInFDE = cieFieldInFDE + sizeof(uint32_t);
> -
> - int64_t functionFromFDE = readSPtr(is64, isBig, frameData +
rangeFieldInFDE);
> - uint64_t rangeStart = ehFrameSection->address + offset +
rangeFieldInFDE;
> - rangeStart += functionFromFDE;
> -
> - const Atom *func =
> - findAtomCoveringAddress(normalizedFile, file, rangeStart,
&addend);
> - atom->addReference(rangeFieldInFDE,
handler.unwindRefToFunctionKind(), func,
> - addend, handler.kindArch());
> - });
> - return std::error_code();
> + return ehFrameErr;
> }
>
>
> Index: test/mach-o/parse-eh-frame-relocs-x86_64.yaml
> ===================================================================
> --- test/mach-o/parse-eh-frame-relocs-x86_64.yaml (revision 0)
> +++ test/mach-o/parse-eh-frame-relocs-x86_64.yaml (working copy)
> @@ -0,0 +1,107 @@
> +# RUN: lld -flavor darwin -arch x86_64 -r -print_atoms %s -o %t |
FileCheck %s
> +#
> +# Test parsing of x86_64 __eh_frame (dwarf unwind) relocations.
> +#
> +#_catchMyException:
> +# pushq %rbp
> +# movq %rsp, %rbp
> +# callq _foo
> +# popq %rbp
> +# retq
> +# movq %rax, %rdi
> +# callq ___cxa_begin_catch
> +# popq %rbp
> +# jmp ___cxa_end_catch
> +
> +--- !mach-o
> +arch: x86_64
> +file-type: MH_OBJECT
> +flags: [ ]
> +sections:
> + - segment: __TEXT
> + section: __text
> + type: S_REGULAR
> + attributes: [ S_ATTR_PURE_INSTRUCTIONS,
S_ATTR_SOME_INSTRUCTIONS ]
> + address: 0x0000000000000000
> + content: [0x55, 0x48, 0x89, 0xe5, 0xe8, 0x00, 0x00, 0x00,
> + 0x00, 0x5d, 0xc3, 0x48, 0x89, 0xc7, 0xe8, 0x00,
> + 0x00, 0x00, 0x00, 0x5d, 0xe9, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00 ]
> + relocations:
> + - offset: 0x00000015
> + type: X86_64_RELOC_BRANCH
> + length: 2
> + pc-rel: true
> + extern: true
> + symbol: 2
> + - offset: 0x0000000f
> + type: X86_64_RELOC_BRANCH
> + length: 2
> + pc-rel: true
> + extern: true
> + symbol: 1
> + - offset: 0x00000005
> + type: X86_64_RELOC_BRANCH
> + length: 2
> + pc-rel: true
> + extern: true
> + symbol: 0
> + - segment: __TEXT
> + section: __gcc_except_tab
> + type: S_REGULAR
> + attributes: [ ]
> + address: 0x000000000000001c
> + content: [ 0x00, 0x00, 0x00, 0x00 ]
> + - segment: __TEXT
> + section: __eh_frame
> + type: S_COALESCED
> + attributes: [ ]
> + address: 0x0000000000000020
> + content: [ 0x1c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x01, 0x7a, 0x50, 0x4c, 0x52, 0x00, 0x01, 0x78,
> + 0x10, 0x07, 0x9b, 0x04, 0x00, 0x00, 0x00, 0x10,
> + 0x10, 0x0c, 0x07, 0x08, 0x90, 0x01, 0x00, 0x00,
> + 0x2c, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
> + 0xB8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> + 0x19, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x08, 0xc3, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> + 0xff, 0x41, 0x0e, 0x10, 0x86, 0x02, 0x43, 0x0d,
> + 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 ]
> + - segment: __DATA
> + section: __data
> + type: S_REGULAR
> + attributes: [ ]
> + address: 0x0000000000000068
> + content: [ 0x00, 0x00, 0x00, 0x00 ]
> +local-symbols:
> + - name: _catchMyException
> + type: N_SECT
> + sect: 1
> + value: 0x0000000000000000
> +undefined-symbols:
> + - name: _foo
> + type: N_UNDF
> + scope: [ N_EXT ]
> + value: 0x0000000000000000
> + - name: ___cxa_begin_catch
> + type: N_UNDF
> + scope: [ N_EXT ]
> + value: 0x0000000000000000
> + - name: ___cxa_end_catch
> + type: N_UNDF
> + scope: [ N_EXT ]
> + value: 0x0000000000000000
> +...
> +
> +# CHECK: - type: unwind-cfi
> +# CHECK-NOT: - type:
> +# CHECK: references:
> +# CHECK-NEXT: - kind: negDelta32
> +# CHECK-NEXT: offset: 4
> +# CHECK-NEXT: target: L000
> +# CHECK-NEXT: - kind: unwindFDEToFunction
> +# CHECK-NEXT: offset: 8
> +# CHECK-NEXT: target: _catchMyException
> +# CHECK-NEXT: - kind: unwindFDEToFunction
> +# CHECK-NEXT: offset: 25
> +# CHECK-NEXT: target: L001
On Tue, May 12, 2015 at 12:44 PM, Lang Hames <lhames at gmail.com> wrote:
> Hi All,
>
> The LSDA field of MachO eh-frames wasn't being properly relocated, and
> this was causing failures on some EH tests in the nightly test suite. This
> patch properly relocates the LSDA field, fixing those tests.
>
> Cheers,
> Lang.
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150512/4b126d10/attachment.html>
More information about the llvm-commits
mailing list