This is an archive of the discontinued LLVM Phabricator instance.

[yaml2obj] - Add a way to set sh_entsize for relocation sections.
ClosedPublic

Authored by grimar on Jan 29 2020, 5:47 AM.

Details

Summary

We are missing ability to override the sh_entsize field for
SHT_REL[A] sections. It would be useful for writing test cases.

Diff Detail

Event Timeline

grimar created this revision.Jan 29 2020, 5:47 AM
Herald added a project: Restricted Project. · View Herald Transcript
MaskRay accepted this revision.Jan 29 2020, 12:24 PM
This revision is now accepted and ready to land.Jan 29 2020, 12:24 PM
jhenderson accepted this revision.Jan 30 2020, 1:12 AM

LGTM, with suggestions.

Idle thought: I wonder if it's worth introducing some kind of command-line macro option to yaml2obj? E.g. -DMACHINE=EM_X86_64 would replace Machine: MACHINE (or some similar syntax). There are a growing number of cases where we have two identical YAML blobs except for some minor difference.

llvm/test/tools/yaml2obj/ELF/reloc-sec-entry-size.yaml
1

set -> sets

32

sh_entsizes -> sh_entsize

('sh_entsize' is an adjective, so doesn't need pluralizing)

39

Check we can set sh_entsize fields to arbitrary values.

57

Same as above

64

Same as above

Idle thought: I wonder if it's worth introducing some kind of command-line macro option to yaml2obj? E.g. -DMACHINE=EM_X86_64 would replace Machine: MACHINE (or some similar syntax). There are a growing number of cases where we have two identical YAML blobs except for some minor difference.

Sounds good. I'd give it a try.

This revision was automatically updated to reflect the committed changes.

LGTM, with suggestions.

Idle thought: I wonder if it's worth introducing some kind of command-line macro option to yaml2obj? E.g. -DMACHINE=EM_X86_64 would replace Machine: MACHINE (or some similar syntax). There are a growing number of cases where we have two identical YAML blobs except for some minor difference.

LG

Idle thought: I wonder if it's worth introducing some kind of command-line macro option to yaml2obj? E.g. -DMACHINE=EM_X86_64 would replace Machine: MACHINE (or some similar syntax). There are a growing number of cases where we have two identical YAML blobs except for some minor difference.

Sounds good. I'd give it a try.

One more idea: what if we make FileHeader to be optional?
e.g. a valid YAML can be something like this:

--- !ELF
Sections:
.....
Symbols:
....

When there is no FileHeader, yaml2obj could require --platform=X option (or alike) to be specified.
It can be similar to linker's -m (emulation) option and can accept the same strings for X:

  std::pair<ELFKind, uint16_t> ret =
      StringSwitch<std::pair<ELFKind, uint16_t>>(s)
          .Cases("aarch64elf", "aarch64linux", "aarch64_elf64_le_vec",
                 {ELF64LEKind, EM_AARCH64})
          .Cases("armelf", "armelf_linux_eabi", {ELF32LEKind, EM_ARM})
...
          .Cases("elf_amd64", "elf_x86_64", {ELF64LEKind, EM_X86_64})
          .Case("elf_i386", {ELF32LEKind, EM_386})
          .Case("elf_iamcu", {ELF32LEKind, EM_IAMCU})
          .Default({ELFNoneKind, EM_NONE});

Perhaps we also could add --elf-type=X to switch the output type: ET_DYN/ET_REL/ET_EXEC
(and have something by default).

A benefit is that we often do not need or use fields of the file header in our tests.
Making it optional can reduce the YAML descriptions.

Idle thought: I wonder if it's worth introducing some kind of command-line macro option to yaml2obj? E.g. -DMACHINE=EM_X86_64 would replace Machine: MACHINE (or some similar syntax). There are a growing number of cases where we have two identical YAML blobs except for some minor difference.

Sounds good. I'd give it a try.

One more idea: what if we make FileHeader to be optional?
e.g. a valid YAML can be something like this:

--- !ELF
Sections:
.....
Symbols:
....

When there is no FileHeader, yaml2obj could require --platform=X option (or alike) to be specified.
It can be similar to linker's -m (emulation) option and can accept the same strings for X:

  std::pair<ELFKind, uint16_t> ret =
      StringSwitch<std::pair<ELFKind, uint16_t>>(s)
          .Cases("aarch64elf", "aarch64linux", "aarch64_elf64_le_vec",
                 {ELF64LEKind, EM_AARCH64})
          .Cases("armelf", "armelf_linux_eabi", {ELF32LEKind, EM_ARM})
...
          .Cases("elf_amd64", "elf_x86_64", {ELF64LEKind, EM_X86_64})
          .Case("elf_i386", {ELF32LEKind, EM_386})
          .Case("elf_iamcu", {ELF32LEKind, EM_IAMCU})
          .Default({ELFNoneKind, EM_NONE});

Perhaps we also could add --elf-type=X to switch the output type: ET_DYN/ET_REL/ET_EXEC
(and have something by default).

A benefit is that we often do not need or use fields of the file header in our tests.
Making it optional can reduce the YAML descriptions.

My concern with this approach is that we can keep on adding switches, with very little gain. Taken to an extreme, you could specify sections, symbols, relocations etc via command-line switches and then not have any YAML at all. I don't think that really helps readability. A generic default file header might be okay though, but what would the parameters be? X86_64? ARM? It probably doesn't matter too much, for the cases its used in, but it is perhaps something we should be aware of.