[yaml2obj] Add new command line option `-docnum`

[atanasyan]

Summary

Input YAML file might contain multiple object file definitions. New option -docnum allows to specify an ordinal number (starting from 1) of definition used for an object file generation.

[silvas]

lib/Support/YAMLTraits.cpp

93–95

This seems like it should be a separately tested bug fix for YAMLIO

tools/yaml2obj/yaml2elf.cpp

498–508

Can we pull this loop up into yaml2obj.cpp and share it between yaml2coff and yaml2elf? I would prefer if all uses of the DocNum cl::opt are kept in the same file (and for there to be just one use).

The current way you are doing this also injects a false dependency on the document type (ELFYAML::Object or COFFYAML::Object) into the loop. It should be possible to skip documents without knowing their type (think Postel's Law). Essentially I'm imagining something like this Python code:

yaml2which = ... # yaml2coff or yaml2elf
for i, doc in enumerate(documents, 1):
    if i == docnum:
        yaml2which(doc)
        break
else:
    errs() << ...

[atanasyan]

Good point. I am going to send a new patch which moves the loop to the yaml2obj.cpp soon.

lib/Support/YAMLTraits.cpp

93–95

It is not a bug fix. Function Input::nextDocument() just incremented the DocIterator. We checked the end of documents stream in the Input::setCurrentDocument(). Now the Input::nextDocument() increments the iterator and return its "status".

[atanasyan]

Move the code which iterates YAML documents to the yaml2obj.cpp.

[silvas]

LGTM with the tiny naming nit above about convertYaml/convertYAML.

I'm slightly worried that using indices could end up being brittle as test files are changed, but I can't think of a better way to select a document without introducing a lot of complexity. We can always change it later if it turns out to be a source of bugs.

tools/yaml2obj/yaml2obj.cpp

65

Tiny nit: typically when we use the acronym "yaml" in camel case, we spell it with all caps, so this function should be "convertYAML".

There are good arguments for doing it the way you have here (My favorite: HTTPSend/HTTPSEnd vs HttpSend/HttpsEnd), but I'd rather stick to convention.

[atanasyan]

Thanks for review.

Initially I planed to implement something like -docname option. But I could not figure out how to name YAML documents. We can use tags for that but it is a good idea to reserve the tags for output file format specification (ELF | COFF). Maybe solution is to support some sort of "complex" tag format like !ELF|<doc label>.

Closed by commit rL209967.

include/llvm/Support/YAMLTraits.h

private:
   void setError(HNode *hnode, const Twine &message);
   void setError(Node *node, const Twine &message);
 
 
 public:
   // These are only used by operator>>. They could be private
   // if those templated things could be made friends.
   bool setCurrentDocument();
-  void nextDocument();
+  bool nextDocument();
 
 private:
   llvm::SourceMgr                     SrcMgr; // must be before Strm
   std::unique_ptr<llvm::yaml::Stream> Strm;
   std::unique_ptr<HNode>              TopNode;
   llvm::error_code                    EC;
   llvm::BumpPtrAllocator              StringAllocator;
   llvm::yaml::document_iterator       DocIterator;

lib/Support/YAMLTraits.cpp

if (DocIterator != Strm->end()) {
     }
     TopNode.reset(this->createHNodes(N));
     CurrentNode = TopNode.get();
     return true;
   }
   return false;
 }
 
-void Input::nextDocument() {
-  ++DocIterator;
+bool Input::nextDocument() {
+  return ++DocIterator != Strm->end();
 }
 
 bool Input::mapTag(StringRef Tag, bool Default) {
   std::string foundTag = CurrentNode->_node->getVerbatimTag();
   if (foundTag.empty()) {
     // If no tag found and 'Tag' is the default, say it was found.
     return Default;
   }
   // Return true iff found tag matches supplied tag.

test/Object/yaml2obj-coff-multi-doc.test

+# RUN: yaml2obj -format=coff -docnum=1 %s \
+# RUN:   | llvm-readobj -symbols - | FileCheck -check-prefix=DOC1 %s
+# RUN: yaml2obj -format=coff -docnum=2 %s \
+# RUN:   | llvm-readobj -symbols - | FileCheck -check-prefix=DOC2 %s
+# RUN: not yaml2obj -format=coff -docnum=3 %s 2>&1 \
+# RUN:   | FileCheck -check-prefix=DOC3 %s
+
+# DOC1: Name: _sym1
+# DOC2: Name: _sym2
+# DOC3: yaml2obj: Cannot find the 3rd document
+
+---
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_DEBUG_STRIPPED ]
+
+sections:
+  - Name: .text
+    Alignment: 16
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE,
+                       IMAGE_SCN_MEM_READ ]
+    SectionData: "00000000"
+
+symbols:
+  - Name: .text
+    Value: 0
+    SectionNumber: 1
+    SimpleType: IMAGE_SYM_TYPE_NULL
+    ComplexType: IMAGE_SYM_DTYPE_NULL
+    StorageClass: IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          36
+      NumberOfRelocations: 3
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          1
+
+  - Name: _main
+    Value: 0
+    SectionNumber: 1
+    SimpleType: IMAGE_SYM_TYPE_NULL
+    ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+
+  - Name: _sym1
+    Value: 0
+    SectionNumber: 0
+    SimpleType: IMAGE_SYM_TYPE_NULL
+    ComplexType: IMAGE_SYM_DTYPE_NULL
+    StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+
+---
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_DEBUG_STRIPPED ]
+
+sections:
+  - Name: .text
+    Alignment: 16
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE,
+                       IMAGE_SCN_MEM_READ ]
+    SectionData: "00000000"
+
+symbols:
+  - Name: .text
+    Value: 0
+    SectionNumber: 1
+    SimpleType: IMAGE_SYM_TYPE_NULL
+    ComplexType: IMAGE_SYM_DTYPE_NULL
+    StorageClass: IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          36
+      NumberOfRelocations: 3
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          1
+
+  - Name: _main
+    Value: 0
+    SectionNumber: 1
+    SimpleType: IMAGE_SYM_TYPE_NULL
+    ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+
+  - Name: _sym2
+    Value: 0
+    SectionNumber: 0
+    SimpleType: IMAGE_SYM_TYPE_NULL
+    ComplexType: IMAGE_SYM_DTYPE_NULL
+    StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+...

