This is an archive of the discontinued LLVM Phabricator instance.

[Object] Implement relocation resolver for COFF ARM/ARM64
ClosedPublic

Authored by mstorsjo on Sep 9 2019, 12:39 AM.

Details

Summary

This is primarily used for reading debug info from unrelocated object files, for handling addr and secrel relocations.

Also add testcases for the existing code for COFF on X86.

@JDevlieghere - I'm adding tests for this under tools/llvm-dwarfdump. Currently there's two subdirs, X86 and AArch64 there, while I'm adding four testcases (coff-i386, coff-x86_64, coff-arm and coff-arm64). Are you ok with them placed in the toplevel dir, or do you want a different structure for them?

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.Sep 9 2019, 12:39 AM
Herald added a project: Restricted Project. · View Herald Transcript
ruiu added inline comments.Sep 9 2019, 1:20 AM
lib/Object/RelocationResolver.cpp
443 ↗(On Diff #219290)

Is it OK to silently wrap-around an overflowed value? I wonder if we should report an error.

mstorsjo marked an inline comment as done.Sep 9 2019, 1:24 AM
mstorsjo added inline comments.
lib/Object/RelocationResolver.cpp
443 ↗(On Diff #219290)

Not sure if the base values will be in range where overflow might even be expected.

In any case, this is the exact same existing code matching resolveCOFFX86 and resolveCOFFX86_64 (see the context of the diff), just with different arch-specific relocation names.

ruiu accepted this revision.Sep 9 2019, 1:29 AM

LGTM

lib/Object/RelocationResolver.cpp
443 ↗(On Diff #219290)

Ah, OK. There precedents there in this file indeed.

This revision is now accepted and ready to land.Sep 9 2019, 1:29 AM
MaskRay accepted this revision.Sep 9 2019, 1:52 AM
MaskRay added inline comments.
lib/Object/RelocationResolver.cpp
443 ↗(On Diff #219290)

On DWARF side, error reporting should be possible now after D63713 added Error *Err to DWARFDataExtractor::getRelocatedValue.

I'll take a stab at improving it. (In any case, this change should not be blocked by that change but I hope @mstorsjo can add test cases about relocation overflows when that is done)

MaskRay added inline comments.Sep 9 2019, 2:35 AM
lib/Object/RelocationResolver.cpp
443 ↗(On Diff #219290)

D67343 [DebugInfo] Change object::RelocationResolver to return Expected<uint64_t>

This revision was automatically updated to reflect the committed changes.