[llvm-readobj] Include section information in summary

[paulkirth]

Summary

Since we include a summary in JSON output, it is useful to surface
aggregate information about the object, such as the total number of
sections, their size, and info on allocated/debug sections. This
has been useful in quickly understanding size issues without post
processing the output. There may be other types of information that
could be useful to summarize.

[ikudrin]

The information about the total size of the sections might be misleading because it does not take into account possible gaps between them.

llvm/tools/llvm-readobj/ELFDumper.cpp

7771–7790

It looks like you forget to include the actual patch in the review.

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/file-summary-json.test

20

TotalSections will be available in the file header already, so isn't this a bit superfluous?

[paulkirth]

Rebase and include base commit

• rename Total to Remaining

llvm/tools/llvm-readobj/ELFDumper.cpp

7771–7790

Indeed. I had forgotten to squash my commit when uploading. This should be fixed now.

[jhenderson]

I'm wary of extending the information specifically for JSON dumping, as it doesn't feel right to me that this output format should provide information that isn't provided in at least the LLVM format too (in practice, the two should be identical, barring how things are formatted, in my opinion).

You talk about how you are trying to understand size issues in this patch. Why does this mean JSON output specifically needs this info? llvm-size already provides similar sorts of information, so is this really the right tool for the job? Also, the "debug" versus "remaining" (which should probably be "other") line is not particularly well-defined. For example, this patch treats some sections that are stripped with --strip-debug in llvm-objcopy as "remaining".

[paulkirth]

In D146753#4291501, @jhenderson wrote:

I'm wary of extending the information specifically for JSON dumping, as it doesn't feel right to me that this output format should provide information that isn't provided in at least the LLVM format too (in practice, the two should be identical, barring how things are formatted, in my opinion).

AFAIK the LLVM format doesn't have a summary, while JSON does. This seemed like a good place to put some summary info.

You talk about how you are trying to understand size issues in this patch. Why does this mean JSON output specifically needs this info? llvm-size already provides similar sorts of information, so is this really the right tool for the job?

So if we're already processing a binary w/ llvm-objdump and consuming its JSON output it seems a lot nicer to get these statistics as part of the output rather than to invoke another tool. We can certainly reconstruct those values by post-processing the data, but it seemed trivial to add to the summary, since it was looping over the sections anyway.

Also, the "debug" versus "remaining" (which should probably be "other") line is not particularly well-defined. For example, this patch treats some sections that are stripped with --strip-debug in llvm-objcopy as "remaining".

"Other" was my first thought, but I went w/ "Remaining", since I thought it was less confusing. Looking at it, I'd have to agree that "Other" is probably a better name.

I wasn't all that confident on the way the current implementation classifies non-allocated sections. Do you have any suggestions as to what would be a better approach? I didn't think any of the other section flags were a strong enough indicator, but I'll be the first to admit that object file flags aren't something I'm particularly well versed in.

[jhenderson]

In D146753#4294137, @paulkirth wrote:

In D146753#4291501, @jhenderson wrote:

I'm wary of extending the information specifically for JSON dumping, as it doesn't feel right to me that this output format should provide information that isn't provided in at least the LLVM format too (in practice, the two should be identical, barring how things are formatted, in my opinion).

AFAIK the LLVM format doesn't have a summary, while JSON does. This seemed like a good place to put some summary info.

See ObjDumper::printFileSummary, which contains the default implementation for printFileSummary that LLVM output uses. Admittedly, "summary" is perhaps a poor name for it, given it doesn't actually summarise much.

You talk about how you are trying to understand size issues in this patch. Why does this mean JSON output specifically needs this info? llvm-size already provides similar sorts of information, so is this really the right tool for the job?

So if we're already processing a binary w/ llvm-objdump and consuming its JSON output it seems a lot nicer to get these statistics as part of the output rather than to invoke another tool. We can certainly reconstruct those values by post-processing the data, but it seemed trivial to add to the summary, since it was looping over the sections anyway.

(I assume you meant "llvm-readobj" here). You're not using the same section loop as any other section loop though, so you're making llvm-readobj unconditionally do more work for JSON output? I'm not opposed to the functionality per se, but I'd consider adding it under a new option. You'd probably want to get buy-in from the GNU community for the option name too, so that equivalent functionality can be added to GNU output (I don't see anything here that is inherent to JSON output).

Also, the "debug" versus "remaining" (which should probably be "other") line is not particularly well-defined. For example, this patch treats some sections that are stripped with --strip-debug in llvm-objcopy as "remaining".

"Other" was my first thought, but I went w/ "Remaining", since I thought it was less confusing. Looking at it, I'd have to agree that "Other" is probably a better name.