test/Object/yaml2obj-elf-multi-doc.test

+# RUN: yaml2obj -format=elf -docnum=1 %s \
+# RUN:   | llvm-readobj -symbols - | FileCheck -check-prefix=DOC1 %s
+# RUN: yaml2obj -format=elf -docnum=2 %s \
+# RUN:   | llvm-readobj -symbols - | FileCheck -check-prefix=DOC2 %s
+# RUN: not yaml2obj -format=elf -docnum=3 %s 2>&1 \
+# RUN:   | FileCheck -check-prefix=DOC3 %s
+
+# DOC1: Name: T1 (1)
+# DOC2: Name: T2 (1)
+# DOC3: yaml2obj: Cannot find the 3rd document
+
+--- !ELF
+FileHeader: !FileHeader
+  Class: ELFCLASS32
+  Data: ELFDATA2LSB
+  Type: ET_REL
+  Machine: EM_MIPS
+  Flags: [EF_MIPS_CPIC]
+
+Sections:
+- Name: .text
+  Type: SHT_PROGBITS
+  Content:  "0000000000000000"
+  AddressAlign: 16
+  Flags: [SHF_EXECINSTR, SHF_ALLOC]
+
+Symbols:
+  Global:
+    - Name: T1
+      Section: .text
+      Type: STT_FUNC
+      Value: 0x0
+      Size: 8
+
+--- !ELF
+FileHeader: !FileHeader
+  Class: ELFCLASS32
+  Data: ELFDATA2LSB
+  Type: ET_REL
+  Machine: EM_MIPS
+
+Sections:
+- Name: .text
+  Type: SHT_PROGBITS
+  Content:  "00000000"
+  AddressAlign: 16
+  Flags: [SHF_EXECINSTR, SHF_ALLOC]
+
+Symbols:
+  Global:
+    - Name: T2
+      Section: .text
+      Type: STT_FUNC
+      Value: 0x0
+      Size: 4
+...

tools/yaml2obj/yaml2coff.cpp

if (i->CLRToken)
          << zeros(i->CLRToken->unused2);
   }
 
   // Output string table.
   OS.write(&CP.StringTable[0], CP.StringTable.size());
   return true;
 }
 
