This is an archive of the discontinued LLVM Phabricator instance.

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

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

Details

Summary

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
(1) https://groups.google.com/forum/#!topic/generic-abi/-XJAV5d8PRg
(2) DT_SYMTAB_SHNDX: http://www.sco.com/developers/gabi/latest/ch5.dynamic.html

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
test/tools/obj2yaml/elf-sht-symtab-shndx.yaml
116

The canonical name is .symtab_shndx

140

.symtab_shndx1

145

.symtab_shndx2

166

`.symtab_shndx

189

.symtab_shndx

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
test/tools/obj2yaml/elf-sht-symtab-shndx.yaml
21

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
  Type: SHT_SYMTAB_SHNDX
  Link: .symtab
  Entries:
    - Value: 0
    - Value: 1
    - Value: 42
51

dump the object that -> dump an object, which

(same throughout the test)

70

contain -> contains

103

TODO -> FIXME

125

Remove "and"

151

is not associated

174

with the/a

test/tools/yaml2obj/elf-sht-symtab-shndx.yaml
1

remove "the"

25–26

index. It

29

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

63

delete "the"

64

missing ')'

index of a section

65

than the total

90

if a symbol index

tools/obj2yaml/elf2yaml.cpp
180

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.
test/tools/obj2yaml/elf-sht-symtab-shndx.yaml
21

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

tools/obj2yaml/elf2yaml.cpp
180

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
test/tools/obj2yaml/elf-sht-symtab-shndx.yaml
21

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.
test/tools/obj2yaml/elf-sht-symtab-shndx.yaml
21

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

test/tools/yaml2obj/elf-sht-symtab-shndx.yaml
29

Yes, done.

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

LGTM, with one nit.

test/tools/yaml2obj/elf-sht-symtab-shndx.yaml
39

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