Add JSONScopedPrinter to llvm-readelf

[Jaysonyan]

Summary

This change adds JSONScopedPrinter to llvm-readelf. It includes an empty JSONELFDumper class which will be used to override any LLVMELFDumper methods which utilize startLine() which JSONScopedPrinter cannot provide.

This change is split from D114052 and builds off of D114224

Depends on D114224

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

712

It's not clear to me why we inherit from LLVMELFDumper in this patch. This probably needs a comment at the very least.

llvm/tools/llvm-readobj/llvm-readobj.cpp

234–239

Probably time to turn this into a StringSwitch.

241–242

It would be great if this message included what the actual value specified was, and now's a good time to change it whilst you're here. Providing more context is useful, because it may not always be obvious what the problem is. For example, a command-line of "llvm-readobj --elf-output-syle="GNU my/input/file.o", resulting from a typo, would not always be immediately obvious where the issue is (especially if this was in a script and the input file was in a variable etc).

340–347

These two sets of if statements are actually in the wrong place, in my opinion: I think they should be behind interfaces in the ObjDumper and sub-classes, as appropriate, something like printFileSummary (which could take care of both).

I think it may be a slightly separate issue though. Here's my thinking:

1. LLVM style is controlled via the --elf-output-style option, which has different defaults for llvm-readelf and llvm-readobj.
2. The name of that option implies it has no impact for non-ELF formats, but this area of code is format-agnostic.
3. JSON output probably actually wants to print Format/Arch/AddressSize.
4. Everything else that is impacted by the --elf-output-style option is driven from the inheritance hierarchy. It seems odd that we have something that isn't.

At the very least, we could move the logic into virtual functions in ObjDumper, and then have JSON format provide its own behaviour. This would be a step in the right direction at least. What do you think?

561–563

[Jaysonyan]

llvm/tools/llvm-readobj/llvm-readobj.cpp

340–347

I agree that --elf-output-style shouldn't impact non-ELF formats. Although the work of adding a JSONScopedPrinter was not ELF-specific and it may be desirable to add json support for other formats in the future. This makes me wonder if --elf-output-style is an appropriate flag to enable json through or if it'd be preferable to add a format-agnostic flag (ex: --output-format). This could lend itself well if we choose to support json output of other obj formats in the future but could be confusing how --elf-output-style and --output-format would interact with each other. I'm not sure if this is something worth considering since there are currently no plans to support other obj formats but don't want to lock us in to an inconvenient flag choice for the future.

Otherwise, conceptually it makes sense to move this work into ObjDumper methods. One issue I see is that any usage of DictScope or ListScope relies on the ctor and dtor to output the opening/closing braces. So it may be difficult to just move this logic to other methods, especially if we have to initialize multiple DictScopes/ListScopes and just returning a unique_ptr would be insufficient.

[jhenderson]

llvm/tools/llvm-readobj/llvm-readobj.cpp

340–347

Yes, changing --elf-output-style to just --output-style was something I've considered a good idea for quite some time. This issue becomes more important as we add JSON support, but I think we can defer it to later on (it shouldn't block this work).

One issue I see is that any usage of DictScope or ListScope relies on the ctor and dtor to output the opening/closing braces.

I'm a little lost which constructor we're talking about here? The ObjDumper constructor or the DictScope/ListScope constructors?

[Jaysonyan]

llvm/tools/llvm-readobj/llvm-readobj.cpp

340–347

Referring to the DictScope/ListScope ctor/dtor. Since the opening braces are printed in the constructor and the closing braces are printed in the destructor, moving any instantiations of ListScope/DictScope inside a printFileSummary method becomes difficult since we would still need DictScope/ListScope to get destructed at the end of the dumpObject method rather than at the end of the printFileSummary method.

[jhenderson]

llvm/tools/llvm-readobj/llvm-readobj.cpp

340–347

Should the top-level DictScope be a member of the JSONELFDumper? Would that help solve this issue?