-int yaml2coff(llvm::raw_ostream &Out, llvm::MemoryBuffer *Buf) {
-  yaml::Input YIn(Buf->getBuffer());
+int yaml2coff(yaml::Input &YIn, raw_ostream &Out) {
   COFFYAML::Object Doc;
   YIn >> Doc;
   if (YIn.error()) {
     errs() << "yaml2obj: Failed to parse YAML file!\n";
     return 1;
   }
 
   COFFParser CP(Doc);

tools/yaml2obj/yaml2elf.cpp

 static bool is64Bit(const ELFYAML::Object &Doc) {
   return Doc.Header.Class == ELFYAML::ELF_ELFCLASS(ELF::ELFCLASS64);
 }
 
 static bool isLittleEndian(const ELFYAML::Object &Doc) {
   return Doc.Header.Data == ELFYAML::ELF_ELFDATA(ELF::ELFDATA2LSB);
 }
 
-int yaml2elf(llvm::raw_ostream &Out, llvm::MemoryBuffer *Buf) {
-  yaml::Input YIn(Buf->getBuffer());
+int yaml2elf(yaml::Input &YIn, raw_ostream &Out) {
   ELFYAML::Object Doc;
   YIn >> Doc;
   if (YIn.error()) {
     errs() << "yaml2obj: Failed to parse YAML file!\n";
     return 1;
   }
   using object::ELFType;
   typedef ELFType<support::little, 8, true> LE64;
   typedef ELFType<support::big, 8, true> BE64;
   typedef ELFType<support::little, 4, false> LE32;
   typedef ELFType<support::big, 4, false> BE32;
   if (is64Bit(Doc)) {
     if (isLittleEndian(Doc))
       return ELFState<LE64>::writeELF(Out, Doc);
     else
       return ELFState<BE64>::writeELF(Out, Doc);
   } else {
     if (isLittleEndian(Doc))
       return ELFState<LE32>::writeELF(Out, Doc);
     else
       return ELFState<BE32>::writeELF(Out, Doc);
   }
 }

tools/yaml2obj/yaml2obj.h

 //===--- yaml2obj.h - -------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 /// \file
 /// \brief Common declarations for yaml2obj
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_TOOLS_YAML2OBJ_H
 #define LLVM_TOOLS_YAML2OBJ_H
 
 namespace llvm {
-  class raw_ostream;
-  class MemoryBuffer;
+namespace yaml {
+class Input;
 }
-int yaml2coff(llvm::raw_ostream &Out, llvm::MemoryBuffer *Buf);
-int yaml2elf(llvm::raw_ostream &Out, llvm::MemoryBuffer *Buf);
+}
+int yaml2coff(llvm::yaml::Input &YIn, llvm::raw_ostream &Out);
+int yaml2elf(llvm::yaml::Input &YIn, llvm::raw_ostream &Out);
 
 #endif

tools/yaml2obj/yaml2obj.cpp

 // This program takes a YAML description of an object file and outputs the
 // binary equivalent.
 //
 // This is used for writing tests that require binary files.
 //
 //===----------------------------------------------------------------------===//
 
 #include "yaml2obj.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/system_error.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/YAMLTraits.h"
 
 using namespace llvm;
 
 static cl::opt<std::string>
   Input(cl::Positional, cl::desc("<input>"), cl::init("-"));
 
 // TODO: The "right" way to tell what kind of object file a given YAML file
 // corresponds to is to look at YAML "tags" (e.g. `!Foo`). Then, different

 cl::opt<YAMLObjectFormat> Format(
   "format",
   cl::desc("Interpret input as this type of object file"),
   cl::values(
     clEnumValN(YOF_COFF, "coff", "COFF object file format"),
     clEnumValN(YOF_ELF, "elf", "ELF object file format"),
   clEnumValEnd));
 
+cl::opt<unsigned>
+DocNum("docnum", cl::init(1),
+       cl::desc("Read specified document from input (default = 1)"));
+
 static cl::opt<std::string> OutputFilename("o", cl::desc("Output filename"),
                                            cl::value_desc("filename"));
 
+typedef int (*ConvertFuncPtr)(yaml::Input & YIn, raw_ostream &Out);
+
+int convertYaml(yaml::Input & YIn, raw_ostream &Out, ConvertFuncPtr Convert) {
+  unsigned CurDocNum = 0;
+  do {
+    if (++CurDocNum == DocNum)
+      return Convert(YIn, Out);
+  } while (YIn.nextDocument());
+
+  errs() << "yaml2obj: Cannot find the " << DocNum
+         << llvm::getOrdinalSuffix(DocNum) << " document\n";
+  return 1;
+}
+
 int main(int argc, char **argv) {
   cl::ParseCommandLineOptions(argc, argv);
   sys::PrintStackTraceOnErrorSignal();
   PrettyStackTraceProgram X(argc, argv);
   llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   if (OutputFilename.empty())
     OutputFilename = "-";
 
   std::string ErrorInfo;
   std::unique_ptr<tool_output_file> Out(
       new tool_output_file(OutputFilename.c_str(), ErrorInfo, sys::fs::F_None));
   if (!ErrorInfo.empty()) {
     errs() << ErrorInfo << '\n';
     return 1;
   }
 
   std::unique_ptr<MemoryBuffer> Buf;
   if (MemoryBuffer::getFileOrSTDIN(Input, Buf))
     return 1;
 
-  int Res = 1;
+  ConvertFuncPtr Convert = nullptr;
   if (Format == YOF_COFF)
-    Res = yaml2coff(Out->os(), Buf.get());
+    Convert = yaml2coff;
   else if (Format == YOF_ELF)
-    Res = yaml2elf(Out->os(), Buf.get());
-  else
+    Convert = yaml2elf;
+  else {
     errs() << "Not yet implemented\n";
+    return 1;
+  }
+
+  yaml::Input YIn(Buf->getBuffer());
 
+  int Res = convertYaml(YIn, Out->os(), Convert);
   if (Res == 0)
     Out->keep();
 
   return Res;
 }