[llvm-objdump] Print the DEBUG type under `--section-headers`.

[Esme]

Summary

Under the option --section-headers, we can only print the section types of TEXT, DATA, and BSS for now. This patch adds the DEBUG type.

[Higuoxing]

llvm/test/tools/llvm-objdump/ELF/PowerPC/section-headers-debug.test

30 ↗

(On Diff #345798)

You can specify the 'Size' field of the section rather than the 'Content' field, e.g.,

- Name:   .data
  Type:   SHT_PROGBITS
  Flags:  [ SHF_WRITE, SHF_ALLOC ]
  Size:   0x10

43 ↗

(On Diff #345798)

Nit: remove the blank line.

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

1771

I think we need some tests to check the output when the section has multiple types, e.g., 'DATA' and 'DEBUG'.

[jhenderson]

llvm/test/tools/llvm-objdump/ELF/PowerPC/section-headers-debug.test

1 ↗

(On Diff #345798)

Rather than having a test dedicated to a specific type field, it would make more sense to have a test that tests all type fields, probably as part of a generic --section-headers test. As such, before you add the DEBUG type, I'd recommend writing a new test/expanding an existing test for --section-headers, to test that option thoroughly (a quick skim of test/tools/llvm-objdump suggests there is no such test there, but there is objdump-sectionheaders.test in llvm/test/Object, which should probably be moved). This would be a separate patch, prior to this one.

Another point: the behaviour is not specific to PowerPC, and the test doesn't rely on the PowerPC target being available, so move the test into the ELF directory.

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

1 ↗

(On Diff #345798)

Similar comments as above. Don't have a dedicated test for the DEBUG type. Just expand the existing test.

Also, it would be preferable to use yaml2obj for this testing in the first place, rather than a canned binary, so rebase this patch on top of the related patches and wait for those to land.

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

1771

+1 - also I think this code is actually slightly incompatible with GNU output, which uses comma separators. It should be fixed to match.

[Esme]

Address comments.

[Esme]

This comment was removed by Esme.

llvm/test/tools/llvm-objdump/ELF/PowerPC/section-headers-debug.test

1 ↗

(On Diff #345798)

There is generic test, llvm/test/tools/llvm-objdump/section-headers.test, so I just expanding it with adding the test for type fields.

[Esme]

Format.

[Higuoxing]

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

1–5 ↗

(On Diff #347578)

It looks that xcoff-section-headers.o and xcoff-long-sec-names.o are useless after this change. Can we remove them?

9–17 ↗

(On Diff #347578)

Please insert a blank space between # and TYPE-NEXT.

[Esme]

Address comment.

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

1–5 ↗

(On Diff #347578)

xcoff-long-sec-names.o is useless, but xcoff-section-headers.o is used by other tests. Those tests may be replaced by yaml-docs in the future, which should be another work.

[jhenderson]

Seems to me like this is two separate patches now: 1) adding generic DEBUG printing support to llvm-objdump, then 2) adding XCOFF recognition of debug section types.

llvm/test/tools/llvm-objdump/XCOFF/section-headers-truncate.test

2 ↗

(On Diff #347588)

You can avoid a new canned binary here by using python to truncate another file. See for example llvm/test/tools/llvm-objcopy/ELF/invalid-e_phoff.test for an example of how to do this.

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

24 ↗

(On Diff #347588)

We probably need to add addresses to some of these too. If memory serves correctly, ELF yaml2obj auto-increments addresses for allocatable sections, if an adderss is not explicitly specified. Perhaps a similar feature could be added to XCOFF yaml2obj in a separate patch? What do you think? In the meantime, the addresses can presumably be done manually here.

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

154–155

I don't think we need an entirely new dedicated test case here. I think you would be better off expanding the first yaml doc in this file to cover the new DEBUG type and a case with multiple types (to pick up the separator change), and then just updating the first set of checks.

[Esme]

In D102603#2778965, @jhenderson wrote:

Seems to me like this is two separate patches now: 1) adding generic DEBUG printing support to llvm-objdump, then 2) adding XCOFF recognition of debug section types.

Split the patch and will post the second patch for XCOFF soon.

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

24 ↗

(On Diff #347588)

Perhaps a similar feature could be added to XCOFF yaml2obj in a separate patch?

Sounds good to me. I will implement the feature for XCOFF yaml2obj in a follow-up work.

[jhenderson]

LGTM, thanks!

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

 
 # 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-NEXT: {{^}}  5 .debug_abbrev 00000000 0000000000000000 0000000000000000 DEBUG{{$}}
+# WHITESPACE-NEXT: {{^}}  6 .debug_info   00000000 0000000000000000 0000000000000000 DATA, DEBUG{{$}}
 
 # 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 {{$}}
+# WHITESPACE-NO-LMA-NEXT: {{^}}  5 .debug_abbrev 00000000 0000000000000000 DEBUG{{$}}
+# WHITESPACE-NO-LMA-NEXT: {{^}}  6 .debug_info   00000000 0000000000000000 DATA, DEBUG{{$}}
 
 --- !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
+  - Name:  .debug_abbrev
+    Type:  SHT_PROGBITS
+  - Name:  .debug_info
+    Type:  SHT_PROGBITS
+    Flags: [ SHF_WRITE, SHF_ALLOC ]
 
 ## 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=2 -o %t-name13chars.o
 # RUN: llvm-objdump -h --show-lma %t-name13chars.o \
FileHeader:
   Class:   ELFCLASS[[BITS]]
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_386
 Sections:
   - Name:    .foo
     Type:    SHT_PROGBITS
     Flags:   [SHF_ALLOC, SHF_EXECINSTR]
     Address: 0x400

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

for (const SectionRef &Section : ToolSectionFilter(*Obj, &Idx)) {
     uint64_t VMA = Section.getAddress();
     if (shouldAdjustVA(Section))
       VMA += AdjustVMA;
 
     uint64_t Size = Section.getSize();
 
     std::string Type = Section.isText() ? "TEXT" : "";
     if (Section.isData())
-      Type += Type.empty() ? "DATA" : " DATA";
+      Type += Type.empty() ? "DATA" : ", DATA";
     if (Section.isBSS())
-      Type += Type.empty() ? "BSS" : " BSS";
+      Type += Type.empty() ? "BSS" : ", BSS";
+    if (Section.isDebugSection())
+      Type += Type.empty() ? "DEBUG" : ", DEBUG";
 
     if (HasLMAColumn)
       outs() << format("%3" PRIu64 " %-*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