Related aside: perhaps in JSON mode, the file summary could become a separate object within the object for the file?

[Jaysonyan]

Update --elf-output-style error messaging.
Add printFileSummary method to ObjDumper.
Misc formatting.

[Jaysonyan]

Update formatting.

[Jaysonyan]

llvm/tools/llvm-readobj/ELFDumper.cpp

712

Added a comment! Let me know if you think it could provide more detail.

llvm/tools/llvm-readobj/llvm-readobj.cpp

234–239

I needed to provide a default option to know when to send an error message so I added an UNKOWN entry to the OutputStyelTy enum. If this seems inappropriate I can also change to just using a switch statement.

340–347

Adding the top level DictScope solves the problem! I also added the file summary info to its own object with attribute FileSummary.

[jhenderson]

Tip: add "Depends on DXXXXXX" to the patch description (but no need for it to be in the final commit message) to get Phabricator to auto-link this patch to the predecessor patch. I believe this helps the pre-merge checks, and also provides a nice little "Stack" view in the interface.

llvm/tools/llvm-readobj/ELFDumper.cpp

712

Thanks. Missing a trailing . though.

The limited nature of this new dumper class shows that we're not far off the ideal of having just one common class, which takes a ScopedPrinter (or JSONScopedPrinter), but I don't think we'll get there with this patch.

3250–3251

Old code used "\n" rather than '\n'. I think the former may be more common elsewhere in LLVM in general, although I'm not aware of any concrete coding standard.

Also, can't we use the existing W in the base class here, to keep the code identical to what it was before?

7319

Isn't this a behaviour change? Is it necesssary? If so, I'd defer it to a separate patch, if at all possible (or put it in a prerequisite patch), so that this patch is a pure NFC if a user doesn't specify --elf-output-style=json. I'm also slightly surprised I didn't see any test failures?

I'd suggest in this patch to call out to the base class version to do most of the work, although there's now a slight impedence to doing that in the form of the additional new line printed in one case but not the other, which I'd query.

The base class version was doing the JSON style version before, so I wonder if there's a good reason for this and the base class to diverge?

llvm/tools/llvm-readobj/llvm-readobj.cpp

234–239

UNKNOWN is a reasonable addition, in my opinion.

241–242

241–242

I think the text as it was was a little better, and I'd just add "but was \"foo\"" to the end.

Aside: general LLVM style for error messages is no leading capital letter.

241–242

Perhaps pull A->getValue() into a helper variable, since you use it in the error message below.

[Jaysonyan]

Update error messaging.
Misc formatting.

llvm/tools/llvm-readobj/ELFDumper.cpp

7319

Sorry, I'm not sure if I fully understand what constitutes as a behaviour change. This method would only be run when the user has specified --elf-output-style=JSON (which is introduced in this patch) so it shouldn't impact any existing behaviour.

The base class version calls W.startLine() which isn't well supported with JSONScopedPrinter and my expectation was to override any methods in the base class which use startLine(). Beyond that, the FileScope is needed in this implementation based on our previous discussion (D114225#inline-1092086) to achieve the desired JSON output discussed in the initial patch (D111658#3072213). If we relied solely on the base class, we wouldn't get each file in their own {}.

