[llvm-readobj] Make -s and -t match llvm-readelf

[MaskRay]

Summary

llvm-readobj is an internal testing tool for binary formats. Its output and
command line options do not need to be stable. It isn't supposed to be part of a
build process.

llvm-readelf was created as a user-facing utility and its interface intends to
be compatible with GNU readelf (unless there are good reasons not to).

The two tools have mostly compatible options. -s and -t are noticeable
exceptions due to history. I think the cost of keeping the inconsistency overweighs the
little history-compatible benefit and hinders transition from cl::opt to
OptTable, so let's change it.

[rupprecht]

Seems fine to me, but can you mention it in the release notes? Even though it's primarily a testing tool, downstream users may use llvm-readobj in their testing.

[MaskRay]

add release notes

[jhenderson]

Seems reasonable to me too. Might be worth an llvm-dev heads-up? Up to you.

llvm/docs/ReleaseNotes.rst

167–168

I think this statement is a little unclear could cause some confusion. How about this: "The llvm-readobj short aliases -s (previously --sections) and -t (previously --syms) have been changed to --syms and --section-details respectively, to match llvm-readelf."

llvm/test/tools/llvm-readobj/ELF/merged.test

55

No need to change this test, I think. You still want -s in the list, and you already have -S there. You might also want to add -t, but it's not really importantn, in my opinion.

[MaskRay]

update release notes; update -aeWhSrnudlVgIS

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/merged.test

55

You've still got S twice in this list (at the end and immediately after the h).

[MaskRay]

llvm/test/tools/llvm-readobj/ELF/merged.test

55

The diagnostic is because --section-headers is specified twice. We need two -S to trigger the previous diagnostic. I'll keep one -s here.

55

Previously both -S and -s were aliases for --section-headers. That was how the diagnostic was triggered.

Keeping two -S can trigger the --section-headers. If I drop one the message will go away.

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/merged.test

55

Oh, sorry, misread what the test was doing, thanks!

llvm/docs/CommandGuide/llvm-readobj.rst

 .. option:: --needed-libs
 
  Display the needed libraries.
 
 .. option:: --relocations, --relocs, -r
 
  Display the relocation entries in the file.
 
-.. option:: --sections, --section-headers, -s, -S
+.. option:: --sections, --section-headers, -S
 
  Display all sections.
 
 .. option:: --section-data, --sd
 
  When used with :option:`--sections`, display section data for each section
  shown. This option has no effect for GNU style output.
 

 
  Display contents of the stackmap section.
 
 .. option:: --string-dump=<section[,section,...]>, -p
 
  Display the specified section(s) as a list of strings. ``section`` may be a
  section index or section name.
 
-.. option:: --symbols, --syms, -t
+.. option:: --symbols, --syms, -s
 
  Display the symbol table.
 
 .. option:: --unwind, -u
 
  Display unwind information.
 
 .. option:: --version

llvm/docs/ReleaseNotes.rst

 
 * Support for in-order processors has been added to ``llvm-mca``.
   (`D94928 <https://reviews.llvm.org/D94928>`_)
 
 * llvm-objdump supports ``-M {att,intel}`` now.
   ``--x86-asm-syntax`` is a deprecated internal option which will be removed in LLVM 14.0.0.
   (`D101695 <https://reviews.llvm.org/D101695>`_)
 
+* The llvm-readobj short aliases ``-s`` (previously ``--sections``) and ``-t``
+  (previously ``--syms``) have been changed to ``--syms`` and
+  ``--section-details`` respectively, to match llvm-readelf.
+  (`D105055 <https://reviews.llvm.org/D105055>`_)
+
 Changes to LLDB
 ---------------------------------
 
 Changes to Sanitizers
 ---------------------
 
 External Open Source Projects Using LLVM 13
 ===========================================

llvm/test/tools/llvm-readobj/ELF/merged.test

- Name:    .gnu.version
     Entries: [ 0, 1 ]
 DynamicSymbols:
   - Name:    foo
     Binding: STB_GLOBAL
 
 ## llvm-readobj does not support merged args, because it also supports some old
 ## flags (-st, -sd, etc.), and it would be confusing if only some merged args
 ## were supported.
-# RUN: not llvm-readobj -aeWhSrnudlVgIs %t.o 2>&1 | FileCheck %s --check-prefix=UNKNOWN
+# RUN: not llvm-readobj -aeWhSsrnudlVgIS %t.o 2>&1 | FileCheck %s --check-prefix=UNKNOWN
 
 # CHECK-NOT: Unknown command line argument
 # UNKNOWN:   for the --section-headers option: may only occur zero or one times!

