[llvm-readelf] Allow single-letter flags to be merged.

[rupprecht]

Summary

This patch adds support for merged arguments (e.g. -SW == -S -W) for llvm-readelf.

No changes are intended for llvm-readobj. There are a few short flags (-sd, -sr, -st, -dt) that would conflict with grouped single letter flags, and having only some grouped flags might be confusing. So, allow merged flags for readelf compatibility, but force separate args for llvm-readobj. From what I can tell, these two-letter flags are only used with llvm-readobj, not llvm-readelf.

This fixes PR40064.

[jhenderson]

One question: what happens if you do enable grouping, and there's an ambiguous alias?

test/tools/llvm-readobj/merged.test

6

Use FileCheck --input-file instead of cat.

11

What's with the |& ?

14–17

These should probably be in the corresponding tests for the appropriate dumps. I don't think we need them here.

tools/llvm-readobj/llvm-readobj.cpp

633

I think this comment needs tweaking. As things stand, the args is ambiguous (i.e. is it referring to "-sd" or "-s" and "-d". I think you can replace "these args" with "single-letter args".

654

Could we come up with some way of enforcing this list to be extended by any new single letter aliases that are added? I don't know if it's possible, but one option might be to set grouping on by default, and disable it for llvm-readobj.

[rupprecht]

• Move two-letter aliases tests to separate tests
• Automatically merge any single-letter args

[rupprecht]

In D56629#1355766, @jhenderson wrote:

One question: what happens if you do enable grouping, and there's an ambiguous alias?

I believe there's a silent failure... I think it prefers the longer alias (i.e. registering s, r, and sr, will cause "-sr" to be interpreted as sr, not s and r).

test/tools/llvm-readobj/merged.test

11

It redirects stderr (i.e. the "unknown command line arg" we're checking) in a pipeline: https://www.gnu.org/software/bash/manual/bash.html#Pipelines

But 2>&1 works just as well and is more widely used (I can't find |& in any llvm tests, so lit probably doesn't support it), so I'll just use that.

tools/llvm-readobj/llvm-readobj.cpp

654

Yes, there is an API for that! Updated.

[jhenderson]

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

3–4

aliases -> alias, names -> name

7

Would diff make more sense here, since these are text files?

38

Here and below, could you check that there are no extraneous characters at the end of the name, by putting a {{$}} at the end, please? This would match other names for which this is a prefix. You might need to capture the string offset as a result.

test/tools/llvm-readobj/merged.test

11

Ah, I didn't know about that. Cool, but yes, use 2>&1 instead!

tools/llvm-readobj/llvm-readobj.cpp

633

missing "args" after the latest change, i.e. "single-letter" to "single-letter args"

[rupprecht]

• Use diff instead of cmp
• Check for full symbol names
• Fix typos

[rupprecht]

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

38

The actual output here is _ITM_deregisterTMCloneTable (28), so I used {{ }} instead, as {{$}} fails due to the trailing (28). I verified that truncating the name (e.g. _ITM_deregisterTMCloneTabl{{ }}) causes the test to fail.

[jhenderson]

LGTM!

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

+RUN: llvm-readobj --dyn-symbols %p/Inputs/dynamic-table-so.x86 | FileCheck %s
+
+# Check the two-letter aliases -dt is equivalent to --dyn-symbols
+# full flag names.
+RUN: llvm-readobj -dt %p/Inputs/dynamic-table-so.x86 > %t.readobj-dt-alias
+RUN: llvm-readobj --dyn-symbols %p/Inputs/dynamic-table-so.x86 > %t.readobj-dt-no-alias
+RUN: cmp %t.readobj-dt-alias %t.readobj-dt-no-alias
+
+# CHECK:      DynamicSymbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name:
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Local
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name:
+# CHECK-NEXT:     Value: 0x618
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Local
+# CHECK-NEXT:     Type: Section
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .init
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name:
+# CHECK-NEXT:     Value: 0x200DC0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Local
+# CHECK-NEXT:     Type: Section
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .tbss
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _ITM_deregisterTMCloneTable
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Weak
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: puts@GLIBC_2.2.5
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Function
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: __tls_get_addr@GLIBC_2.3
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Function
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: __gmon_start__
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Weak
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _Jv_RegisterClasses
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Weak
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _ITM_registerTMCloneTable
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Weak
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: __cxa_finalize@GLIBC_2.2.5
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Weak
+# CHECK-NEXT:     Type: Function
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _edata
+# CHECK-NEXT:     Value: 0x201030
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .data
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: foo
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 4
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: TLS
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .tbss
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _end
+# CHECK-NEXT:     Value: 0x201038
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .bss
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: __bss_start
+# CHECK-NEXT:     Value: 0x201030
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: None
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .bss
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: main
+# CHECK-NEXT:     Value: 0x780
+# CHECK-NEXT:     Size: 59
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Function
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .text
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _init
+# CHECK-NEXT:     Value: 0x618
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Function
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .init
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _fini
+# CHECK-NEXT:     Value: 0x7BC
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type: Function
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .fini
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]

test/tools/llvm-readobj/merged.test

+# Check merged args produce identical output to when not merged.
+RUN: llvm-readelf -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 > %t.merged
+RUN: llvm-readelf -a -e -W -h -S -r -n -u -d -l -V -g -I -s %p/Inputs/trivial.obj.elf-i386 > %t.not-merged
+RUN: cmp %t.merged %t.not-merged
+RUN: FileCheck %s --input-file %t.merged
+
+# 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 %p/Inputs/trivial.obj.elf-i386 2>&1 | FileCheck %s --check-prefix=UNKNOWN
+
+CHECK-NOT: Unknown command line argument
+UNKNOWN:   Unknown command line argument

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

 RUN: llvm-readobj -s -st -sr -sd %p/Inputs/trivial.obj.coff-i386 \
 RUN:   | FileCheck %s -check-prefix COFF
 RUN: llvm-readobj -s -st -sr -sd %p/Inputs/trivial.obj.elf-i386 \
 RUN:   | FileCheck %s -check-prefix ELF
 RUN: llvm-readobj -s -st -sr -sd %p/Inputs/trivial.obj.macho-i386 \
 RUN:   | FileCheck %s -check-prefix MACHO-I386
 RUN: llvm-readobj -s -st -sr -sd %p/Inputs/trivial.obj.macho-x86-64 \
 RUN:   | FileCheck %s -check-prefix MACHO-X86-64
 RUN: llvm-readobj --expand-relocs -s -st -sr -sd %p/Inputs/trivial.obj.macho-ppc \
 RUN:   | FileCheck %s -check-prefix MACHO-PPC
 RUN: llvm-readobj --expand-relocs -s -st -sr -sd %p/Inputs/trivial.obj.macho-ppc64 \
 RUN:   | FileCheck %s -check-prefix MACHO-PPC64
 RUN: llvm-readobj -expand-relocs -s -st -sr -sd %p/Inputs/trivial.obj.macho-arm \
 RUN:   | FileCheck %s -check-prefix MACHO-ARM
 
+# Check the two-letter aliases above (-st, -sr, -sd) are equivalent to their
+# full flag names.
+RUN: llvm-readobj -s -st %p/Inputs/trivial.obj.elf-i386 > %t.readobj-st-alias
+RUN: llvm-readobj -s --section-symbols %p/Inputs/trivial.obj.elf-i386 > %t.readobj-st-no-alias
+RUN: cmp %t.readobj-st-alias %t.readobj-st-no-alias
+
+RUN: llvm-readobj -s -sr %p/Inputs/trivial.obj.elf-i386 > %t.readobj-sr-alias
+RUN: llvm-readobj -s --section-relocations %p/Inputs/trivial.obj.elf-i386 > %t.readobj-sr-no-alias
+RUN: cmp %t.readobj-sr-alias %t.readobj-sr-no-alias
+
+RUN: llvm-readobj -s -sd %p/Inputs/trivial.obj.elf-i386 > %t.readobj-sd-alias
+RUN: llvm-readobj -s --section-data %p/Inputs/trivial.obj.elf-i386 > %t.readobj-sd-no-alias
+RUN: cmp %t.readobj-sd-alias %t.readobj-sd-no-alias
+
 COFF:      Sections [
 COFF-NEXT:   Section {
 COFF-NEXT:     Number: 1
 COFF-NEXT:     Name: .text (2E 74 65 78 74 00 00 00)
 COFF-NEXT:     VirtualSize: 0x0
 COFF-NEXT:     VirtualAddress: 0x0
 COFF-NEXT:     RawDataSize: 22
 COFF-NEXT:     PointerToRawData: 0x64

tools/llvm-readobj/llvm-readobj.cpp

namespace opts {
   cl::opt<bool> SectionHeaders("section-headers",
                                cl::desc("Display all section headers."));
   cl::alias SectionsShortUpper("S", cl::desc("Alias for --section-headers"),
                                cl::aliasopt(SectionHeaders), cl::NotHidden);
   cl::alias SectionHeadersAlias("sections",
                                 cl::desc("Alias for --section-headers"),
                                 cl::aliasopt(SectionHeaders), cl::NotHidden);
 
-  // -section-relocations, -sr
+  // -section-relocations
+  // Also -sr in llvm-readobj mode.
   cl::opt<bool> SectionRelocations("section-relocations",
     cl::desc("Display relocations for each section shown."));
-  cl::alias SectionRelocationsShort("sr",
-    cl::desc("Alias for --section-relocations"),
-    cl::aliasopt(SectionRelocations));
 
-  // -section-symbols, -st
+  // -section-symbols
+  // Also -st in llvm-readobj mode.
   cl::opt<bool> SectionSymbols("section-symbols",
     cl::desc("Display symbols for each section shown."));
-  cl::alias SectionSymbolsShort("st",
-    cl::desc("Alias for --section-symbols"),
-    cl::aliasopt(SectionSymbols));
 
-  // -section-data, -sd
+  // -section-data
+  // Also -sd in llvm-readobj mode.
   cl::opt<bool> SectionData("section-data",
     cl::desc("Display section data for each section shown."));
-  cl::alias SectionDataShort("sd",
-    cl::desc("Alias for --section-data"),
-    cl::aliasopt(SectionData));
 
   // -relocations, -relocs, -r
   cl::opt<bool> Relocations("relocations",
     cl::desc("Display the relocation entries in the file"));
   cl::alias RelocationsShort("r", cl::desc("Alias for --relocations"),
                              cl::aliasopt(Relocations), cl::NotHidden);
   cl::alias RelocationsGNU("relocs", cl::desc("Alias for --relocations"),
                            cl::aliasopt(Relocations));
 
   // -notes, -n
   cl::opt<bool> Notes("notes", cl::desc("Display the ELF notes in the file"));
   cl::alias NotesShort("n", cl::desc("Alias for --notes"), cl::aliasopt(Notes));
 
   // -dyn-relocations
   cl::opt<bool> DynRelocs("dyn-relocations",
     cl::desc("Display the dynamic relocation entries in the file"));
 
   // -symbols
   // Also -s in llvm-readelf mode, or -t in llvm-readobj mode.
   cl::opt<bool> Symbols("symbols",
     cl::desc("Display the symbol table"));
   cl::alias SymbolsGNU("syms", cl::desc("Alias for --symbols"),
                        cl::aliasopt(Symbols));
 
-  // -dyn-symbols, -dyn-syms, -dt
+  // -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 DynamicSymbolsShort("dt",
-    cl::desc("Alias for --dyn-symbols"),
-    cl::aliasopt(DynamicSymbols));
   cl::alias DynSymsGNU("dyn-syms", cl::desc("Alias for --dyn-symbols"),
                        cl::aliasopt(DynamicSymbols));
 
   // -unwind, -u
   cl::opt<bool> UnwindInfo("unwind",
     cl::desc("Display unwind information"));
   cl::alias UnwindInfoShort("u",
     cl::desc("Alias for --unwind"),
static void registerReadobjAliases() {
   static cl::alias SectionsShort("s", cl::desc("Alias for --section-headers"),
                                  cl::aliasopt(opts::SectionHeaders),
                                  cl::NotHidden);
 
   // Only register -t in llvm-readobj, as readelf reserves it for
   // --section-details (not implemented yet).
   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 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::aliasopt(opts::Symbols), cl::NotHidden,
+                                cl::Grouping);
+
+  // 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)
+      Option->setFormattingFlag(cl::Grouping);
+  }
 }
 
 int main(int argc, const char *argv[]) {
   InitLLVM X(argc, argv);
 
   // Register the target printer for --version.
   cl::AddExtraVersionPrinter(TargetRegistry::printRegisteredTargetsForVersion);