Page MenuHomePhabricator

Allow yaml2obj to order implicit sections for ELF
ClosedPublic

Authored by kastiglione on Nov 7 2017, 10:52 AM.

Details

Summary

This change allows yaml input to control the order of implicitly added sections
(.symtab, .strtab, .shstrtab). The order is controlled by adding a
placeholder section of the given name to the Sections field.

This change is to support changes in D39582, where it is desirable to control
the location of the .dynsym section.

Diff Detail

Repository
rL LLVM

Event Timeline

kastiglione created this revision.Nov 7 2017, 10:52 AM
jakehehrlich edited edge metadata.Nov 7 2017, 11:32 AM

Overall looks good to me. I just have one nit about the lookup function.

tools/yaml2obj/yaml2elf.cpp
78 ↗(On Diff #121936)

I think you should use an Optional here instead of returning a maximal value. Also can you implement one of the lookup functions in terms of the other to dedup code?

Rename lookup to get and make it assert when key not found

kastiglione added inline comments.Nov 7 2017, 1:35 PM
tools/yaml2obj/yaml2elf.cpp
78 ↗(On Diff #121936)

I deduped, and instead of returning an optional, I added an assert. The intended semantics of this function is it's called only for inputs known to exist. The result should never be a none value. Thoughts?

This revision is now accepted and ready to land.Nov 7 2017, 1:45 PM
kastiglione marked an inline comment as done.Nov 7 2017, 1:57 PM
This revision was automatically updated to reflect the committed changes.
kastiglione added inline comments.Nov 7 2017, 2:29 PM
llvm/trunk/tools/yaml2obj/yaml2elf.cpp
79–81

The uninitialized Idx broke some builds. What's the expedient way to fix this, another diff?

kastiglione added inline comments.Nov 7 2017, 2:34 PM
llvm/trunk/tools/yaml2obj/yaml2elf.cpp
79–81

Addressed in r317626.