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

Mon Feb 6 09:49:40 PST 2023

aidengrossman added inline comments.

================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6641
+    std::stringstream DictName;
+    DictName << "Section (" << std::to_string(SecNdx) << ") " << Name.str();
+    DictScope D(W, DictName.str());
----------------
paulkirth wrote:
> I’m not sure that this is the approach we should be going for with JSON output. Ideally we’d be providing better structure than the text output and not lumping things together like this. 
> 
> My take for some relocs is here https://reviews.llvm.org/D137089.  That builds on earlier work cleaning up and making changes to the json output. There’s about 7 patches in that stack so once you dig in you’ll find lots of things actually need fixing in the JSON output. 
I totally didn't see that you had a patch up for relocations before writing this. There's definitely a lot of things that need fixing for JSON output. This patch was intended solely to get output from `--relocs --expand-relocs` parseable, but richer information would definitely be more useful. Based on a quick glance, it seems like this should be complementary to the current patches you have up (so we're not duplicating effort)? I'll work on some modifications to make the output richer/more JSONic.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143310