In my opinion, it makes sense to not break this change out into its own patch because it relies on the existence of a JSONELFDumper which is introduced in this patch and this patch relies on this change so that the output is in a reasonable format (each file has it's own object).

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

7319

Sorry, I'm not sure if I fully understand what constitutes as a behaviour change. This method would only be run when the user has specified --elf-output-style=JSON (which is introduced in this patch) so it shouldn't impact any existing behaviour.

Apologies, I think I was looking at the "changes since previous action" view, so didn't notice that this was in the JSONELFDumper function.

llvm/tools/llvm-readobj/llvm-readobj.cpp

249

Missed that you need a comma here.

354

I guess there's a subtle behaviour change as a result of this patch, which wouldn't hurt to be called out in the commit description, but I otherwise think is fine: if a user had exactly one non-ELF object/archive (e.g. a COFF object), and they had previously specified --elf-output-style=GNU (or run using llvm-readelf, which has that as the default), they would previously have had no file summary, but now they'd get the default summary, provided by the ObjDumper class. Similarly, if they'd had more than one input, or the objects were in an archive, they'd before have had just the File part of the summary, but now they have the full file summary.

628–630

To recap: we only need this scope for JSON, so that there is a top-level object, ensuring we always have valid JSON in the output. What's stopping us making the ListScope a member of the JSONScopedPrinter class, so that the scoping is added on construction/destruction of the Writer?

Taking it from another point of view, if you had another piece of client code using JSONScopedPrinter, would they expect the output to be a valid JSON object, without additional changes, or expect to need to provide the outer object wrapping themselves? If the former, the scoping belongs to the JSCONScopedPrinter.

[jhenderson]

Oh, you should also add one test in this patch to show that the file summary output is correct for JSON output, with a single file, multiple files, and a single archive file (possibly with only one object).

[Jaysonyan]

Add missing comma in error message.

llvm/tools/llvm-readobj/llvm-readobj.cpp

354

I placed the same opts::InputFilenames.size() > 1 || A check inside GNUELFDumper::printFileSummary method so I think the behaviour is the same.

Before, if the opts::Output is LLVM both if(...) statements would trigger. Now the ObjDumper::printFileSummary performs all the work in both if statements and would be called for any Dumper that is not GNUELFDumper or JSONELFDumper which should be any case where opts::Output is LLVM.

And the GNUELFDumper::printFileSummary only prints the File part and still does the multiple inputs / archive check.

628–630

I think providing an optional ListScope/DictScope constructor could be a nice quality-of-life improvement, but I think there should still be a way to construct a JSONScopedPrinter without {}/[] already added. Since technically, "string" is still valid json so I wouldn't want to remove the option to output json that don't have an outermost []/{}.

[Jaysonyan]

Add JSON FileSummary tests.
Add pretty-print option to llvm-readelf.

[Jaysonyan]

Add pretty-print test.

[jhenderson]

Please remember to update llvm/docs/CommandGuide/llvm-readelf.rst and llvm/docs/CommandGuide/llvm-readobj.rst as appropriate for the new options.

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

1 ↗

(On Diff #391945)

I'd rename this test file-summary-json.test.

I'd also test the entire output, so that you catch the opening and closing braces (if any) as well as any formatting points. To do this, I'd add --match-full-lines, --strict-whitespace and --implicit-check-not={{.}} to the FileCheck executions. These ensure that a) there is no other output on the same line (by default, FileCheck matches substrings); b) there is no additional whitespace on the line (by default, FileCheck ignores leading and trailing whitespace, and compresses all other whitespace into single ' ' characters); c) ensures the regex pattern . doesn't match anything that isn't covered by an existing check.

4 ↗

