This is an archive of the discontinued LLVM Phabricator instance.

[yaml2obj] - Allow setting custom section types for implicit sections.
ClosedPublic

Authored by grimar on Jun 13 2019, 7:07 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Jun 13 2019, 7:07 AM
jhenderson added inline comments.Jun 13 2019, 7:56 AM
test/tools/llvm-readobj/elf-wrong-shstrtab-type.test
1–2 ↗(On Diff #204530)

Change this comment to:
"Check we do not fail to dump the section headers when a .shstrtab section does not have a SHT_STRTAB type."

test/tools/yaml2obj/elf-symtab-shinfo.yaml
27–37 ↗(On Diff #204530)

This change doesn't seem related?

test/tools/yaml2obj/explicit-dynsym-no-dynstr.yaml
22 ↗(On Diff #204530)

Unrelated?

test/tools/yaml2obj/implicit-sections-types.test
4 ↗(On Diff #204530)

flags -> types

4–5 ↗(On Diff #204530)

restructure the second half of this sentence to:

"in case sections were implicitly added and not described in the YAML"

32 ↗(On Diff #204530)

flags -> types

59 ↗(On Diff #204530)

It might be nice if this picked a range of section types (e.g. SHT_RELA, SHT_DYNAMIC etc).

tools/yaml2obj/yaml2elf.cpp
471 ↗(On Diff #204530)

Is the cast needed? It isn't above.

grimar updated this revision to Diff 204737.Jun 14 2019, 3:41 AM
grimar marked 11 inline comments as done.
  • Addressed review comments.
test/tools/yaml2obj/elf-symtab-shinfo.yaml
27–37 ↗(On Diff #204530)

It is related. This test has a following check lines:

# CHECK:      Name: .dynsym
# CHECK-NEXT: Type: SHT_DYNSYM

We set SHT_DYNSYM by default before this patch.
But YAML seems contains SHT_SYMTAB by mistake.

This patch revealed this errors in this and other test cases, so I fixed them.

test/tools/yaml2obj/explicit-dynsym-no-dynstr.yaml
22 ↗(On Diff #204530)

The same.

tools/yaml2obj/yaml2elf.cpp
471 ↗(On Diff #204530)

Nope. Removed.

jhenderson accepted this revision.Jun 14 2019, 4:24 AM

LGTM, with one nit.

test/tools/yaml2obj/implicit-sections-types.test
5 ↗(On Diff #204737)

Missing trailing full stop.

This revision is now accepted and ready to land.Jun 14 2019, 4:24 AM
This revision was automatically updated to reflect the committed changes.
grimar marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptJun 14 2019, 5:15 AM