This is an archive of the discontinued LLVM Phabricator instance.

[yaml2obj][obj2yaml] - Support SHT_GNU_versym (.gnu.version) section.
ClosedPublic

Authored by grimar on Feb 15 2019, 5:29 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Feb 15 2019, 5:29 AM
jhenderson added inline comments.Feb 18 2019, 9:20 AM
test/tools/obj2yaml/versym-section.yaml
18 ↗(On Diff #186999)

Should this have a link field? As far as I can see, the field is not specified in the specification.

grimar marked 2 inline comments as done.Feb 19 2019, 12:55 AM
grimar added inline comments.
test/tools/obj2yaml/versym-section.yaml
18 ↗(On Diff #186999)

Yes, it should:
sh_link here is a "The section header index of the associated symbol table. "
according to generic specification about the special sections:
(https://docs.oracle.com/cd/E19683-01/816-1386/chapter6-94076/index.html,
Table 7-15 ELF sh_link and sh_info Interpretation)

jhenderson accepted this revision.Feb 19 2019, 2:08 AM

LGTM, with two small comments.

tools/yaml2obj/yaml2elf.cpp
563 ↗(On Diff #186999)

I don't know what the type here is, so best not to use auto, I think.

565 ↗(On Diff #186999)

Do you need this cast here? It looks superfluous.

This revision is now accepted and ready to land.Feb 19 2019, 2:08 AM
grimar marked 3 inline comments as done.Feb 19 2019, 2:13 AM
grimar added inline comments.
tools/yaml2obj/yaml2elf.cpp
563 ↗(On Diff #186999)

Generally, I follow the same rule, but the whole file use auto, so I did that for consistency.
Perhaps it is a good idea to fix it though.

565 ↗(On Diff #186999)

Yep :(, MSVS2017 complains:

error C2440: 'initializing': cannot convert from 'uint16_t' to 'llvm::support::detail::packed_endian_specific_integral<uint16_t,llvm::support::little,1>'

jhenderson added inline comments.Feb 19 2019, 2:17 AM
tools/yaml2obj/yaml2elf.cpp
565 ↗(On Diff #186999)

Sounds to me like there's a missing conversion constructor or similar in that type then...

grimar marked an inline comment as done.Feb 19 2019, 6:26 AM
grimar added inline comments.
tools/yaml2obj/yaml2elf.cpp
565 ↗(On Diff #186999)

I think it would be a bit simpler to do something like:

for (uint16_t V : Section.Entries)
  support::endian::write<uint16_t>(OS, V, ELFT::TargetEndianness);

(Elf_Half is always 2 bytes).
It seems can simplify/cleanup a few other methods of this file too.
I'll take a look.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2019, 7:28 AM