Page MenuHomePhabricator

[ELF] Treat dynamic tag values as virtual addresses instead of offsets
ClosedPublic

Authored by wolfgangp on Jun 6 2019, 12:12 PM.

Details

Summary

Fixes PR41832.
The ELF gABI requires the tag values of DT_REL, DT_RELA and DT_JMPREL to be treated as virtual addresses (d_ptr). They were treated as offsets.

Diff Detail

Repository
rL LLVM

Event Timeline

wolfgangp created this revision.Jun 6 2019, 12:12 PM
jhenderson added inline comments.
test/tools/llvm-objdump/X86/elf-dynamic-relocs.test
1 ↗(On Diff #203414)

Move this line to immediately before the rest of the YAML.

2–4 ↗(On Diff #203414)

In many of the newer tests in the llvm tools, we've been using '##' to distinguish comments from RUN/CHECK commands. It might be nice if you added those.

9 ↗(On Diff #203414)

As this is the first and only test for --dynamic-relocs, could you check the full output here (including relevant headers etc if any), please.

Also, you should make your checks CHECK-NEXT, and add something to show that there are no other relocations printed (in case later lines produce garbage or similar). This might be most easily achieved by doing an --implicit-check-not=R_X86 or similar.

11 ↗(On Diff #203414)

RELATIVE + symbol doesn't make sense. Probably just change the type to something else (e.g. R_X86_64_NONE).

27 ↗(On Diff #203414)

Nit, this is formatted differently to [SHF_ALLOC] above (i.e. spacing is different). Please could you make it consistent.

29 ↗(On Diff #203414)

I'm not sure an AddressAlign of 0x1000 makes sense if the address is only aligned to 0x100. Not sure it matters that much, but it might be nice to avoid surprises. Same comment applies in other places later on.

47 ↗(On Diff #203414)

See above re. formatting.

50 ↗(On Diff #203414)

Nit: double-space between '2' and '#'

wolfgangp updated this revision to Diff 203583.Jun 7 2019, 11:04 AM
wolfgangp marked 8 inline comments as done.

Addressed review comments as suggested.

This revision is now accepted and ready to land.Jun 10 2019, 2:47 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2019, 10:47 AM