This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Implemented support for R_386_PC8/R_386_8 relocations.
ClosedPublic

Authored by grimar on Dec 24 2016, 3:27 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 82439.Dec 24 2016, 3:27 AM
grimar retitled this revision from to [ELF] - Implemented support for R_386_PC8/R_386_8 relocations..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar, evgeny777.
davide added a subscriber: davide.Dec 24 2016, 4:12 AM

I think we can either implement or error out on these relocations, I personally don't care as implementing this relocations is literally 3 lines of code, but wait for a second opinion.
What I'm most worried about is that we should never produce a broken output given valid input, we should emit a diagnostic and exit.

ruiu edited edge metadata.Jan 3 2017, 3:57 PM

Somewhat orthogonal to this patch, but I think we need to modify X86TargetInfo::relocateOne so that we explicitly list relocation types that we understand and error out other relocations.

grimar abandoned this revision.Jan 11 2017, 12:41 AM

D28516 was landed instead.

grimar reclaimed this revision.Jan 24 2017, 5:03 AM

Reported to be used in linux kernel. Will rebase in a minute.

grimar updated this revision to Diff 85573.Jan 24 2017, 5:04 AM
  • Rebased.
grimar updated this revision to Diff 85589.Jan 24 2017, 8:24 AM
  • Addressed review comments.
ruiu added inline comments.Jan 24 2017, 10:55 AM
ELF/Target.cpp
493 ↗(On Diff #85589)

No such thing like a one byte little endian value as the endianness is inherently a concept when you have more than one byte, so don't use read<>. That's why we don't have read8{be,le}. Just dereference the uint8_t pointer.

grimar updated this revision to Diff 85706.Jan 25 2017, 12:53 AM
  • Addressed review comments.
ELF/Target.cpp
493 ↗(On Diff #85589)

Yep. Not sure why I wrote in that way.

grimar edited the summary of this revision. (Show Details)Jan 25 2017, 5:47 AM
This revision was automatically updated to reflect the committed changes.
ruiu added inline comments.Jan 25 2017, 9:09 AM
lld/trunk/ELF/Target.cpp
518

Do you need a cast?

lld/trunk/ELF/Target.cpp