llvm/test/tools/llvm-readobj/ELF/sections.test

 ## Check how llvm-readobj prints sections with --sections.
 ## We test the --sections flag for llvm-readelf in the gnu-sections.test.
 
 # RUN: yaml2obj %s -o %t64
 # RUN: llvm-readobj --sections %t64 | FileCheck %s --check-prefixes=ELF,ELF64
 # RUN: yaml2obj -DBITS=32 %s -o %t32
 # RUN: llvm-readobj --sections %t32 | FileCheck %s --check-prefixes=ELF,ELF32
 
 ## Check flag aliases produce identical output.
 # RUN: llvm-readobj --sections %t64 > %t64.llvm.sections
 # RUN: llvm-readobj -S %t64 > %t64.llvm.upper.s
 # RUN: cmp %t64.llvm.sections %t64.llvm.upper.s
-# RUN: llvm-readobj -s %t64 > %t64.llvm.lower.s
-# RUN: cmp %t64.llvm.sections %t64.llvm.lower.s
 # RUN: llvm-readobj --section-headers %t64 > %t64.llvm.section-headers
 # RUN: cmp %t64.llvm.sections %t64.llvm.section-headers
 
 # RUN: llvm-readobj --sections %t32 > %t32.llvm.sections
 # RUN: llvm-readobj -S %t32 > %t32.llvm.upper.s
 # RUN: cmp %t32.llvm.sections %t32.llvm.upper.s
-# RUN: llvm-readobj -s %t32 > %t32.llvm.lower.s
-# RUN: cmp %t32.llvm.sections %t32.llvm.lower.s
 # RUN: llvm-readobj --section-headers %t32 > %t32.llvm.section-headers
 # RUN: cmp %t32.llvm.sections %t32.llvm.section-headers
 
 # ELF:      Sections [
 # ELF-NEXT:   Section {
 # ELF-NEXT:     Index: 0
 # ELF-NEXT:     Name:  (0)
 # ELF-NEXT:     Type: SHT_NULL (0x0)

llvm/test/tools/llvm-readobj/ELF/symbols.test

 # SYMBOLS-GNU32-NEXT:     0: 00000000     0 NOTYPE  LOCAL  DEFAULT   UND 
 # SYMBOLS-GNU32-NEXT:     1: 00000001     0 NOTYPE  LOCAL  DEFAULT   UND foo
 # SYMBOLS-GNU32-NEXT:     2: 00000002     0 NOTYPE  LOCAL  DEFAULT   UND bar
 
 ## Case 2: Check flag aliases produce identical output
 # RUN: llvm-readobj --symbols %t64 > %t.symbols
 # RUN: llvm-readobj --syms %t64 > %t.syms
 # RUN: cmp %t.symbols %t.syms
-# RUN: llvm-readobj -t %t64 > %t.t
-# RUN: cmp %t.symbols %t.t
-# RUN: llvm-readelf -s --elf-output-style=LLVM %t64 > %t.lowers
-# RUN: cmp %t.symbols %t.lowers
+# RUN: llvm-readobj -s %t64 | diff %t.symbols -
+# RUN: llvm-readelf -s --elf-output-style=LLVM %t64 | diff %t.symbols -
 
 # RUN: llvm-readelf --symbols %t64 > %t.symbols.gnu
 # RUN: llvm-readelf --syms %t64 > %t.syms.gnu
 # RUN: cmp %t.symbols.gnu %t.syms.gnu
-
-## -s is an llvm-readobj option to dump sections.
-# RUN: llvm-readobj -s --elf-output-style=GNU %t64 | FileCheck %s --implicit-check-not="Symbol table"
+# RUN: llvm-readelf -s %t64 | diff %t.symbols.gnu -
 
 ## Case 3: Test that both regular and dynamic symbols are dumped when `--symbols` and `--dyn-symbols`
 ##         are specified together. Note that the order is different for different styles.
 # RUN: llvm-readobj --symbols --dyn-symbols %t64 | FileCheck %s --check-prefix=BOTH-LLVM
 # RUN: llvm-readelf --symbols --dyn-symbols %t64 | FileCheck %s --check-prefix=BOTH-GNU
 
 # BOTH-LLVM: Symbols [
 # BOTH-LLVM: DynamicSymbols [

llvm/test/tools/llvm-readobj/basic.test

 
 # Test help switch.
 RUN: llvm-readobj --help | FileCheck %s --check-prefixes=HELP,OBJ
 RUN: llvm-readelf --help | FileCheck %s --check-prefixes=HELP,ELF
 HELP: OVERVIEW: LLVM Object Reader
 OBJ: llvm-readobj{{.*}} [options] <input object files>
 ELF: llvm-readelf{{.*}} [options] <input object files>
 HELP: OPTIONS:
-OBJ: -s - Alias for --section-headers
-OBJ: -t - Alias for --symbols
-ELF: -s - Alias for --symbols
-ELF: -t - Alias for --section-details
+HELP -s - Alias for --symbols
+HELP -t - Alias for --section-details
 HELP: @FILE

