[PATCH] D143310: [llvm-readobj] Make relocations output valid JSON

Mon Feb 6 01:04:55 PST 2023

jhenderson added a reviewer: paulkirth.
jhenderson added a subscriber: paulkirth.
jhenderson added a comment.

Thanks for the fix. Could you please post the output without your change, just so that it's obvious what's going on before this fix?

I've also added @paulkirth as a reviewer, as he's recently posted a whole series of patches to fix JSON things. I haven't had a chance to go through them yet due to a combination of vacation and catching up with my regular work, but it's possible there's some overlap.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/json-relocations.test:7-19
+# CHECK:        "Section (1) .rela.text": {
+# CHECK-NEXT:          "Relocation": {
+# CHECK-NEXT:            "Offset": 0,
+# CHECK-NEXT:            "Type": {
+# CHECK-NEXT:              "Value": "R_X86_64_PC32",
+# CHECK-NEXT:              "RawValue": 2
+# CHECK-NEXT:            },
----------------
Nit: you can get rid of the excessive indentation for the purposes of the test. Just have it all indented consistently.

================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6640-6642
+    std::stringstream DictName;
+    DictName << "Section (" << std::to_string(SecNdx) << ") " << Name.str();
+    DictScope D(W, DictName.str());
----------------
If I'm not mistaken, you can use Twines to do this. Something along the lines of the inline edit (NB: I've not got quick access to check the exact final converesion to string, but it'll be something like this).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143310/new/

https://reviews.llvm.org/D143310