[llvm-objdump] Use a counter for llvm-objdump -h instead of the section index.

[rupprecht]

Summary

When listing the index in llvm-objdump -h, use a zero-based counter instead of the actual section index (e.g. shdr->sh_index for ELF).

While this is effectively a noop for now (except one unit test for XCOFF), the index values will change in a future patch that filters certain sections out (e.g. symbol tables). See D68669 for more context. Note: the test case in test/tools/llvm-objdump/X86/section-index.s already covers the case of incrementing the section index counter when sections are skipped.

[rupprecht]

Rebase against D68730

[grimar]

A few ideas/suggestions.

llvm/tools/llvm-objdump/llvm-objdump.cpp

345–350

It's common to wrap such things (helper types) into anonymous namespaces:
I think you can also add a helper function too:

namespace {
struct FilterResult {
  bool Keep;
  bool IncrementIndex;
};

FilterResult checkSectionFilter(object::SectionRef S) {
...
}
};

347

I'd comment these fields. It wasn't obvious to me what they are used for.

352

I think we use a full variable name usually, i.e. Increment -> IncrementIndex

374

nit: if (Idx) would be shorer.

378

Please avoid using auto for return types that are not obvious.

1699

Looking at this,
should ToolSectionFilter just return [(SectionRef&)Ref, (uint64_t )Index] struct/pair instead? It seems could make the whole logic simper.

llvm/tools/llvm-objdump/llvm-objdump.h

86

I think this needs a full comment (it does not give an information about what this helper avtually do atm):

I.e. would be nice to see something like:

// Function is used to ...
// Idx is a optional output parameter that ...

[rupprecht]

• Add/update several comments
• Use simplified pointer check
• Avoid auto as the returned type is not obvious

[rupprecht]

Rebase against D68730 again

[rupprecht]

llvm/tools/llvm-objdump/llvm-objdump.cpp

345–350

Wrapped just the struct per http://llvm.org/docs/CodingStandards.html#anonymous-namespaces

352

Sorry, I originally had the variable named Increment but forgot to update these comments.

1699

That's one of the paths I considered while writing this patch. A couple thoughts:

1. This is the only place that needs this counter value, and loop iteration in every other case (7 other places in this file, 1 in the MachO dumper) would now be a little more complicated even when callers don't care about the index, e.g. it would now be:

for (const SomeWrapperType &Foo : ToolSectionFilter(*Obj)) {
  const SectionRef &Section = Foo.Section;

2. llvm has make_filter_range which probably did not exist at the time this code was first written, and it would be nice to completely remove SectionFilter and SectionFilterIterator from llvm-objdump.h in favor of those standard llvm libraries, but AIUI in order to use that, the return type would need to be SectionRef, not some wrapper type. (I'm trying to do that in a separate branch, but I'm dealing with template woes).
3. But on the plus side, it does avoid out parameters which would be very nice.

I think 1&2 outweigh 3, so I'm leaning towards this approach. But I'm still exploring alternatives.

[grimar]

llvm/tools/llvm-objdump/llvm-objdump.cpp

368

Can we have a test for this logic? I.e. for a case when Keep=false, IncrementIndex=true.
(Doesn't seem we have it)

1699

I see. I have no much better/different ideas atm. Perhaps the current approach is OK for now.

[rupprecht]

llvm/tools/llvm-objdump/llvm-objdump.cpp

368

This is already covered by llvm/test/tools/llvm-objdump/X86/section-index.s:

# RUN: llvm-objdump -section-headers %t | FileCheck %s
...
# CHECK-NEXT: 4  .bar

# RUN: llvm-objdump -section-headers -section=.bar %t \
# RUN:   | FileCheck %s --check-prefix=BAR
...
# BAR-NEXT:  4  .bar

I'll mention this in the patch description.

[grimar]

LGTM

llvm/tools/llvm-objdump/llvm-objdump.cpp

368

Ah, OK.

[rupprecht]

Thanks for the review!

lld/test/ELF/got32-i386.s

  nop
 
 _start:
  movl foo@GOT, %ebx
 
 ## 73728 == 0x12000 == ADDR(.got)
 # CHECK: Sections:
 # CHECK:  Name Size     VMA
-# CHECK:  .got 00000004 00000000004020fc
+# CHECK:  .got 00000004 004020fc
 # CHECK:       _start:
 # CHECK-NEXT:   4010f5: 8b 1d {{.*}}  movl 4202748, %ebx
 
 # RUN: not ld.lld %t.o -o %t -pie 2>&1 | FileCheck %s --check-prefix=ERR
 # ERR: error: symbol 'foo' cannot be preempted; recompile with -fPIE

lld/test/ELF/got32x-i386.s

 ## register, it should be calculated as G + A. Using without base register
 ## is only allowed for non-PIC code.
 ##
 # RUN: ld.lld %S/Inputs/i386-got32x-baseless.elf -o %t1
 # RUN: llvm-objdump -section-headers -d %t1 | FileCheck %s
 
 ## 73728 == 0x12000 == ADDR(.got)
 # CHECK: Sections:
-# CHECK:  Name Size     VMA
-# CHECK:  .got.plt 0000000c 0000000000403134
+# CHECK:  Name     Size     VMA
+# CHECK:  .got.plt 0000000c 00403134
 # CHECK:       _start:
 # CHECK-NEXT:   401115: 8b 05 {{.*}} movl 4202800, %eax
 # CHECK-NEXT:   40111b: 8b 1d {{.*}} movl 4202800, %ebx
 # CHECK-NEXT:   401121: 8b 80 {{.*}} movl -4100(%eax), %eax
 # CHECK-NEXT:   401127: 8b 83 {{.*}} movl -4100(%ebx), %eax
 
 # RUN: not ld.lld %S/Inputs/i386-got32x-baseless.elf -o %t1 -pie 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=ERR
 # ERR-COUNT-2: error: symbol 'foo' cannot be preempted; recompile with -fPIE

llvm/test/tools/llvm-objdump/section-headers-address-width.test

+## Check that objdump -h only prints the necessary amount of bytes for
+## addresses.
+
+# RUN: yaml2obj %s --docnum=1 -o %t-32bit.o
+# RUN: yaml2obj %s --docnum=2 -o %t-64bit.o
+
+# RUN: llvm-objdump -h --show-lma %t-32bit.o \
+# RUN:   | FileCheck %s --check-prefix=32 --strict-whitespace
+# 32: {{^}}Idx Name          Size     VMA      LMA      Type{{$}}
+# 32: {{^}}  1 .foo          00000000 00000400 00000400 TEXT{{$}}
+
+# RUN: llvm-objdump -h %t-32bit.o \
+# RUN:   | FileCheck %s --check-prefix=32-NO-LMA --strict-whitespace
+# 32-NO-LMA: {{^}}Idx Name          Size     VMA      Type{{$}}
+# 32-NO-LMA: {{^}}  1 .foo          00000000 00000400 TEXT{{$}}
+
+# RUN: llvm-objdump -h --show-lma %t-64bit.o \
+# RUN:   | FileCheck %s --check-prefix=64 --strict-whitespace
+# 64: {{^}}Idx Name          Size     VMA              LMA              Type{{$}}
+# 64: {{^}}  1 .foo          00000000 0000000000000400 0000000000000400 TEXT{{$}}
+
+# RUN: llvm-objdump -h %t-64bit.o \
+# RUN:   | FileCheck %s --check-prefix=64-NO-LMA --strict-whitespace
+# 64-NO-LMA: {{^}}Idx Name          Size     VMA              Type{{$}}
+# 64-NO-LMA: {{^}}  1 .foo          00000000 0000000000000400 TEXT{{$}}
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_386
+Sections:
+  - Name:    .foo
+    Type:    SHT_PROGBITS
+    Flags:   [SHF_ALLOC, SHF_EXECINSTR]
+    Address: 0x400
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:    .foo
+    Type:    SHT_PROGBITS
+    Flags:   [SHF_ALLOC, SHF_EXECINSTR]
+    Address: 0x400

llvm/test/tools/llvm-objdump/section-headers-name-width.test

+## The name field automatically expands past the default 13 columns when a
+## section name is longer than that.
+## Note: check with and without --show-lma to make sure resizing happens in
+## both cases.
+
+# RUN: yaml2obj %s --docnum=1 -o %t-name13chars.o
+# RUN: llvm-objdump -h --show-lma %t-name13chars.o \
+# RUN:   | FileCheck %s --check-prefix=NAME-13 --strict-whitespace
+# RUN: llvm-objdump -h %t-name13chars.o \
+# RUN:   | FileCheck %s --check-prefix=NAME-13 --strict-whitespace
+
+# NAME-13:      Sections:
+# NAME-13-NEXT: {{^}}Idx Name          Size
+# NAME-13-NEXT: {{^}}  0               00000000
+# NAME-13-NEXT: {{^}}  1 .foo          00000000
+# NAME-13-NEXT: {{^}}  2 .thirteenchar 00000000
+# NAME-13-NEXT: {{^}}  3 .bar          00000000
+
+## Including a section with a name of length 14 expands the width of the "Name"
+## column.
+# RUN: yaml2obj %s --docnum=2 -o %t-name14chars.o
+# RUN: llvm-objdump -h --show-lma %t-name14chars.o \
+# RUN:   | FileCheck %s --check-prefix=NAME-14 --strict-whitespace
+# RUN: llvm-objdump -h %t-name14chars.o \
+# RUN:   | FileCheck %s --check-prefix=NAME-14 --strict-whitespace
+
+# NAME-14:      Sections:
+# NAME-14-NEXT: {{^}}Idx Name           Size
+# NAME-14-NEXT: {{^}}  0                00000000
+# NAME-14-NEXT: {{^}}  1 .foo           00000000
+# NAME-14-NEXT: {{^}}  2 .fourteenchars 00000000
+# NAME-14-NEXT: {{^}}  3 .bar           00000000
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name: .foo
+    Type: SHT_PROGBITS
+  - Name: .thirteenchar
+    Type: SHT_PROGBITS
+  - Name: .bar
+    Type: SHT_PROGBITS
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name: .foo
+    Type: SHT_PROGBITS
+  - Name: .fourteenchars
+    Type: SHT_PROGBITS
+  - Name: .bar
+    Type: SHT_PROGBITS

llvm/test/tools/llvm-objdump/section-headers-spacing.test

+## Check leading and trailing whitespace for full lines.
+
+# RUN: yaml2obj %s -o %t-whitespace.o
+# RUN: llvm-objdump -h --show-lma %t-whitespace.o \
+# RUN:   | FileCheck %s --check-prefix=WHITESPACE --strict-whitespace
+# RUN: llvm-objdump -h %t-whitespace.o \
+# RUN:   | FileCheck %s --check-prefix=WHITESPACE-NO-LMA --strict-whitespace
+
+# WHITESPACE:      {{^}}Sections:{{$}}
+# WHITESPACE-NEXT: {{^}}Idx Name          Size     VMA              LMA              Type{{$}}
+# WHITESPACE-NEXT: {{^}}  0               00000000 0000000000000000 0000000000000000 {{$}}
+# WHITESPACE-NEXT: {{^}}  1 .text         00000000 0000000000000000 0000000000000000 TEXT{{$}}
+# WHITESPACE-NEXT: {{^}}  2 .data         00000000 0000000000000000 0000000000000000 DATA{{$}}
+# WHITESPACE-NEXT: {{^}}  3 .bss          00000000 0000000000000000 0000000000000000 BSS{{$}}
+# WHITESPACE-NEXT: {{^}}  4 .other        00000000 0000000000000000 0000000000000000 {{$}}
+
+# WHITESPACE-NO-LMA:      {{^}}Sections:{{$}}
+# WHITESPACE-NO-LMA-NEXT: {{^}}Idx Name          Size     VMA              Type{{$}}
+# WHITESPACE-NO-LMA-NEXT: {{^}}  0               00000000 0000000000000000 {{$}}
+# WHITESPACE-NO-LMA-NEXT: {{^}}  1 .text         00000000 0000000000000000 TEXT{{$}}
+# WHITESPACE-NO-LMA-NEXT: {{^}}  2 .data         00000000 0000000000000000 DATA{{$}}
+# WHITESPACE-NO-LMA-NEXT: {{^}}  3 .bss          00000000 0000000000000000 BSS{{$}}
+# WHITESPACE-NO-LMA-NEXT: {{^}}  4 .other        00000000 0000000000000000 {{$}}
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:  .text
+    Type:  SHT_PROGBITS
+    Flags: [SHF_ALLOC, SHF_EXECINSTR]
+  - Name:  .data
+    Type:  SHT_PROGBITS
+    Flags: [SHF_ALLOC, SHF_WRITE]
+  - Name:  .bss
+    Type:  SHT_NOBITS
+    Flags: [SHF_ALLOC, SHF_WRITE]
+  - Name:  .other
+    Type:  SHT_REL

llvm/test/tools/llvm-objdump/wasm.txt

 # RUN: llvm-objdump -h %p/Inputs/trivial.obj.wasm | FileCheck %s
 
 # CHECK:      Sections:
-# CHECK-NEXT: Idx Name          Size    VMA              Type
-# CHECK-NEXT:  0 TYPE          00000011 0000000000000000
-# CHECK-NEXT:  1 IMPORT        0000005d 0000000000000000
-# CHECK-NEXT:  2 FUNCTION      00000003 0000000000000000
-# CHECK-NEXT:  3 CODE          00000024 0000000000000000 TEXT
-# CHECK-NEXT:  4 DATA          0000001c 0000000000000000 DATA
-# CHECK-NEXT:  5 linking       0000006d 0000000000000000
-# CHECK-NEXT:  6 reloc.CODE    0000000f 0000000000000000
+# CHECK-NEXT: Idx Name          Size     VMA      Type
+# CHECK-NEXT:   0 TYPE          00000011 00000000
+# CHECK-NEXT:   1 IMPORT        0000005d 00000000
+# CHECK-NEXT:   2 FUNCTION      00000003 00000000
+# CHECK-NEXT:   3 CODE          00000024 00000000 TEXT
+# CHECK-NEXT:   4 DATA          0000001c 00000000 DATA
+# CHECK-NEXT:   5 linking       0000006d 00000000
+# CHECK-NEXT:   6 reloc.CODE    0000000f 00000000
 
 # RUN: llvm-objdump -p %p/Inputs/trivial.obj.wasm | FileCheck %s -check-prefix CHECK-HEADER
 
 # CHECK-HEADER: Program Header:
 # CHECK-HEADER: Version: 0x1
 
 # RUN: llvm-objdump -s --section=CODE %p/Inputs/trivial.obj.wasm | FileCheck %s -check-prefix CHECK-SECTIONS
 
 # CHECK-SECTIONS: Contents of section CODE:
 # CHECK-SECTIONS: 0000 02170041 80808080 00108080 8080001a  ...A............
 # CHECK-SECTIONS: 0010 10838080 80004100 0b0a0041 00108180  ......A....A....
 # CHECK-SECTIONS: 0020 8080000b                             ....

llvm/test/tools/llvm-objdump/xcoff-section-headers.test

 # RUN: llvm-objdump --section-headers %p/Inputs/xcoff-section-headers.o | \
 # RUN: FileCheck %s
 
 # RUN: llvm-objdump --section-headers %p/Inputs/xcoff-long-sec-names.o | \
 # RUN: FileCheck --check-prefix=LONG %s
 
 # RUN: not llvm-objdump --section-headers 2>&1 \
 # RUN: %p/Inputs/xcoff-section-headers-truncate.o | FileCheck \
 # RUN: --check-prefix=ERROR %s
 
 # ERROR: The end of the file was unexpectedly encountered
 
 # CHECK: xcoff-section-headers.o:      file format aixcoff-rs6000
 # CHECK: Sections:
-# CHECK: Idx Name          Size     VMA          Type
-# CHECK:   1 .text         00000080 0000000000000000 TEXT
-# CHECK:   2 .data         00000024 0000000000000080 DATA
-# CHECK:   3 .bss          00000004 00000000000000a4 BSS
-# CHECK:   4 .tdata        00000008 0000000000000000 DATA
-# CHECK:   5 .tbss         00000004 0000000000000008 BSS
+# CHECK: Idx Name          Size     VMA      Type
+# CHECK:   0 .text         00000080 00000000 TEXT
+# CHECK:   1 .data         00000024 00000080 DATA
+# CHECK:   2 .bss          00000004 000000a4 BSS
+# CHECK:   3 .tdata        00000008 00000000 DATA
+# CHECK:   4 .tbss         00000004 00000008 BSS
 
 # xcoff-section-headers.o Compiled with IBM XL C/C++ for AIX, V16.1.0
 # test.c:
 # int a;
 # int b = 12345;
 # __thread int c;
 # __thread double d = 3.14159;
 #
 # int func(void)  {
 #   return a;
 # }
 
 # LONG: xcoff-long-sec-names.o:      file format aixcoff-rs6000
 # LONG: Sections:
-# LONG: Idx Name          Size     VMA          Type
-# LONG: 1 .dwarnge      00000004 0000000000000000
-# LONG: 2 .dwpbnms      00000004 0000000000000000
-# LONG: 3 .dwpbtyp      00000004 0000000000000000
+# LONG: Idx Name          Size     VMA      Type
+# LONG:   0 .dwarnge      00000004 00000000
+# LONG:   1 .dwpbnms      00000004 00000000
+# LONG:   2 .dwpbtyp      00000004 00000000
 
 # xcoff-long-sec-names.o was generated by assembling the following .s file:
 #  .dwsect 0x30000 # .dwpbnms section
 #  .dwsect 0x40000 # .dwpbtyp section
 #  .dwsect 0x50000 # .dwarnge section

llvm/tools/llvm-objdump/llvm-objdump.h

public:
   }
 
 private:
   FilterPredicate Predicate;
   llvm::object::ObjectFile const &Object;
 };
 
 // Various helper functions.
