Page MenuHomePhabricator

[yaml2obj][ELF] Make symbols optional for relocations
ClosedPublic

Authored by jakehehrlich on Aug 29 2017, 2:27 PM.

Details

Summary

Some kinds of relocations do not have symbols, like R_X86_64_RELATIVE for instance. I would like to test this case in D36554 but currently can't because symbols are required by yaml2obj. The other option is using the empty symbol but that doesn't seem quite right to me.

This change makes the Symbol field of Relocation optional and in the case where the user does not specify a symbol name the Symbol index is 0.

Diff Detail

Repository
rL LLVM

Event Timeline

jakehehrlich created this revision.Aug 29 2017, 2:27 PM
jakehehrlich added a subscriber: llvm-commits.
silvas edited edge metadata.Aug 29 2017, 9:24 PM

Seems pretty straightforward. LGTM.

Also it's probably good to add a test documenting what happens when you have a relocation that does need a symbol but you don't provide one.

Added a test which documents the invalid relocations that can now be produced.

silvas accepted this revision.Aug 30 2017, 2:16 PM
silvas added inline comments.
test/tools/yaml2obj/invalid-symboless-relocation.yaml
1 ↗(On Diff #113285)

small wording nit: "should succeed" -> "succeeds", otherwise it isn't clear whether "should" implies "we don't do it now, but it should" and requires some context to disambiguate.

This revision is now accepted and ready to land.Aug 30 2017, 2:16 PM

Updated. Should be good to land.

This revision was automatically updated to reflect the committed changes.