This is an archive of the discontinued LLVM Phabricator instance.

[yaml2obj] - Introduce the "NoHeaders" key for "SectionHeaderTable"
ClosedPublic

Authored by grimar on Jun 11 2020, 7:21 AM.

Details

Summary

We have an issue currently. The following YAML piece just ignores the Excluded key.

SectionHeaderTable:
  Sections: []
  Excluded:
    - Name: .foo

Currently the meaning is: exclude the whole table.

The code checks that the Sections key is empty and doesn't catch/check
invalid/duplicated/missed Excluded entries.

Also there is no way to exclude all sections except the first null section,
because Sections: [] currently just excludes the whole the sections header table.

To fix it, I suggest a change of the behavior.

  1. A new NoHeaders key is added. It provides an explicit syntax to drop the whole table.
  2. The meaning of the following is changed:
SectionHeaderTable:
  Sections: []
  Excluded:
    - Name: .foo

Assuming there are 2 sections in the object (a null section and .foo), with this patch it
means: exclude the .foo section, keep the null section. The null section is an implicit
section and I think it is reasonable to make "Sections: []" to mean it is implicitly added.
It will be consistent with the global "Sections" tag that is used to describe sections.

  1. SectionHeaderTable->Sections is now optional. No Sections is the same as Sections: [] (I think it avoids a confusion).
  2. Using of NoHeaders together with Sections/Excluded is not allowed.
  3. It is possible to use the Excluded key without the Sections key now (in this case Excluded must contain all sections).
  4. SectionHeaderTable: or SectionHeaderTable: [] is not allowed.
  5. When the SectionHeaderTable key is present, we still require all sections to be present in Sections and Excluded lists. No changes here, we are still strict.

Diff Detail

Event Timeline

grimar created this revision.Jun 11 2020, 7:21 AM
Herald added a project: Restricted Project. · View Herald Transcript
grimar added a comment.EditedJun 11 2020, 7:23 AM

Currently the test\ObjectYAML\MachO\DWARF-pubsections.yaml
fails with it (--docnum=2)

II happens because of the bug in ObjectYAML\MachOEmitter.cpp

It does:

IO.mapOptional("debug_pubnames", DWARF.PubNames);
IO.mapOptional("debug_pubtypes", DWARF.PubTypes);

where

Optional<PubSection> PubNames;
Optional<PubSection> PubTypes;

And since this patch fixes an issue mentioned in the description
(in YAMLTraits.cpp), the following code does not work anymore:

if (Obj.DWARF.PubNames)
  Err = DWARFYAML::emitPubSection(OS, *Obj.DWARF.PubNames, Obj.IsLittleEndian);

because Obj.DWARF.PubNames was never Optional::None previously, but now it is.
Seems previously it always wrote a piece of broken data.

I am going to investigate if I can fix it separatelly from this patch.

grimar updated this revision to Diff 270336.Jun 12 2020, 2:18 AM
  • Rebased. Now all tests pass.
grimar edited the summary of this revision. (Show Details)Jun 12 2020, 2:18 AM
jhenderson accepted this revision.Jun 15 2020, 12:23 AM

Some mostly comment nits, otherwise LGTM.

llvm/test/tools/yaml2obj/ELF/section-headers-exclude.yaml
80

section with -> section, with

llvm/test/tools/yaml2obj/ELF/section-headers.yaml
82–102

an empty section header table.

86–87

Can these be -NEXT?

101

section header -> an empty section header table

to match above comment.

This revision is now accepted and ready to land.Jun 15 2020, 12:23 AM
This revision was automatically updated to reflect the committed changes.
grimar marked 4 inline comments as done.