[PATCH] D81655: [yaml2obj] - Introduce the "NoHeaders" key for "SectionHeaderTable"

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
grimar added reviewers: jhenderson, MaskRay.
Herald added subscribers: hiraditya, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.
grimar added a comment.

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.


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.

3. `SectionHeaderTable->Sections` is now optional. No `Sections` is the same as `Sections: []` (I think it avoids a confusion).
4. Using of `NoHeaders` together with `Sections`/`Excluded` is not allowed.
5. It is possible to use the `Excluded` key without the `Sections` key now (in this case `Excluded` must contain all sections).
6. `SectionHeaderTable:` or `SectionHeaderTable: []` is not allowed.
7. 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.

Note: I am also fixing an issue in YAMLTraits.cpp here. Without it, when there is no "Sections",
the following code "Optional<std::vector<SectionHeader>> Section" assigns an empty vector
instead of Optional::None to the "Section" member.


https://reviews.llvm.org/D81655

Files:
  llvm/include/llvm/ObjectYAML/ELFYAML.h
  llvm/lib/ObjectYAML/ELFEmitter.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/YAMLTraits.cpp
  llvm/test/tools/yaml2obj/ELF/section-headers-exclude.yaml
  llvm/test/tools/yaml2obj/ELF/section-headers.yaml

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81655.270133.patch
Type: text/x-patch
Size: 19131 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200611/b13d6759/attachment.bin>