I wasn't all that confident on the way the current implementation classifies non-allocated sections. Do you have any suggestions as to what would be a better approach? I didn't think any of the other section flags were a strong enough indicator, but I'll be the first to admit that object file flags aren't something I'm particularly well versed in.

llvm-objcopy has a function for identifying debug sections (see https://github.com/llvm/llvm-project/blob/43436993f48b1d75d9d80796cb0889ce7d191888/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp#L54). We should mirror that, or better yet, try to pull that function into somewhere both tools can access it. The other thing to note is that strictly speaking, there's no requirement that debug sections are non-alloc. I could imagine a system where a debugger reads the in-memory file, rather than reading additional stuff off disk, and as such it would need SHF_ALLOC debug sections (I don't know of any practical examples of this, but I think we need to be careful about baking in assumptions around this area).

[paulkirth]

After the feedback, and some further thinking, I don't think this is the right approach. As @jhenderson pointed out there are some real mismatches with what I was trying to achieve and what this patch actually does. thanks everyone for their input, and dealing with the very slow progress/responses on my part.

llvm/test/tools/llvm-readobj/ELF/file-summary-json.test

 
 #      SINGLE:[
 # SINGLE-NEXT:  {
 # SINGLE-NEXT:    "FileSummary": {
 # SINGLE-NEXT:      "File": "{{.*}}/obj",
 # SINGLE-NEXT:      "Format": "elf64-x86-64",
 # SINGLE-NEXT:      "Arch": "x86_64",
 # SINGLE-NEXT:      "AddressSize": "64bit",
-# SINGLE-NEXT:      "LoadName": "<Not found>"
+# SINGLE-NEXT:      "LoadName": "<Not found>",
+# SINGLE-NEXT:      "TotalSections": 3,
+# SINGLE-NEXT:      "TotalSize": 20,
+# SINGLE-NEXT:      "AllocSections": 0,
+# SINGLE-NEXT:      "AllocSize": 0,
+# SINGLE-NEXT:      "DebugSections": 0,
+# SINGLE-NEXT:      "DebugSize": 0
 # SINGLE-NEXT:    }
 # SINGLE-NEXT:  }
 # SINGLE-NEXT:]
 
 ## Test outputting file summary for multiple files.
 # RUN: llvm-readobj --elf-output-style=JSON --pretty-print %t.dir/obj %t.dir/obj | \
 # RUN:   FileCheck %s --check-prefix=MULTI \
 # RUN:     --match-full-lines --strict-whitespace --implicit-check-not={{.}}
 
 #      MULTI:[
 # MULTI-NEXT:  {
 # MULTI-NEXT:    "FileSummary": {
 # MULTI-NEXT:      "File": "{{.*}}/obj",
 # MULTI-NEXT:      "Format": "elf64-x86-64",
 # MULTI-NEXT:      "Arch": "x86_64",
 # MULTI-NEXT:      "AddressSize": "64bit",
-# MULTI-NEXT:      "LoadName": "<Not found>"
+# MULTI-NEXT:      "LoadName": "<Not found>",
+# MULTI-NEXT:      "TotalSections": 3,
+# MULTI-NEXT:      "TotalSize": 20,
+# MULTI-NEXT:      "AllocSections": 0,
+# MULTI-NEXT:      "AllocSize": 0,
+# MULTI-NEXT:      "DebugSections": 0,
+# MULTI-NEXT:      "DebugSize": 0
 # MULTI-NEXT:    }
 # MULTI-NEXT:  },
 # MULTI-NEXT:  {
 # MULTI-NEXT:    "FileSummary": {
 # MULTI-NEXT:      "File": "{{.*}}/obj",
 # MULTI-NEXT:      "Format": "elf64-x86-64",
 # MULTI-NEXT:      "Arch": "x86_64",
 # MULTI-NEXT:      "AddressSize": "64bit",
-# MULTI-NEXT:      "LoadName": "<Not found>"
+# MULTI-NEXT:      "LoadName": "<Not found>",
+# MULTI-NEXT:      "TotalSections": 3,
+# MULTI-NEXT:      "TotalSize": 20,
+# MULTI-NEXT:      "AllocSections": 0,
+# MULTI-NEXT:      "AllocSize": 0,
+# MULTI-NEXT:      "DebugSections": 0,
+# MULTI-NEXT:      "DebugSize": 0
 # MULTI-NEXT:    }
 # MULTI-NEXT:  }
 # MULTI-NEXT:]
 
 ## Test outputting file summary for an archive with a single file.
 # RUN: rm -f %t.archive-single
 # RUN: llvm-ar rc %t.archive-single %t.dir/obj
 # RUN: llvm-readobj --elf-output-style=JSON --pretty-print %t.archive-single | \
 # RUN:   FileCheck %s --check-prefix=ARCH-SINGLE \
 # RUN:     --match-full-lines --strict-whitespace --implicit-check-not={{.}}
 
 #      ARCH-SINGLE:[
 # ARCH-SINGLE-NEXT:  {
 # ARCH-SINGLE-NEXT:    "FileSummary": {
 # ARCH-SINGLE-NEXT:      "File": "{{.*}}.archive-single(obj)",
 # ARCH-SINGLE-NEXT:      "Format": "elf64-x86-64",
 # ARCH-SINGLE-NEXT:      "Arch": "x86_64",
 # ARCH-SINGLE-NEXT:      "AddressSize": "64bit",
-# ARCH-SINGLE-NEXT:      "LoadName": "<Not found>"
+# ARCH-SINGLE-NEXT:      "LoadName": "<Not found>",
+# ARCH-SINGLE-NEXT:      "TotalSections": 3,
+# ARCH-SINGLE-NEXT:      "TotalSize": 20,
+# ARCH-SINGLE-NEXT:      "AllocSections": 0,
+# ARCH-SINGLE-NEXT:      "AllocSize": 0,
+# ARCH-SINGLE-NEXT:      "DebugSections": 0,
+# ARCH-SINGLE-NEXT:      "DebugSize": 0
 # ARCH-SINGLE-NEXT:    }
 # ARCH-SINGLE-NEXT:  }
 # ARCH-SINGLE-NEXT:]
 
 ## Test outputting file summary for an archive with multiple files.
 # RUN: rm -f %t.archive-multiple
 # RUN: llvm-ar rc %t.archive-multiple %t.dir/obj %t.dir/obj
 # RUN: llvm-readobj --elf-output-style=JSON --pretty-print %t.archive-multiple | \
 # RUN:   FileCheck %s --check-prefix=ARCH-MULTI \
 # RUN:     --match-full-lines --strict-whitespace --implicit-check-not={{.}}
 
 #      ARCH-MULTI:[
 # ARCH-MULTI-NEXT:  {
 # ARCH-MULTI-NEXT:    "FileSummary": {
 # ARCH-MULTI-NEXT:      "File": "{{.*}}.archive-multiple(obj)",
 # ARCH-MULTI-NEXT:      "Format": "elf64-x86-64",
 # ARCH-MULTI-NEXT:      "Arch": "x86_64",
 # ARCH-MULTI-NEXT:      "AddressSize": "64bit",
-# ARCH-MULTI-NEXT:      "LoadName": "<Not found>"
+# ARCH-MULTI-NEXT:      "LoadName": "<Not found>",
+# ARCH-MULTI-NEXT:      "TotalSections": 3,
+# ARCH-MULTI-NEXT:      "TotalSize": 20,
+# ARCH-MULTI-NEXT:      "AllocSections": 0,
+# ARCH-MULTI-NEXT:      "AllocSize": 0,
+# ARCH-MULTI-NEXT:      "DebugSections": 0,
+# ARCH-MULTI-NEXT:      "DebugSize": 0
 # ARCH-MULTI-NEXT:    }
 # ARCH-MULTI-NEXT:  },
 # ARCH-MULTI-NEXT:  {
 # ARCH-MULTI-NEXT:    "FileSummary": {
 # ARCH-MULTI-NEXT:      "File": "{{.*}}.archive-multiple(obj)",
 # ARCH-MULTI-NEXT:      "Format": "elf64-x86-64",
 # ARCH-MULTI-NEXT:      "Arch": "x86_64",
 # ARCH-MULTI-NEXT:      "AddressSize": "64bit",
-# ARCH-MULTI-NEXT:      "LoadName": "<Not found>"
+# ARCH-MULTI-NEXT:      "LoadName": "<Not found>",
+# ARCH-MULTI-NEXT:      "TotalSections": 3,
+# ARCH-MULTI-NEXT:      "TotalSize": 20,
+# ARCH-MULTI-NEXT:      "AllocSections": 0,
+# ARCH-MULTI-NEXT:      "AllocSize": 0,
+# ARCH-MULTI-NEXT:      "DebugSections": 0,
+# ARCH-MULTI-NEXT:      "DebugSize": 0
 # ARCH-MULTI-NEXT:    }
 # ARCH-MULTI-NEXT:  }
 # ARCH-MULTI-NEXT:]
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64

llvm/test/tools/llvm-readobj/ELF/pretty-print.test

 ## Test the JSON pretty-print flag.
 #
 # RUN: yaml2obj %s -o %t.pretty
 
 ## Test JSON with pretty-print off.
 # RUN: llvm-readobj --elf-output-style=JSON %t.pretty | \
 # RUN:   FileCheck %s --check-prefix=NO-PRETTY \
 # RUN:     --strict-whitespace --implicit-check-not={{.}}
 
 #      NO-PRETTY:[
 # NO-PRETTY-SAME:{
 # NO-PRETTY-SAME:"FileSummary":{
 # NO-PRETTY-SAME:"File":"{{.*}}.pretty",
 # NO-PRETTY-SAME:"Format":"elf64-x86-64",
 # NO-PRETTY-SAME:"Arch":"x86_64",
 # NO-PRETTY-SAME:"AddressSize":"64bit",
-# NO-PRETTY-SAME:"LoadName":"<Not found>"
+# NO-PRETTY-SAME:"LoadName":"<Not found>",
+# NO-PRETTY-SAME:"TotalSections":3,
+# NO-PRETTY-SAME:"TotalSize":20,
+# NO-PRETTY-SAME:"AllocSections":0,
+# NO-PRETTY-SAME:"AllocSize":0,
+# NO-PRETTY-SAME:"DebugSections":0,
+# NO-PRETTY-SAME:"DebugSize":0
 # NO-PRETTY-SAME:}
 # NO-PRETTY-SAME:}
 # NO-PRETTY-SAME:]
 
 ## Test JSON with pretty-print on.
 # RUN: llvm-readobj --elf-output-style=JSON --pretty-print %t.pretty | \
 # RUN:   FileCheck %s --check-prefix=PRETTY \
 # RUN:     --match-full-lines --strict-whitespace --implicit-check-not={{.}}
 
 #      PRETTY:[
 # PRETTY-NEXT:  {
 # PRETTY-NEXT:    "FileSummary": {
 # PRETTY-NEXT:      "File": "{{.*}}.pretty",
 # PRETTY-NEXT:      "Format": "elf64-x86-64",
 # PRETTY-NEXT:      "Arch": "x86_64",
 # PRETTY-NEXT:      "AddressSize": "64bit",
-# PRETTY-NEXT:      "LoadName": "<Not found>"
+# PRETTY-NEXT:      "LoadName": "<Not found>",
+# PRETTY-NEXT:      "TotalSections": 3,
+# PRETTY-NEXT:      "TotalSize": 20,
+# PRETTY-NEXT:      "AllocSections": 0,
+# PRETTY-NEXT:      "AllocSize": 0,
+# PRETTY-NEXT:      "DebugSections": 0,
+# PRETTY-NEXT:      "DebugSize": 0
 # PRETTY-NEXT:    }
 # PRETTY-NEXT:  }
 # PRETTY-NEXT:]
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64

llvm/tools/llvm-readobj/ELFDumper.cpp

void JSONELFDumper<ELFT>::printFileSummary(StringRef FileStr, ObjectFile &Obj,
   this->W.printString("File", FileStr);
   this->W.printString("Format", Obj.getFileFormatName());
   this->W.printString("Arch", Triple::getArchTypeName(Obj.getArch()));
   this->W.printString(
       "AddressSize",
       std::string(formatv("{0}bit", 8 * Obj.getBytesInAddress())));
   this->printLoadName();
 
-  int TotalSections = 0;
-  int AllocSections = 0;
-  int TotalSize = 0;
-  int AllocSize = 0;
-  int DebugSections = 0;
-  int DebugSize = 0;
+  uint64_t TotalSections = 0;
+  uint64_t AllocSections = 0;
+  uint64_t TotalSize = 0;
+  uint64_t AllocSize = 0;
+  uint64_t DebugSections = 0;
+  uint64_t DebugSize = 0;
   std::vector<EnumEntry<unsigned>> FlagsList =
       getSectionFlagsForTarget(this->Obj.getHeader().e_ident[ELF::EI_OSABI],
                                this->Obj.getHeader().e_machine);
   for (const Elf_Shdr &Sec : cantFail(this->Obj.sections())) {
     ++TotalSections;
     TotalSize += Sec.sh_size;
     if (Sec.sh_flags & SHF_ALLOC) {
       ++AllocSections;
       AllocSize += Sec.sh_size;
     } else if (this->getPrintableSectionName(Sec).startswith(".debug")) {
       ++DebugSections;
       ++DebugSize;
     }
   }
   this->W.printNumber("TotalSections", TotalSections);
   this->W.printNumber("TotalSize", TotalSize);
   this->W.printNumber("AllocSections", AllocSections);
   this->W.printNumber("AllocSize", AllocSize);
   this->W.printNumber("DebugSections", DebugSections);
   this->W.printNumber("DebugSize", DebugSize);
 }
 
 template <class ELFT>
 void JSONELFDumper<ELFT>::printZeroSymbolOtherField(
     const Elf_Sym &Symbol) const {
   // We want the JSON format to be uniform, since it is machine readable, so
   // always print the `Other` field the same way.
   this->printSymbolOtherField(Symbol);