[obj2yaml] Add -o to specify output filename

[MaskRay]

Summary

-o is very common among tools. yaml2obj supports -o and it surprised me that
obj2yaml doesn't support -o. Just add it which doesn't take much code.

[jhenderson]

llvm/test/tools/obj2yaml/Archives/regular.yaml

11–13

I wonder if we could avoid some test duplication in these test cases by doing the following pattern instead:

# RUN: obj2yaml %t.empty.a > %t.stdout.yaml
# RUN: obj2yaml %t.empty.a -o %t.file.yaml 2>&1 | count 0
# RUN: FileCheck --input-file=%t.stdout.yaml %s --check-prefix=EMPTY
# RUN: diff %t.stdout.yaml %t.file.yaml

What do you think?

The same sort of pattern could apply to the other test cases too.

llvm/tools/obj2yaml/obj2yaml.cpp

64

I guess there isn't an appropriate offloading test case you could update like you have with the others?

114

Should we have a test case that if there was an error, the output file is not kept? It seems like a simple extension to me.

[MaskRay]

address comments

llvm/tools/obj2yaml/obj2yaml.cpp

64

Found a suitable test: updated llvm/test/ObjectYAML/Offload/default.yaml

[jhenderson]

LGTM, thanks!

llvm/test/ObjectYAML/Offload/default.yaml

-# RUN: yaml2obj %s | obj2yaml | FileCheck %s
+# RUN: yaml2obj %s -o %t
+# RUN: obj2yaml %t > %t.stdout.yaml
+# RUN: obj2yaml %t -o %t.file.yaml 2>&1 | count 0
+# RUN: FileCheck --input-file=%t.stdout.yaml %s
+# RUN: diff %t.stdout.yaml %t.file.yaml
 !Offload
 Members:
   - 
 
 # CHECK: --- !Offload
 # CHECK-NEXT: Members:
 # CHECK-NEXT:   - ImageKind:       IMG_None
 # CHECK-NEXT:     OffloadKind:     OFK_None
 # CHECK-NEXT:     Flags:           0
 # CHECK-NEXT: ...

llvm/test/ObjectYAML/wasm/header.yaml

-# RUN: yaml2obj %s | obj2yaml | FileCheck %s
+# RUN: yaml2obj %s -o %t
+# RUN: obj2yaml %t > %t.stdout.yaml
+# RUN: obj2yaml %t -o %t.file.yaml 2>&1 | count 0
+# RUN: FileCheck --input-file=%t.stdout.yaml %s
+# RUN: diff %t.stdout.yaml %t.file.yaml
 --- !WASM
 FileHeader:
   Version:         0x00000001
 ...
 # CHECK: --- !WASM
 # CHECK: FileHeader:
 # CHECK:   Version:           0x1
 # CHECK: ...

llvm/test/tools/obj2yaml/Archives/regular.yaml

 ## Check how obj2yaml dumps regular archives.
 
 ## Check how we dump an empty archive.
 
 # RUN: yaml2obj %s --docnum=1 -o %t.empty.a
-# RUN: obj2yaml %t.empty.a | FileCheck %s --check-prefix=EMPTY
+# RUN: obj2yaml %t.empty.a > %t.stdout.yaml
+# RUN: obj2yaml %t.empty.a -o %t.file.yaml 2>&1 | count 0
+# RUN: FileCheck --input-file=%t.stdout.yaml %s --check-prefix=EMPTY
+# RUN: diff %t.stdout.yaml %t.file.yaml
 
 # EMPTY:      --- !Arch
 # EMPTY-NEXT: Members: []
 # EMPTY-NEXT: ...
 
 --- !Arch
 Members: []
 
 ## Check how we dump archives with multiple members.
 ## Check we don't dump excessive spaces when dumping fields.
 ## Check we don't dump fields with values that are equal to default values.
 ## Check how we dump empty field values.

llvm/test/tools/obj2yaml/COFF/test-1.test

-# RUN: obj2yaml %S/Inputs/test-1.o | yaml2obj -o %t.o
+# RUN: obj2yaml %S/Inputs/test-1.o > %t.stdout.yaml
+# RUN: obj2yaml %S/Inputs/test-1.o -o %t.file.yaml 2>&1 | count 0
+# RUN: yaml2obj %t.stdout.yaml -o %t.o
 # RUN: llvm-pdbutil dump --types %t.o | FileCheck %s -check-prefix=ALL