-SectionFilter ToolSectionFilter(llvm::object::ObjectFile const &O);
+
+// The optional index parameter is to keep track of which section index this is.
+// This may be different than the actual section number, as some sections may be
+// filtered (e.g. symbol tables).
+SectionFilter ToolSectionFilter(llvm::object::ObjectFile const &O,
+                                uint64_t *Idx = nullptr);
 
 Error getELFRelocationValueString(const object::ELFObjectFileBase *Obj,
                                   const object::RelocationRef &Rel,
                                   llvm::SmallVectorImpl<char> &Result);
 Error getCOFFRelocationValueString(const object::COFFObjectFile *Obj,
                                    const object::RelocationRef &Rel,
                                    llvm::SmallVectorImpl<char> &Result);
 Error getWasmRelocationValueString(const object::WasmObjectFile *Obj,

llvm/tools/llvm-objdump/llvm-objdump.cpp

static cl::extrahelp
     HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
 
 static StringSet<> DisasmFuncsSet;
 static StringSet<> FoundSectionSet;
 static StringRef ToolName;
 
 typedef std::vector<std::tuple<uint64_t, StringRef, uint8_t>> SectionSymbolsTy;
 
-static bool shouldKeep(object::SectionRef S) {
+struct FilterResult {
+  bool Keep;
+  bool IncrementIndex;
+};
+
+static FilterResult checkSectionFilter(object::SectionRef S) {
   if (FilterSections.empty())
-    return true;
+    return {/*Keep=*/true, /*Increment=*/true};
 
   Expected<StringRef> SecNameOrErr = S.getName();
   if (!SecNameOrErr) {
     consumeError(SecNameOrErr.takeError());
-    return false;
+    return {/*Keep=*/false, /*Increment=*/false};
   }
   StringRef SecName = *SecNameOrErr;
 
   // StringSet does not allow empty key so avoid adding sections with
   // no name (such as the section with index 0) here.
   if (!SecName.empty())
     FoundSectionSet.insert(SecName);
-  return is_contained(FilterSections, SecName);
-}
 
-SectionFilter ToolSectionFilter(object::ObjectFile const &O) {
-  return SectionFilter([](object::SectionRef S) { return shouldKeep(S); }, O);
+  // Only show the section if it's in the FilterSections list, but always
+  // increment so the indexing is stable.
+  return {/*Keep=*/is_contained(FilterSections, SecName), /*Increment=*/true};
+}
+
+SectionFilter ToolSectionFilter(object::ObjectFile const &O, uint64_t *Idx) {
+  // Start at UINT64_MAX so that the first index returned after an increment is
+  // zero (after the unsigned wrap).
+  if (Idx != nullptr)
+    *Idx = UINT64_MAX;
+  return SectionFilter(
+      [Idx](object::SectionRef S) {
+        auto Result = checkSectionFilter(S);
+        if (Idx != nullptr && Result.IncrementIndex)
+          *Idx += 1;
+        return Result.Keep;
+      },
+      O);
 }
 
 std::string getFileNameForError(const object::Archive::Child &C,
                                 unsigned Index) {
   Expected<StringRef> NameOrErr = C.getName();
   if (NameOrErr)
     return NameOrErr.get();
   // If we have an error getting the name then we print the index of the archive

 }
 
 // Returns a map from sections to their relocations.
 static std::map<SectionRef, std::vector<RelocationRef>>
 getRelocsMap(object::ObjectFile const &Obj) {
   std::map<SectionRef, std::vector<RelocationRef>> Ret;
   for (SectionRef Sec : Obj.sections()) {
     section_iterator Relocated = Sec.getRelocatedSection();
-    if (Relocated == Obj.section_end() || !shouldKeep(*Relocated))
+    if (Relocated == Obj.section_end() || !checkSectionFilter(*Relocated).Keep)
       continue;
     std::vector<RelocationRef> &V = Ret[*Relocated];
     for (const RelocationRef &R : Sec.relocations())
       V.push_back(R);
     // Sort relocations by address.
     llvm::stable_sort(V, isRelocAddressLess);
   }
   return Ret;
static bool shouldDisplayLMA(const ObjectFile *Obj) {
   if (!Obj->isELF())
     return false;
   for (const SectionRef &S : ToolSectionFilter(*Obj))
     if (S.getAddress() != getELFSectionLMA(S))
       return true;
   return ShowLMA;
 }
 
+static size_t getMaxSectionNameWidth(const ObjectFile *Obj) {
+  // Default column width for names is 13 even if no names are that long.
+  size_t MaxWidth = 13;
+  for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
+    StringRef Name = unwrapOrError(Section.getName(), Obj->getFileName());
+    MaxWidth = std::max(MaxWidth, Name.size());
+  }
+  return MaxWidth;
+}
+
 void printSectionHeaders(const ObjectFile *Obj) {
+  size_t NameWidth = getMaxSectionNameWidth(Obj);
+  size_t AddressWidth = 2 * Obj->getBytesInAddress();
   bool HasLMAColumn = shouldDisplayLMA(Obj);
   if (HasLMAColumn)
     outs() << "Sections:\n"
-              "Idx Name          Size     VMA              LMA              "
-              "Type\n";
+              "Idx "
+           << left_justify("Name", NameWidth) << " Size     "
+           << left_justify("VMA", AddressWidth) << " "
+           << left_justify("LMA", AddressWidth) << " Type\n";
   else
     outs() << "Sections:\n"
-              "Idx Name          Size     VMA          Type\n";
+              "Idx "
+           << left_justify("Name", NameWidth) << " Size     "
+           << left_justify("VMA", AddressWidth) << " Type\n";
 
-  for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
+  uint64_t Idx;
+  for (const SectionRef &Section : ToolSectionFilter(*Obj, &Idx)) {
     StringRef Name = unwrapOrError(Section.getName(), Obj->getFileName());
     uint64_t VMA = Section.getAddress();
     if (shouldAdjustVA(Section))
       VMA += AdjustVMA;
 
     uint64_t Size = Section.getSize();
-    bool Text = Section.isText();
-    bool Data = Section.isData();
-    bool BSS = Section.isBSS();
-    std::string Type = (std::string(Text ? "TEXT " : "") +
-                        (Data ? "DATA " : "") + (BSS ? "BSS" : ""));
+
+    std::vector<std::string> SectionTypes;
+    if (Section.isText())
+      SectionTypes.push_back("TEXT");
+    if (Section.isData())
+      SectionTypes.push_back("DATA");
+    if (Section.isBSS())
+      SectionTypes.push_back("BSS");
+    std::string Type = llvm::join(SectionTypes, " ");
 
     if (HasLMAColumn)
-      outs() << format("%3d %-13s %08" PRIx64 " %016" PRIx64 " %016" PRIx64
-                       " %s\n",
-                       (unsigned)Section.getIndex(), Name.str().c_str(), Size,
-                       VMA, getELFSectionLMA(Section), Type.c_str());
-    else
-      outs() << format("%3d %-13s %08" PRIx64 " %016" PRIx64 " %s\n",
-                       (unsigned)Section.getIndex(), Name.str().c_str(), Size,
-                       VMA, Type.c_str());
+      outs() << format("%3d %-*s %08" PRIx64 " ", Idx, NameWidth,
+                       Name.str().c_str(), Size)
+             << format_hex_no_prefix(VMA, AddressWidth) << " "
+             << format_hex_no_prefix(getELFSectionLMA(Section), AddressWidth)
+             << " " << Type << "\n";
+    else
+      outs() << format("%3d %-*s %08" PRIx64 " ", Idx, NameWidth,
+                       Name.str().c_str(), Size)
+             << format_hex_no_prefix(VMA, AddressWidth) << " " << Type << "\n";
   }
   outs() << "\n";
 }
 
 void printSectionContents(const ObjectFile *Obj) {
   for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
     StringRef Name = unwrapOrError(Section.getName(), Obj->getFileName());
     uint64_t BaseAddr = Section.getAddress();