[PATCH] D107955: [ELF] Use SHF_SUNW_NODISCARD instead of SHF_GNU_RETAIN on Solaris

[Rainer Orth via Phabricator via llvm-commits]

ro marked an inline comment as done.
ro added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/ELF/retain-section.yaml:1
+## Check how yaml2obj handles retain (SHF_GNU_RETAIN and
+## SHF_SUNW_NODISCARD) section flags.
----------------
jhenderson wrote:
> It might be interesting to show what happens if you use SHF_GNU_RETAIN on Solaris and SHF_SUNW_NODISCARD on non-Solaris.
Excellent point: the first silently creates what Solaris `elfdump` identifies as `SHF_SUNW_ABSENT`, but should be rejected.  The second at least errors out:
```
YAML:68:14: error: unknown bit value
    Flags: [ SHF_SUNW_NODISCARD ]
             ^~~~~~~~~~~~~~~~~~~
```
although the error message could be clearer.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1249
+const EnumEntry<unsigned> ElfGNUSectionFlags[] = {
+  ENUM_ENT(SHF_GNU_RETAIN,       "R"),
+};
----------------
jhenderson wrote:
> ro wrote:
> > MaskRay wrote:
> > > Fix clang-format lints
> > I'd rather not: `clang-format-diff.py` makes a total mess here: with LLVM 14 `clang-format`, the block for the gABI flags is changed like this:
> > ```
> >  const EnumEntry<unsigned> ElfSectionFlags[] = {
> > -  ENUM_ENT(SHF_WRITE,            "W"),
> > -  ENUM_ENT(SHF_ALLOC,            "A"),
> > -  ENUM_ENT(SHF_EXECINSTR,        "X"),
> > -  ENUM_ENT(SHF_MERGE,            "M"),
> > -  ENUM_ENT(SHF_STRINGS,          "S"),
> > -  ENUM_ENT(SHF_INFO_LINK,        "I"),
> > -  ENUM_ENT(SHF_LINK_ORDER,       "L"),
> > -  ENUM_ENT(SHF_OS_NONCONFORMING, "O"),
> > -  ENUM_ENT(SHF_GROUP,            "G"),
> > -  ENUM_ENT(SHF_TLS,              "T"),
> > -  ENUM_ENT(SHF_COMPRESSED,       "C"),
> > -  ENUM_ENT(SHF_EXCLUDE,          "E"),
> > +    ENUM_ENT(SHF_WRITE, "W"),      ENUM_ENT(SHF_ALLOC, "A"),
> > +    ENUM_ENT(SHF_EXECINSTR, "X"),  ENUM_ENT(SHF_MERGE, "M"),
> > +    ENUM_ENT(SHF_STRINGS, "S"),    ENUM_ENT(SHF_INFO_LINK, "I"),
> > +    ENUM_ENT(SHF_LINK_ORDER, "L"), ENUM_ENT(SHF_OS_NONCONFORMING, "O"),
> > +    ENUM_ENT(SHF_GROUP, "G"),      ENUM_ENT(SHF_TLS, "T"),
> > +    ENUM_ENT(SHF_COMPRESSED, "C"), ENUM_ENT(SHF_EXCLUDE, "E"),
> >  };
> > ```
> > and the next two indented further, making them inconsistent with the following ones.
> You should be able to format the new enum entry lists at least though.
Right, fixed.  Indentation is still confusing, though: aligned for e.g. `ElfSectionFlags`, just one space for the single-entry arrays.  Oh well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107955/new/

https://reviews.llvm.org/D107955