llvm/test/tools/yaml2obj/ELF/duplicate-symbol-names.yaml

 ## Check that yaml2obj is able to produce an object from YAML
 ## containing symbols with duplicate names (but different name suffixes).
 
 # RUN: yaml2obj --docnum=1 %s -o %t1
-# RUN: llvm-readobj -t %t1 | FileCheck %s --check-prefix=CASE1
+# RUN: llvm-readobj --syms %t1 | FileCheck %s --check-prefix=CASE1
 
 # CASE1: Name: localfoo (1)
 # CASE1: Name: localfoo (1)
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB

llvm/tools/llvm-readobj/llvm-readobj.cpp

namespace opts {
   // --dyn-relocations
   cl::opt<bool> DynRelocs("dyn-relocations",
     cl::desc("Display the dynamic relocation entries in the file"));
 
   // --section-details
   // Also -t in llvm-readelf mode.
   cl::opt<bool> SectionDetails("section-details",
                                cl::desc("Display the section details"));
+  static cl::alias SectionDetailsShort("t",
+                                       cl::desc("Alias for --section-details"),
+                                       cl::aliasopt(SectionDetails),
+                                       cl::NotHidden);
 
   // --symbols
   // Also -s in llvm-readelf mode, or -t in llvm-readobj mode.
   cl::opt<bool>
       Symbols("symbols",
               cl::desc("Display the symbol table. Also display the dynamic "
                        "symbol table when using GNU output style for ELF"));
   cl::alias SymbolsGNU("syms", cl::desc("Alias for --symbols"),
                        cl::aliasopt(Symbols));
+  static cl::alias SymbolsShort("s", cl::desc("Alias for --symbols"),
+                                cl::aliasopt(Symbols), cl::NotHidden,
+                                cl::Grouping);
 
   // --dyn-symbols, --dyn-syms
   // Also --dt in llvm-readobj mode.
   cl::opt<bool> DynamicSymbols("dyn-symbols",
     cl::desc("Display the dynamic symbol table"));
   cl::alias DynSymsGNU("dyn-syms", cl::desc("Alias for --dyn-symbols"),
                        cl::aliasopt(DynamicSymbols));
 
else
     llvm_unreachable("unrecognized file type");
 
   CVTypes.Binaries.push_back(
       OwningBinary<Binary>(std::move(Bin), std::move(Buffer)));
 }
 
 /// Registers aliases that should only be allowed by readobj.
 static void registerReadobjAliases() {
-  // -s has meant --sections for a very long time in llvm-readobj despite
-  // meaning --symbols in readelf.
-  static cl::alias SectionsShort("s", cl::desc("Alias for --section-headers"),
-                                 cl::aliasopt(opts::SectionHeaders),
-                                 cl::NotHidden);
-
-  // llvm-readelf reserves it for --section-details.
-  static cl::alias SymbolsShort("t", cl::desc("Alias for --symbols"),
-                                cl::aliasopt(opts::Symbols), cl::NotHidden);
-
   // The following two-letter aliases are only provided for readobj, as readelf
   // allows single-letter args to be grouped together.
   static cl::alias SectionRelocationsShort(
       "sr", cl::desc("Alias for --section-relocations"),
       cl::aliasopt(opts::SectionRelocations));
   static cl::alias SectionDataShort("sd", cl::desc("Alias for --section-data"),
                                     cl::aliasopt(opts::SectionData));
   static cl::alias SectionSymbolsShort("st",
                                        cl::desc("Alias for --section-symbols"),
                                        cl::aliasopt(opts::SectionSymbols));
   static cl::alias DynamicSymbolsShort("dt",
                                        cl::desc("Alias for --dyn-symbols"),
                                        cl::aliasopt(opts::DynamicSymbols));
 }
 
 /// Registers aliases that should only be allowed by readelf.
 static void registerReadelfAliases() {
-  // -s is here because for readobj it means --sections.
-  static cl::alias SymbolsShort("s", cl::desc("Alias for --symbols"),
-                                cl::aliasopt(opts::Symbols), cl::NotHidden,
-                                cl::Grouping);
-
-  // -t is here because for readobj it is an alias for --symbols.
-  static cl::alias SectionDetailsShort(
-      "t", cl::desc("Alias for --section-details"),
-      cl::aliasopt(opts::SectionDetails), cl::NotHidden);
-
   // Allow all single letter flags to be grouped together.
   for (auto &OptEntry : cl::getRegisteredOptions()) {
     StringRef ArgName = OptEntry.getKey();
     cl::Option *Option = OptEntry.getValue();
     if (ArgName.size() == 1)
       apply(Option, cl::Grouping);
   }
 }