[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

5 ↗

(On Diff #181410)

Use FileCheck --input-file instead of cat.

10 ↗

(On Diff #181410)

What's with the |& ?

13–16 ↗

(On Diff #181410)

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 ↗

(On Diff #181410)

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 ↗

(On Diff #181410)

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

10 ↗

(On Diff #181410)

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 ↗

(On Diff #181410)

Yes, there is an API for that! Updated.

[jhenderson]

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

3–4 ↗

(On Diff #181617)

aliases -> alias, names -> name

7 ↗

(On Diff #181617)

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

38 ↗

(On Diff #181617)

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

10 ↗

(On Diff #181410)

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

tools/llvm-readobj/llvm-readobj.cpp

633 ↗

(On Diff #181617)

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 ↗

(On Diff #181617)

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!

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

+RUN: llvm-readobj --dyn-symbols %p/Inputs/dynamic-table-so.x86 | FileCheck %s
+
+# Check the two-letter alias -dt is equivalent to the --dyn-symbols full flag
+# name.
+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: diff %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: ]

llvm/trunk/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

llvm/trunk/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: diff %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: diff %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: diff %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

llvm/trunk/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 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::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);