[llvm-readobj] Fix "Section" output when emitting relocations in JSON

[paulkirth]

Summary

Prior to this patch, the JSON output would emit an invalid key from the
shared LLVM implementation. This caused llvm-readobj to output invalid
JSON. This patch introduces a small helper function to print the
relocation information differently between the LLVM and JSON formats.

Before this patch:

"Relocations": [Section (2) .rel.text {

  {
    "Relocation": {
      "Offset": 0,
      "Type": {
        "Value": "R_X86_64_NONE",
        "RawValue": 0
      },
      "Symbol": {
        "Value": "rel_0",
        "RawValue": 1
      }
    }
  },
  ...

After this patch:

"Relocations": [
   {
     "SectionIdx": 2,
     "Relocs": [
       {
         "Relocation": {
           "Offset": 0,
           "Type": {
             "Name": "R_X86_64_NONE",
             "Value": 0
           },
           "Symbol": {
             "Name": "rel_0",
             "Value": 1
           }
         }
       },
       ...

[jhenderson]

Please provide before and after examples, and test cases for the fix. Also, please fix the title to use "llvm-readobj" in the commit message/patch description, not simply "readobj".

[paulkirth]

Rebase

• Update summary
• Add tests
• Simplify implementation to avoid code duplication

[paulkirth]

Rebase to fix patch application

[paulkirth]

Rebase

[paulkirth]

Rebase

[paulkirth]

Rebase.

[aidengrossman]

llvm/tools/llvm-readobj/ELFDumper.cpp

6659

If this section is already getting moved around, it might be worth it to transition to a DictScope here as well (similar to what I did in D143310, but with proper string formatting logic) to leave all the formatting in a centralized place and simplify this function.

[paulkirth]

Rebase.

[paulkirth]

Address comments

• Use scoped dict to simplify implementation and be more consistent

[paulkirth]

Rebase.

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

6656

I wonder if this would be better named printRelocationSectionInfo, since it's not printing any info about the relocations themselves.

7621

Any particular reason we don't just name this key "SectionIndex" or similar?

[paulkirth]

llvm/tools/llvm-readobj/ELFDumper.cpp

6656

That's a good point. I'll update this to reflect.

7621

No, I don't recall any a particular reason. I just went w/ the Idx suffix since it was still clear, and is a pretty common way to shorten or identify index variables. I don't have a preference here so I can change this when I address the other comments.

[paulkirth]

Rebase, address comments, and update tests.

[paulkirth]

Move test code into existing test file

[paulkirth]

Rebase

[paulkirth]

Fix bad rebase between two stacked diffs. I accidentally merged the Group section commit into this one, this commit should restore the correct split between the two.

[paulkirth]

Fix formatting

[jhenderson]

Looks good, with one nit.

llvm/tools/llvm-readobj/ELFDumper.cpp

6661

Nit: blank line between this and the next function.

[paulkirth]

Add missing blank line between functions.

[paulkirth]

Rebase and remove const qualifier from StringRef.

[paulkirth]

Add back funcion removed when rebasing

[paulkirth]

Add missing blank line.