+# RUN: diff %t.stdout.yaml %t.file.yaml
 
 # ALL: {{.*}} guid = {00C903AB-0968-4639-84F8-7D3E719A1BE1}

llvm/test/tools/obj2yaml/DXContainer/DXILPart.yaml

-# RUN: yaml2obj %s | obj2yaml | FileCheck %s 
+# RUN: yaml2obj %s -o %t
+# RUN: obj2yaml %t > %t.stdout.yaml
+# RUN: obj2yaml %t -o %t.file.yaml 2>&1 | count 0
+# RUN: FileCheck --input-file=%t.stdout.yaml %s
+# RUN: diff %t.stdout.yaml %t.file.yaml
 
 --- !dxcontainer
 Header:
   Hash:            [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 
                      0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
   Version:
     Major:           1
     Minor:           0

llvm/test/tools/obj2yaml/Minidump/basic.yaml

-# RUN: yaml2obj %s | obj2yaml - | FileCheck %s
+# RUN: yaml2obj %s -o %t
+# RUN: obj2yaml %t > %t.stdout.yaml
+# RUN: obj2yaml %t -o %t.file.yaml 2>&1 | count 0
+# RUN: FileCheck --input-file=%t.stdout.yaml %s
+# RUN: diff %t.stdout.yaml %t.file.yaml
 
 --- !minidump
 Streams:
   - Type:            SystemInfo
     Processor Arch:  BP_ARM64
     Platform ID:     Linux
     CSD Version:     Linux 3.13.0-91-generic
     CPU:

llvm/test/tools/obj2yaml/XCOFF/aix.yaml

 ## This is a basic test to check if we can parse the XCOFF object file correctly.
 # RUN: yaml2obj %s -DMAGIC=0x01DF -o %t-32
 # RUN: obj2yaml %t-32 | FileCheck %s --check-prefix=CHECK32
 # RUN: yaml2obj %s -DMAGIC=0x01F7 -o %t-64
-# RUN: obj2yaml %t-64 | FileCheck %s --check-prefix=CHECK64
+# RUN: obj2yaml %t-64 -o %t-64.yaml 2>&1 | count 0
+# RUN: FileCheck --input-file %t-64.yaml %s --check-prefix=CHECK64
 
 # CHECK32:      --- !XCOFF
 # CHECK32-NEXT: FileHeader:
 # CHECK32-NEXT:   MagicNumber:     0x1DF
 # CHECK32-NEXT:   NumberOfSections: 2
 # CHECK32-NEXT:   CreationTime:    0
 # CHECK32-NEXT:   OffsetToSymbolTable: 0x80
 # CHECK32-NEXT:   EntriesInSymbolTable: 4

llvm/test/tools/obj2yaml/output-file.yaml

+## Test that -o sets the output file name.
+
+# RUN: yaml2obj %s -o %t
+# RUN: rm -f %t.yaml && obj2yaml %t -o %t.yaml
+# RUN: ls %t.yaml
+# RUN: rm -f %t.yaml && cat %t | obj2yaml -o%t.yaml
+# RUN: ls %t.yaml
+
+## In case of an error, don't create the output file.
+# RUN: rm -f %t.yaml
+# RUN: echo | not obj2yaml -o %t.yaml
+# RUN: not ls %t.yaml
+
+# RUN: not obj2yaml %t -o %p/path/does/not/exist 2>&1 | FileCheck -DMSG=%errc_ENOENT %s
+
+# CHECK: obj2yaml: error: failed to open '{{.*}}/path/does/not/exist': [[MSG]]
+
+!ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_NONE

llvm/tools/obj2yaml/obj2yaml.cpp

 //===------ utils/obj2yaml.cpp - obj2yaml conversion tool -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "obj2yaml.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/Minidump.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/WithColor.h"
 
 using namespace llvm;
 using namespace llvm::object;
 
 static cl::opt<std::string>
     InputFilename(cl::Positional, cl::desc("<input file>"), cl::init("-"));
+static cl::opt<std::string> OutputFilename("o", cl::desc("Output filename"),
+                                           cl::value_desc("filename"),
+                                           cl::init("-"), cl::Prefix);
 static cl::bits<RawSegments> RawSegment(
     "raw-segment",
     cl::desc("Mach-O: dump the raw contents of the listed segments instead of "
              "parsing them:"),
     cl::values(clEnumVal(data, "__DATA"), clEnumVal(linkedit, "__LINKEDIT")));
 
-static Error dumpObject(const ObjectFile &Obj) {
+static Error dumpObject(const ObjectFile &Obj, raw_ostream &OS) {
   if (Obj.isCOFF())
-    return errorCodeToError(coff2yaml(outs(), cast<COFFObjectFile>(Obj)));
+    return errorCodeToError(coff2yaml(OS, cast<COFFObjectFile>(Obj)));
 
   if (Obj.isXCOFF())
-    return xcoff2yaml(outs(), cast<XCOFFObjectFile>(Obj));
+    return xcoff2yaml(OS, cast<XCOFFObjectFile>(Obj));
 
   if (Obj.isELF())
-    return elf2yaml(outs(), Obj);
+    return elf2yaml(OS, Obj);
 
   if (Obj.isWasm())
-    return errorCodeToError(wasm2yaml(outs(), cast<WasmObjectFile>(Obj)));
+    return errorCodeToError(wasm2yaml(OS, cast<WasmObjectFile>(Obj)));
 
   llvm_unreachable("unexpected object file format");
 }
 
-static Error dumpInput(StringRef File) {
+static Error dumpInput(StringRef File, raw_ostream &OS) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
       MemoryBuffer::getFileOrSTDIN(File, /*IsText=*/false,
                                    /*RequiresNullTerminator=*/false);
   if (std::error_code EC = FileOrErr.getError())
     return errorCodeToError(EC);
   std::unique_ptr<MemoryBuffer> &Buffer = FileOrErr.get();
   MemoryBufferRef MemBuf = Buffer->getMemBufferRef();
   switch (identify_magic(MemBuf.getBuffer())) {
   case file_magic::archive:
-    return archive2yaml(outs(), MemBuf);
+    return archive2yaml(OS, MemBuf);
   case file_magic::dxcontainer_object:
-    return dxcontainer2yaml(outs(), MemBuf);
+    return dxcontainer2yaml(OS, MemBuf);
   case file_magic::offload_binary:
-    return offload2yaml(outs(), MemBuf);
+    return offload2yaml(OS, MemBuf);
   default:
     break;
   }
 
   Expected<std::unique_ptr<Binary>> BinOrErr =
       createBinary(MemBuf, /*Context=*/nullptr);
   if (!BinOrErr)
     return BinOrErr.takeError();
 
   Binary &Binary = *BinOrErr->get();
   // Universal MachO is not a subclass of ObjectFile, so it needs to be handled
   // here with the other binary types.
   if (Binary.isMachO() || Binary.isMachOUniversalBinary())
-    return macho2yaml(outs(), Binary, RawSegment.getBits());
+    return macho2yaml(OS, Binary, RawSegment.getBits());
   if (ObjectFile *Obj = dyn_cast<ObjectFile>(&Binary))
-    return dumpObject(*Obj);
+    return dumpObject(*Obj, OS);
   if (MinidumpFile *Minidump = dyn_cast<MinidumpFile>(&Binary))
-    return minidump2yaml(outs(), *Minidump);
+    return minidump2yaml(OS, *Minidump);
 
   return Error::success();
 }
 
 static void reportError(StringRef Input, Error Err) {
   if (Input == "-")
     Input = "<stdin>";
   std::string ErrMsg;
   raw_string_ostream OS(ErrMsg);
   logAllUnhandledErrors(std::move(Err), OS);
   OS.flush();
   errs() << "Error reading file: " << Input << ": " << ErrMsg;
   errs().flush();
 }
 
 int main(int argc, char *argv[]) {
   InitLLVM X(argc, argv);
   cl::ParseCommandLineOptions(argc, argv);
 
-  if (Error Err = dumpInput(InputFilename)) {
+  std::error_code EC;
+  std::unique_ptr<ToolOutputFile> Out(
+      new ToolOutputFile(OutputFilename, EC, sys::fs::OF_None));
+  if (EC) {
+    WithColor::error(errs(), "obj2yaml")
+        << "failed to open '" + OutputFilename + "': " + EC.message() << '\n';
+    return 1;
+  }
+  if (Error Err = dumpInput(InputFilename, Out->os())) {
     reportError(InputFilename, std::move(Err));
     return 1;
   }
+  Out->keep();
 
   return 0;
 }