(On Diff #391945)

I'd avoid reusing the same output filename, as it makes things a little easier to debug sometimes when there's a test problem. Probably name the files %t.single, %t.multiple and %t.archive.

5 ↗

(On Diff #391945)

These RUN lines are getting quite long. I'd break them up as in the suggested edit. You can use the \ to continue a RUN statement onto the next line, so this might be useful too to allow the FileCheck command to run across multiple lines, when you add the extra options.

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

3 ↗

(On Diff #391945)

4–5 ↗

(On Diff #391945)

Same comments as above re. FileCheck additional options, output file name, and breaking up over multiple lines. Also applies below.

llvm/tools/llvm-readobj/Opts.td

31 ↗

(On Diff #391945)

llvm/tools/llvm-readobj/llvm-readobj.cpp

354

No, I don't think the behaviour is the same. Non-ELF objects (e.g. COFF/XCOFF etc) won't use the *ELFDumper classes, regardless of the --elf-output-style option's value. This is unchanged from before. The difference is that the opts::Output value now doesn't have a direct impact on the output (which is good), and only picks the dumper class to use for ELF objects. This means non-ELF objects will always have the default file summary behaviour.

I think this is a good behaviour change (GNU output style should have no effect for non-ELF objects), but it needs highlighting, as said already.

628–630

Yes, I see your point, and agree with you. Are you suggesting deferring to a separate patch, or as part of the original JSONScopedPrinter patch? I suggest the latter, because it will directly improve the code here, in my opinion.

[Jaysonyan]

Use JSONScopedPrinter ctor to initialize outer-most [].

[Jaysonyan]

Update docs.
Clean up tests.

[Jaysonyan]

llvm/tools/llvm-readobj/llvm-readobj.cpp

354

Sorry, don't mean to drag this on but I wanted to ensure we're on the same page.

opts::Output by default is initialized to LLVM so if --elf-output-style is not specified, then opts::Output will be LLVM. So I believe that even beforehand non-ELF objects would still have file summary information. Looking at a readobj tests with COFF here it looks like it's printing file summary info despite not being an ELF object.

628–630

Updated the JSONScopedPrinter patch to add this and use that change here.

[jhenderson]

llvm/docs/CommandGuide/llvm-readelf.rst

77 ↗

(On Diff #392298)

This line might need reflowing, to keep to a standard width (80 presumably).

78 ↗

(On Diff #392298)

JSON is an acronym, so should always be all-caps.

133 ↗

(On Diff #392298)

I'd suggest ", ... JSON output will be formatted in a more readable format." instead of the "will ..." part of the sentence.

llvm/docs/CommandGuide/llvm-readobj.rst

189 ↗

(On Diff #392298)

213 ↗

(On Diff #392298)

Use the same description as the other doc.

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

4 ↗

(On Diff #392298)

I'm being dumb: you only need one yaml2obj invocation for all test inputs. Delete the others, and rename the file %t or %t.o or similar.

6–7 ↗

(On Diff #392298)

Add some indentation to make it clear these are continuation lines. Same elsewhere.

56 ↗

(On Diff #392298)

llvm-ar r doesn't delete the existing archive or its contents, which can lead to unexpected results, if the test were aborted/updated in the future. Delete the archive file before recreating it here. Same below.

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

6 ↗

(On Diff #392298)

-D is usually a single-dash option; --check-prefix usually has two dashes.

llvm/tools/llvm-readobj/llvm-readobj.cpp

354

Default and --elf-output-style=LLVM behaviours are unchanged. I'm not talking about those. I'm talking about the following command-line:

$ llvm-readobj coff.obj --elf-output-style=GNU

Before this had no file summary. Now it does. I think it is okay if it does, but I think this should be called out in the commit description.

[Jaysonyan]

Update docs formatting.
Update file-summary and pretty print tests.

llvm/tools/llvm-readobj/llvm-readobj.cpp

354

Ok I see now, sorry for taking so long to understand. Will definitely call out this change in the commit description.

[jhenderson]

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

3–4 ↗

(On Diff #392686)

No need to rm an object file. That's only necessary for archive files (yaml2obj overwrites the existing file, whereas llvm-ar appends to it).

64–66 ↗

(On Diff #392686)

You should be able to be more precise, rather than skipping a chunk of the name. I think the relevant lit macro is %basename_t or something to that effect. You could pass that into an additional argument e.g. -DMEMBER.

[Jaysonyan]

Fix up file-summary test.

[Jaysonyan]

Just as a heads up, Dec 10th is the last day of my internship so unfortunately after that I won't be able to work on this change as actively as before. If there is still feedback to address a member of my team may be able to pick this up from where I left off. And thank you for taking the time to review all of my code changes these past months!

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

64–66 ↗

(On Diff #392686)

I was able to remove the {{.*}} by matching the structure of llvm-readobj/archive.test.

[jhenderson]

Looks good!

In D114225#3184814, @Jaysonyan wrote:

Just as a heads up, Dec 10th is the last day of my internship so unfortunately after that I won't be able to work on this change as actively as before. If there is still feedback to address a member of my team may be able to pick this up from where I left off. And thank you for taking the time to review all of my code changes these past months!

No problem. I think this is now in a good state to land the first sequence of patches. Hopefully you can get that done today.

[thakis]

The test fails on Windows: http://45.33.8.238/win/50629/step_11.txt

Please take a look and revert for now if it takes a while to fix.

[Jaysonyan]

In D114225#3186242, @thakis wrote:

The test fails on Windows: http://45.33.8.238/win/50629/step_11.txt

Please take a look and revert for now if it takes a while to fix.

Thank you for the heads up, I'm taking a look now, hopefully I'll have a fix soon and if not I'll revert these changes.

[jhenderson]

In D114225#3186298, @Jaysonyan wrote:

In D114225#3186242, @thakis wrote:

The test fails on Windows: http://45.33.8.238/win/50629/step_11.txt

Please take a look and revert for now if it takes a while to fix.

Thank you for the heads up, I'm taking a look now, hopefully I'll have a fix soon and if not I'll revert these changes.

An alternative test fix (and arguably a better one), would be to cd into the %t.dir directory after creating it, and then running all operations relative to the cwd, rather than the absolute paths %t translates into. You might want to consider making that change.

llvm/test/tools/llvm-readobj/ELF/output-style.test

 ## Error for an unknown output style.
 RUN: not llvm-readobj --elf-output-style=unknown 2>&1 | FileCheck %s
 
-CHECK: error: --elf-output-style value should be either 'LLVM' or 'GNU'
+CHECK: error: --elf-output-style value should be either 'LLVM', 'GNU', or 'JSON'

llvm/tools/llvm-readobj/ELFDumper.cpp

private:
   void printSectionMapping() override {}
   void printStackSizeEntry(uint64_t Size,
                            ArrayRef<std::string> FuncNames) override;
 
   void printMipsGOT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsPLT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsABIFlags() override;
 
+protected:
   ScopedPrinter &W;
 };
 
+template <typename ELFT> class JSONELFDumper : public LLVMELFDumper<ELFT> {
+public:
+  LLVM_ELF_IMPORT_TYPES_ELFT(ELFT)
+
+  JSONELFDumper(const object::ELFObjectFile<ELFT> &ObjF, ScopedPrinter &Writer)
+      : LLVMELFDumper<ELFT>(ObjF, Writer) {}
+};
+
 } // end anonymous namespace
 
 namespace llvm {
 
 template <class ELFT>
 static std::unique_ptr<ObjDumper>
 createELFDumper(const ELFObjectFile<ELFT> &Obj, ScopedPrinter &Writer) {
   if (opts::Output == opts::GNU)
     return std::make_unique<GNUELFDumper<ELFT>>(Obj, Writer);
+  else if (opts::Output == opts::JSON)
+    return std::make_unique<JSONELFDumper<ELFT>>(Obj, Writer);
   return std::make_unique<LLVMELFDumper<ELFT>>(Obj, Writer);
 }
 
 std::unique_ptr<ObjDumper> createELFDumper(const object::ELFObjectFileBase &Obj,
                                            ScopedPrinter &Writer) {
   // Little-endian 32-bit
   if (const ELF32LEObjectFile *ELFObj = dyn_cast<ELF32LEObjectFile>(&Obj))
     return createELFDumper(*ELFObj, Writer);
static const EnumEntry<unsigned> *getObjectFileEnumEntry(unsigned Type) {
   return nullptr;
 }
 
 template <class ELFT> void GNUELFDumper<ELFT>::printFileHeaders() {
   const Elf_Ehdr &e = this->Obj.getHeader();
   OS << "ELF Header:\n";
   OS << "  Magic:  ";
   std::string Str;
   for (int i = 0; i < ELF::EI_NIDENT; i++)
     OS << format(" %02x", static_cast<int>(e.e_ident[i]));
   OS << "\n";
   Str = printEnum(e.e_ident[ELF::EI_CLASS], makeArrayRef(ElfClass));
   printFields(OS, "Class:", Str);
   Str = printEnum(e.e_ident[ELF::EI_DATA], makeArrayRef(ElfDataEncoding));
   printFields(OS, "Data:", Str);
   OS.PadToColumn(2u);
   OS << "Version:";
   OS.PadToColumn(37u);
template <class ELFT> void LLVMELFDumper<ELFT>::printMipsABIFlags() {
   W.printEnum("ISA Extension", Flags->isa_ext, makeArrayRef(ElfMipsISAExtType));
   W.printFlags("ASEs", Flags->ases, makeArrayRef(ElfMipsASEFlags));
   W.printEnum("FP ABI", Flags->fp_abi, makeArrayRef(ElfMipsFpABIType));
   W.printNumber("GPR size", getMipsRegisterSize(Flags->gpr_size));
   W.printNumber("CPR1 size", getMipsRegisterSize(Flags->cpr1_size));
   W.printNumber("CPR2 size", getMipsRegisterSize(Flags->cpr2_size));
   W.printFlags("Flags 1", Flags->flags1, makeArrayRef(ElfMipsFlags1));
   W.printHex("Flags 2", Flags->flags2);
 }

llvm/tools/llvm-readobj/llvm-readobj.h

 namespace opts {
 extern bool SectionRelocations;
 extern bool SectionSymbols;
 extern bool SectionData;
 extern bool ExpandRelocs;
 extern bool RawRelr;
 extern bool CodeViewSubsectionBytes;
 extern bool Demangle;
-enum OutputStyleTy { LLVM, GNU };
+enum OutputStyleTy { LLVM, GNU, JSON };
 extern OutputStyleTy Output;
 } // namespace opts
 
 #define LLVM_READOBJ_ENUM_ENT(ns, enum) \
   { #enum, ns::enum }
 
 #define LLVM_READOBJ_ENUM_CLASS_ENT(enum_class, enum) \
   { #enum, std::underlying_type<enum_class>::type(enum_class::enum) }
 
 #endif

llvm/tools/llvm-readobj/llvm-readobj.cpp

static void parseOptions(const opt::InputArgList &Args) {
   opts::Symbols = Args.hasArg(OPT_symbols);
   opts::UnwindInfo = Args.hasArg(OPT_unwind);
 
   // ELF specific options.
   opts::DynamicTable = Args.hasArg(OPT_dynamic_table);
   opts::ELFLinkerOptions = Args.hasArg(OPT_elf_linker_options);
   if (Arg *A = Args.getLastArg(OPT_elf_output_style_EQ)) {
     StringRef V(A->getValue());
     if (V == "LLVM")
       opts::Output = opts::OutputStyleTy::LLVM;
     else if (V == "GNU")
       opts::Output = opts::OutputStyleTy::GNU;
+    else if (V == "JSON")
+      opts::Output = opts::OutputStyleTy::JSON;
     else
-      error("--elf-output-style value should be either 'LLVM' or 'GNU'");
+      error(
+          "--elf-output-style value should be either 'LLVM', 'GNU', or 'JSON'");
   }
   opts::GnuHashTable = Args.hasArg(OPT_gnu_hash_table);
   opts::HashSymbols = Args.hasArg(OPT_hash_symbols);
   opts::HashTable = Args.hasArg(OPT_hash_table);
   opts::HashHistogram = Args.hasArg(OPT_histogram);
   opts::NeededLibraries = Args.hasArg(OPT_needed_libs);
   opts::Notes = Args.hasArg(OPT_notes);
   opts::ProgramHeaders = Args.hasArg(OPT_program_headers);
   opts::RawRelr = Args.hasArg(OPT_raw_relr);
   opts::SectionGroups = Args.hasArg(OPT_section_groups);
   opts::VersionInfo = Args.hasArg(OPT_version_info);
 
   // Mach-O specific options.
   opts::MachODataInCode = Args.hasArg(OPT_macho_data_in_code);
   opts::MachODysymtab = Args.hasArg(OPT_macho_dysymtab);
ContentErrString = "unable to continue dumping, the file is corrupt: " +
                        toString(std::move(ContentErr));
 
   ObjDumper *Dumper;
   Expected<std::unique_ptr<ObjDumper>> DumperOrErr = createDumper(Obj, Writer);
   if (!DumperOrErr)
     reportError(DumperOrErr.takeError(), FileStr);
   Dumper = (*DumperOrErr).get();
 
-  if (opts::Output == opts::LLVM || opts::InputFilenames.size() > 1 || A) {
+  std::unique_ptr<DictScope> D;
+  if (opts::Output == opts::JSON) {
+    D = std::make_unique<DictScope>(Writer, FileStr);
+  } else if (opts::Output == opts::LLVM || opts::InputFilenames.size() > 1 ||
+             A) {
     Writer.startLine() << "\n";
     Writer.printString("File", FileStr);
   }
   if (opts::Output == opts::LLVM) {
     Writer.printString("Format", Obj.getFileFormatName());
     Writer.printString("Arch", Triple::getArchTypeName(Obj.getArch()));
     Writer.printString(
         "AddressSize",
         std::string(formatv("{0}bit", 8 * Obj.getBytesInAddress())));
     Dumper->printLoadName();
   }
 
   if (opts::FileHeaders)
     Dumper->printFileHeaders();
 
   if (Obj.isXCOFF() && opts::XCOFFAuxiliaryHeader)
     Dumper->printAuxiliaryHeader();
 
   // This is only used for ELF currently. In some cases, when an object is
else if (WindowsResource *WinRes = dyn_cast<WindowsResource>(Bin.get()))
     dumpWindowsResourceFile(WinRes, Writer);
   else
     llvm_unreachable("unrecognized file type");
 
   CVTypes.Binaries.push_back(
       OwningBinary<Binary>(std::move(Bin), std::move(Buffer)));
 }
 
+std::unique_ptr<ScopedPrinter> createWriter() {
+  if (opts::Output == opts::JSON) {
+    return std::make_unique<JSONScopedPrinter>(fouts());
+  }
+  return std::make_unique<ScopedPrinter>(fouts());
+}
+
 int main(int argc, char *argv[]) {
   InitLLVM X(argc, argv);
   BumpPtrAllocator A;
   StringSaver Saver(A);
   ReadobjOptTable Tbl;
   ToolName = argv[0];
   opt::InputArgList Args =
       Tbl.parseArgs(argc, argv, OPT_UNKNOWN, Saver, [&](StringRef Msg) {
int main(int argc, char *argv[]) {
 
   if (opts::Headers) {
     opts::FileHeaders = true;
     opts::XCOFFAuxiliaryHeader = true;
     opts::ProgramHeaders = true;
     opts::SectionHeaders = true;
   }
 
-  ScopedPrinter Writer(fouts());
+  std::unique_ptr<ScopedPrinter> Writer = createWriter();
+  std::unique_ptr<ListScope> D;
+  if (opts::Output == opts::JSON)
+    D = std::make_unique<ListScope>(*Writer.get());
+
   for (const std::string &I : opts::InputFilenames)
-    dumpInput(I, Writer);
+    dumpInput(I, *Writer.get());
 
   if (opts::CodeViewMergedTypes) {
     if (opts::CodeViewEnableGHash)
-      dumpCodeViewMergedTypes(Writer, CVTypes.GlobalIDTable.records(),
+      dumpCodeViewMergedTypes(*Writer.get(), CVTypes.GlobalIDTable.records(),
                               CVTypes.GlobalTypeTable.records());
     else
-      dumpCodeViewMergedTypes(Writer, CVTypes.IDTable.records(),
+      dumpCodeViewMergedTypes(*Writer.get(), CVTypes.IDTable.records(),
                               CVTypes.TypeTable.records());
   }
 
   return 0;
 }