Page MenuHomePhabricator

[yaml2obj/obj2yaml] - Add a basic support for extended section indexes.

Authored by grimar on Jul 30 2019, 6:20 AM.



In some cases a symbol might have section index == SHN_XINDEX.
This is an escape value indicating that the actual section header index
is too large to fit in the containing field.
Then the SHT_SYMTAB_SHNDX section is used. It contains the 32bit values
that stores section indexes.

ELF gABI says that there can be multiple SHT_SYMTAB_SHNDX sections,
i.e. for example one for .symtab and one for .dynsym

In this patch I am only supporting a single SHT_SYMTAB_SHNDX associated
with a .symtab. This is a more or less common case which is used a few tests I saw in LLVM.

I decided not to create the SHT_SYMTAB_SHNDX section as "implicit",
but implement is like a kind of regular section for now.
i.e. tools do not recreate this section or its content, like they do for
symbol table sections, for example. That should allow to write all kind of
possible broken test cases for our needs and keep the output closer to requested.

Diff Detail


Event Timeline

grimar created this revision.Jul 30 2019, 6:20 AM
MaskRay added inline comments.Aug 3 2019, 6:28 PM
115 ↗(On Diff #212328)

The canonical name is .symtab_shndx

139 ↗(On Diff #212328)


144 ↗(On Diff #212328)


165 ↗(On Diff #212328)


188 ↗(On Diff #212328)


grimar updated this revision to Diff 213336.Aug 5 2019, 5:51 AM
  • Addressed review comments (changed the section name).
jhenderson added inline comments.Aug 6 2019, 2:52 AM
21 ↗(On Diff #213336)

I think it would be clearer for obj2yaml to emit an array of entries (and yaml2obj to consume them). What do you think? (We should allow explicit EntSize/Content in the YAML to allow creation of broken stuff too).

- Name: .symtab_shndx
  Link: .symtab
    - Value: 0
    - Value: 1
    - Value: 42
51 ↗(On Diff #213336)

dump the object that -> dump an object, which

(same throughout the test)

70 ↗(On Diff #213336)

contain -> contains

103 ↗(On Diff #213336)


125 ↗(On Diff #213336)

Remove "and"

151 ↗(On Diff #213336)

is not associated

174 ↗(On Diff #213336)

with the/a

1 ↗(On Diff #213336)

remove "the"

25–26 ↗(On Diff #213336)

index. It

29 ↗(On Diff #213336)

You could use --section-data to print the bytes of the symbol table and check those maybe?

63 ↗(On Diff #213336)

delete "the"

64 ↗(On Diff #213336)

missing ')'

index of a section

65 ↗(On Diff #213336)

than the total

90 ↗(On Diff #213336)

if a symbol index

180 ↗(On Diff #213336)

I'm not sure what this comment is for? It doesn't seem really relevant for the code.

grimar marked 2 inline comments as done.Aug 6 2019, 2:57 AM
grimar added inline comments.
21 ↗(On Diff #213336)

Sounds good. What about me doing this in a follow-up or do you prefer to have this from the start?

180 ↗(On Diff #213336)

I am trying to say here that SHT_SYMTAB_SHNDX should be handled first, becase the code below (dumpSymbols)
might want to use ShndxTable. I'll reword.

jhenderson added inline comments.Aug 6 2019, 3:11 AM
21 ↗(On Diff #213336)

As obj2yaml should probably print the array version, that should be in first, otherwise you just have to update this test (but it's okay to then defer adding custom content to a later change if you want).

grimar updated this revision to Diff 213875.Aug 7 2019, 7:34 AM
grimar marked 17 inline comments as done.
  • Addressed review comments.
21 ↗(On Diff #213336)

I switched to Entries in this patch. This required adding a bit more code.

29 ↗(On Diff #213336)

Yes, done.

jhenderson accepted this revision.Aug 7 2019, 8:10 AM

LGTM, with one nit.

38 ↗(On Diff #213875)

This might be a Phabricator issue, but it looks like your arrow doesn't line up with the 0xFFFF?

This revision is now accepted and ready to land.Aug 7 2019, 8:10 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2019, 2:48 AM
Herald added a subscriber: rupprecht. · View Herald Transcript