[SystemZ][z/OS] yaml2obj for header and end records

[ysyeda]

This PR implements part 1 of yaml2obj for the GOFF Object File Format. It adds support for the header and end records.

[llvmbot]

@llvm/pr-subscribers-objectyaml

Author: Yusra Syeda (ysyeda)

Changes

This PR implements part 1 of yaml2obj for the GOFF Object File Format. It adds support for the header and end records.

---

Full diff: https://github.com/llvm/llvm-project/pull/73859.diff

9 Files Affected:

• (added) llvm/include/llvm/ObjectYAML/GOFFYAML.h (+49)
• (modified) llvm/include/llvm/ObjectYAML/ObjectYAML.h (+2)
• (modified) llvm/include/llvm/ObjectYAML/yaml2obj.h (+5)
• (modified) llvm/lib/ObjectYAML/CMakeLists.txt (+2)
• (added) llvm/lib/ObjectYAML/GOFFEmitter.cpp (+292)
• (added) llvm/lib/ObjectYAML/GOFFYAML.cpp (+46)
• (modified) llvm/lib/ObjectYAML/ObjectYAML.cpp (+5)
• (modified) llvm/lib/ObjectYAML/yaml2obj.cpp (+2)
• (added) llvm/test/ObjectYAML/GOFF/GOFF-Header-End.yaml (+18)

diff --git a/llvm/include/llvm/ObjectYAML/GOFFYAML.h b/llvm/include/llvm/ObjectYAML/GOFFYAML.h
new file mode 100644
index 000000000000000..8053dd1e640c289
--- /dev/null
+++ b/llvm/include/llvm/ObjectYAML/GOFFYAML.h
@@ -0,0 +1,49 @@
+//===- GOFFYAML.h - GOFF YAMLIO implementation ------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file declares classes for handling the YAML representation of GOFF.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_OBJECTYAML_GOFFYAML_H
+#define LLVM_OBJECTYAML_GOFFYAML_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/BinaryFormat/GOFF.h"
+#include "llvm/ObjectYAML/YAML.h"
+#include <cstdint>
+#include <vector>
+
+namespace llvm {
+
+// The structure of the yaml files is not an exact 1:1 match to GOFF. In order
+// to use yaml::IO, we use these structures which are closer to the source.
+namespace GOFFYAML {
+
+struct FileHeader {
+  uint32_t TargetEnvironment;
+  uint32_t TargetOperatingSystem;
+  uint16_t CCSID;
+  StringRef CharacterSetName;
+  StringRef LanguageProductIdentifier;
+  uint32_t ArchitectureLevel;
+  std::optional<uint16_t> InternalCCSID;
+  std::optional<uint8_t> TargetSoftwareEnvironment;
+};
+
+struct Object {
+  FileHeader Header;
+  Object();
+};
+} // end namespace GOFFYAML
+} // end namespace llvm
+
+LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::FileHeader)
+LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::Object)
+
+#endif // LLVM_OBJECTYAML_GOFFYAML_H
diff --git a/llvm/include/llvm/ObjectYAML/ObjectYAML.h b/llvm/include/llvm/ObjectYAML/ObjectYAML.h
index b63607e6796b0b7..7fd15cf290f4a4b 100644
--- a/llvm/include/llvm/ObjectYAML/ObjectYAML.h
+++ b/llvm/include/llvm/ObjectYAML/ObjectYAML.h
@@ -13,6 +13,7 @@
 #include "llvm/ObjectYAML/COFFYAML.h"
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ObjectYAML/ELFYAML.h"
+#include "llvm/ObjectYAML/GOFFYAML.h"
 #include "llvm/ObjectYAML/MachOYAML.h"
 #include "llvm/ObjectYAML/MinidumpYAML.h"
 #include "llvm/ObjectYAML/OffloadYAML.h"
@@ -30,6 +31,7 @@ struct YamlObjectFile {
   std::unique_ptr<ArchYAML::Archive> Arch;
   std::unique_ptr<ELFYAML::Object> Elf;
   std::unique_ptr<COFFYAML::Object> Coff;
+  std::unique_ptr<GOFFYAML::Object> Goff;
   std::unique_ptr<MachOYAML::Object> MachO;
   std::unique_ptr<MachOYAML::UniversalBinary> FatMachO;
   std::unique_ptr<MinidumpYAML::Object> Minidump;
diff --git a/llvm/include/llvm/ObjectYAML/yaml2obj.h b/llvm/include/llvm/ObjectYAML/yaml2obj.h
index 000da077bb18c6a..3b458c3cd890b12 100644
--- a/llvm/include/llvm/ObjectYAML/yaml2obj.h
+++ b/llvm/include/llvm/ObjectYAML/yaml2obj.h
@@ -32,6 +32,10 @@ namespace ELFYAML {
 struct Object;
 }
 
+namespace GOFFYAML {
+struct Object;
+}
+
 namespace MinidumpYAML {
 struct Object;
 }
@@ -64,6 +68,7 @@ using ErrorHandler = llvm::function_ref<void(const Twine &Msg)>;
 
 bool yaml2archive(ArchYAML::Archive &Doc, raw_ostream &Out, ErrorHandler EH);
 bool yaml2coff(COFFYAML::Object &Doc, raw_ostream &Out, ErrorHandler EH);
+bool yaml2goff(GOFFYAML::Object &Doc, raw_ostream &Out, ErrorHandler EH);
 bool yaml2elf(ELFYAML::Object &Doc, raw_ostream &Out, ErrorHandler EH,
               uint64_t MaxSize);
 bool yaml2macho(YamlObjectFile &Doc, raw_ostream &Out, ErrorHandler EH);
diff --git a/llvm/lib/ObjectYAML/CMakeLists.txt b/llvm/lib/ObjectYAML/CMakeLists.txt
index c081009653d4f8c..b36974d47d9f897 100644
--- a/llvm/lib/ObjectYAML/CMakeLists.txt
+++ b/llvm/lib/ObjectYAML/CMakeLists.txt
@@ -13,6 +13,8 @@ add_llvm_component_library(LLVMObjectYAML
   DXContainerYAML.cpp
   ELFEmitter.cpp
   ELFYAML.cpp
+  GOFFEmitter.cpp
+  GOFFYAML.cpp
   MachOEmitter.cpp
   MachOYAML.cpp
   ObjectYAML.cpp
diff --git a/llvm/lib/ObjectYAML/GOFFEmitter.cpp b/llvm/lib/ObjectYAML/GOFFEmitter.cpp
new file mode 100644
index 000000000000000..bfa44304345e6a3
--- /dev/null
+++ b/llvm/lib/ObjectYAML/GOFFEmitter.cpp
@@ -0,0 +1,292 @@
+//===- yaml2goff - Convert YAML to a GOFF object file ---------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// The GOFF component of yaml2obj.
+///
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/IndexedMap.h"
+#include "llvm/ObjectYAML/ObjectYAML.h"
+#include "llvm/ObjectYAML/yaml2obj.h"
+#include "llvm/Support/ConvertEBCDIC.h"
+#include "llvm/Support/Endian.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm;
+
+namespace {
+
+// Common flag values on records.
+enum {
+  // Flag: This record is continued.
+  Rec_Continued = 1,
+
+  // Flag: This record is a continuation.
+  Rec_Continuation = 1 << (8 - 6 - 1),
+};
+
+template <typename ValueType> struct BinaryBeImpl {
+  ValueType Value;
+  BinaryBeImpl(ValueType V) : Value(V) {}
+};
+
+template <typename ValueType>
+raw_ostream &operator<<(raw_ostream &OS, const BinaryBeImpl<ValueType> &BBE) {
+  char Buffer[sizeof(BBE.Value)];
+  support::endian::write<ValueType, llvm::endianness::big, support::unaligned>(
+      Buffer, BBE.Value);
+  OS.write(Buffer, sizeof(BBE.Value));
+  return OS;
+}
+
+template <typename ValueType> BinaryBeImpl<ValueType> binaryBe(ValueType V) {
+  return BinaryBeImpl<ValueType>(V);
+}
+
+struct ZerosImpl {
+  size_t NumBytes;
+};
+
+raw_ostream &operator<<(raw_ostream &OS, const ZerosImpl &Z) {
+  OS.write_zeros(Z.NumBytes);
+  return OS;
+}
+
+ZerosImpl zeros(const size_t NumBytes) { return ZerosImpl{NumBytes}; }
+
+// The GOFFOstream is responsible to write the data into the fixed physical
+// records of the format. A user of this class announces the start of a new
+// logical record and the size of its payload. While writing the payload, the
+// physical records are created for the data. Possible fill bytes at the end of
+// a physical record are written automatically.
+class GOFFOstream : public raw_ostream {
+public:
+  explicit GOFFOstream(raw_ostream &OS)
+      : OS(OS), LogicalRecords(0), RemainingSize(0), NewLogicalRecord(false) {
+    SetBufferSize(GOFF::PayloadLength);
+  }
+
+  ~GOFFOstream() { finalize(); }
+
+  void makeNewRecord(GOFF::RecordType Type, size_t Size);
+
+  void finalize() { fillRecord(); }
+
+  uint32_t logicalRecords() { return LogicalRecords; }
+
+private:
+  /// The underlying raw_ostream.
+  raw_ostream &OS;
+
+  /// The number of logical records emitted so far.
+  uint32_t LogicalRecords;
+
+  /// The remaining size of this logical record, including fill
+  /// bytes.
+  size_t RemainingSize;
+
+  /// The type of the current (logical) record.
+  GOFF::RecordType CurrentType;
+
+  /// Signals start of new record.
+  bool NewLogicalRecord;
+
+  // Return the number of bytes left to write until next physical record.
+  // Please note that we maintain the total number of bytes left, not the
+  // written size.
+  size_t bytesToNextPhysicalRecord() {
+    size_t Bytes = RemainingSize % GOFF::PayloadLength;
+    return Bytes ? Bytes : GOFF::PayloadLength;
+  }
+
+  /// Write the record prefix of a physical record, using the current record
+  /// type.
+  static void writeRecordPrefix(raw_ostream &OS, GOFF::RecordType Type,
+                                size_t RemainingSize,
+                                uint8_t Flags = Rec_Continuation);
+
+  /// Fill the last physical record of a logical record with zero bytes.
+  void fillRecord();
+
+  /// See raw_ostream::write_impl.
+  void write_impl(const char *Ptr, size_t Size) override;
+
+  /// Return the current position within the stream, not counting the bytes
+  /// currently in the buffer.
+  uint64_t current_pos() const override { return OS.tell(); }
+};
+
+void GOFFOstream::writeRecordPrefix(raw_ostream &OS, GOFF::RecordType Type,
+                                    size_t RemainingSize, uint8_t Flags) {
+  uint8_t TypeAndFlags = Flags | (Type << 4);
+  if (RemainingSize > GOFF::RecordLength)
+    TypeAndFlags |= Rec_Continued;
+  OS << binaryBe(static_cast<unsigned char>(GOFF::PTVPrefix))
+     << binaryBe(static_cast<unsigned char>(TypeAndFlags))
+     << binaryBe(static_cast<unsigned char>(0));
+}
+
+void GOFFOstream::makeNewRecord(GOFF::RecordType Type, size_t Size) {
+  fillRecord();
+  CurrentType = Type;
+  RemainingSize = Size;
+  if (size_t Gap = (RemainingSize % GOFF::PayloadLength))
+    RemainingSize += GOFF::PayloadLength - Gap;
+  NewLogicalRecord = true;
+  ++LogicalRecords;
+}
+
+void GOFFOstream::fillRecord() {
+  assert((GetNumBytesInBuffer() <= RemainingSize) &&
+         "More bytes in buffer than expected");
+  size_t Remains = RemainingSize - GetNumBytesInBuffer();
+  if (Remains) {
+    assert((Remains < GOFF::RecordLength) &&
+           "Attempting to fill more than one physical record");
+    raw_ostream::write_zeros(Remains);
+  }
+  flush();
+  assert(RemainingSize == 0 && "Not fully flushed");
+  assert(GetNumBytesInBuffer() == 0 && "Buffer not fully empty");
+}
+
+void GOFFOstream::write_impl(const char *Ptr, size_t Size) {
+  assert((RemainingSize >= Size) && "Attempt to write too much data");
+  assert(RemainingSize && "Logical record overflow");
+  if (!(RemainingSize % GOFF::PayloadLength)) {
+    writeRecordPrefix(OS, CurrentType, RemainingSize,
+                      NewLogicalRecord ? 0 : Rec_Continuation);
+    NewLogicalRecord = false;
+  }
+  assert(!NewLogicalRecord &&
+         "New logical record not on physical record boundary");
+
+  size_t Idx = 0;
+  while (Size > 0) {
+    size_t BytesToWrite = bytesToNextPhysicalRecord();
+    if (BytesToWrite > Size)
+      BytesToWrite = Size;
+    OS.write(Ptr + Idx, BytesToWrite);
+    Idx += BytesToWrite;
+    Size -= BytesToWrite;
+    RemainingSize -= BytesToWrite;
+    if (Size) {
+      writeRecordPrefix(OS, CurrentType, RemainingSize);
+    }
+  }
+}
+
+class GOFFState {
+  void writeHeader(GOFFYAML::FileHeader &FileHdr);
+  void writeEnd();
+
+  void reportError(const Twine &Msg) {
+    ErrHandler(Msg);
+    HasError = true;
+  }
+
+  GOFFState(raw_ostream &OS, GOFFYAML::Object &Doc,
+            yaml::ErrorHandler ErrHandler)
+      : GW(OS), Doc(Doc), ErrHandler(ErrHandler), HasError(false) {}
+
+  ~GOFFState() { GW.finalize(); }
+
+  bool writeObject();
+
+public:
+  static bool writeGOFF(raw_ostream &OS, GOFFYAML::Object &Doc,
+                        yaml::ErrorHandler ErrHandler);
+
+private:
+  GOFFOstream GW;
+  GOFFYAML::Object &Doc;
+  yaml::ErrorHandler ErrHandler;
+  bool HasError;
+};
+
+void GOFFState::writeHeader(GOFFYAML::FileHeader &FileHdr) {
+  SmallString<16> CCSIDName;
+  if (std::error_code EC =
+          ConverterEBCDIC::convertToEBCDIC(FileHdr.CharacterSetName, CCSIDName))
+    reportError("Conversion error on " + FileHdr.CharacterSetName);
+  if (CCSIDName.size() > 16) {
+    reportError("CharacterSetName too long");
+    CCSIDName.resize(16);
+  }
+  SmallString<16> LangProd;
+  if (std::error_code EC = ConverterEBCDIC::convertToEBCDIC(
+          FileHdr.LanguageProductIdentifier, LangProd))
+    reportError("Conversion error on " + FileHdr.LanguageProductIdentifier);
+  if (LangProd.size() > 16) {
+    reportError("LanguageProductIdentifier too long");
+    LangProd.resize(16);
+  }
+
+  GW.makeNewRecord(GOFF::RT_HDR, GOFF::PayloadLength);
+  GW << binaryBe(FileHdr.TargetEnvironment)     // TargetEnvironment
+     << binaryBe(FileHdr.TargetOperatingSystem) // TargetOperatingSystem
+     << zeros(2)                                // Reserved
+     << binaryBe(FileHdr.CCSID)                 // CCSID
+     << CCSIDName                               // CharacterSetName
+     << zeros(16 - CCSIDName.size())            // Fill bytes
+     << LangProd                                // LanguageProductIdentifier
+     << zeros(16 - LangProd.size())             // Fill bytes
+     << binaryBe(FileHdr.ArchitectureLevel);    // ArchitectureLevel
+  // The module propties are optional. Figure out if we need to write them.
+  uint16_t ModPropLen = 0;
+  if (FileHdr.TargetSoftwareEnvironment)
+    ModPropLen = 3;
+  else if (FileHdr.InternalCCSID)
+    ModPropLen = 2;
+  if (ModPropLen) {
+    GW << binaryBe(ModPropLen) << zeros(6);
+    if (ModPropLen >= 2)
+      GW << binaryBe(FileHdr.InternalCCSID ? *FileHdr.InternalCCSID : 0);
+    if (ModPropLen >= 3)
+      GW << binaryBe(FileHdr.TargetSoftwareEnvironment
+                         ? *FileHdr.TargetSoftwareEnvironment
+                         : 0);
+  }
+}
+
+void GOFFState::writeEnd() {
+  GW.makeNewRecord(GOFF::RT_END, GOFF::PayloadLength);
+  GW << binaryBe(uint8_t(0)) // No entry point
+     << binaryBe(uint8_t(0)) // No AMODE
+     << zeros(3)             // Reserved
+     << binaryBe(GW.logicalRecords());
+  // No entry point yet. Automatically fill remaining space with zero bytes.
+  GW.finalize();
+}
+
+bool GOFFState::writeObject() {
+  writeHeader(Doc.Header);
+  if (HasError)
+    return false;
+  writeEnd();
+  return true;
+}
+
+bool GOFFState::writeGOFF(raw_ostream &OS, GOFFYAML::Object &Doc,
+                          yaml::ErrorHandler ErrHandler) {
+  GOFFState State(OS, Doc, ErrHandler);
+  return State.writeObject();
+}
+} // namespace
+
+namespace llvm {
+namespace yaml {
+
+bool yaml2goff(llvm::GOFFYAML::Object &Doc, raw_ostream &Out,
+               ErrorHandler ErrHandler) {
+  return GOFFState::writeGOFF(Out, Doc, ErrHandler);
+}
+
+} // namespace yaml
+} // namespace llvm
diff --git a/llvm/lib/ObjectYAML/GOFFYAML.cpp b/llvm/lib/ObjectYAML/GOFFYAML.cpp
new file mode 100644
index 000000000000000..a7d0021e9fd21e7
--- /dev/null
+++ b/llvm/lib/ObjectYAML/GOFFYAML.cpp
@@ -0,0 +1,46 @@
+//===-- GOFFYAML.cpp - GOFF YAMLIO implementation ---------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines classes for handling the YAML representation of GOFF.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ObjectYAML/GOFFYAML.h"
+#include "llvm/BinaryFormat/GOFF.h"
+#include <string.h>
+
+namespace llvm {
+namespace GOFFYAML {
+
+Object::Object() { memset(&Header, 0, sizeof(Header)); }
+
+} // namespace GOFFYAML
+
+namespace yaml {
+
+void MappingTraits<GOFFYAML::FileHeader>::mapping(
+    IO &IO, GOFFYAML::FileHeader &FileHdr) {
+  IO.mapOptional("TargetEnvironment", FileHdr.TargetEnvironment, 0);
+  IO.mapOptional("TargetOperatingSystem", FileHdr.TargetOperatingSystem, 0);
+  IO.mapOptional("CCSID", FileHdr.CCSID, 0);
+  IO.mapOptional("CharacterSetName", FileHdr.CharacterSetName, "");
+  IO.mapOptional("LanguageProductIdentifier", FileHdr.LanguageProductIdentifier,
+                 "");
+  IO.mapOptional("ArchitectureLevel", FileHdr.ArchitectureLevel, 1);
+  IO.mapOptional("InternalCCSID", FileHdr.InternalCCSID);
+  IO.mapOptional("TargetSoftwareEnvironment",
+                 FileHdr.TargetSoftwareEnvironment);
+}
+
+void MappingTraits<GOFFYAML::Object>::mapping(IO &IO, GOFFYAML::Object &Obj) {
+  IO.mapTag("!GOFF", true);
+  IO.mapRequired("FileHeader", Obj.Header);
+}
+
+} // namespace yaml
+} // namespace llvm
diff --git a/llvm/lib/ObjectYAML/ObjectYAML.cpp b/llvm/lib/ObjectYAML/ObjectYAML.cpp
index d57e5583016b507..1815eaff8e36dde 100644
--- a/llvm/lib/ObjectYAML/ObjectYAML.cpp
+++ b/llvm/lib/ObjectYAML/ObjectYAML.cpp
@@ -26,6 +26,8 @@ void MappingTraits<YamlObjectFile>::mapping(IO &IO,
       MappingTraits<ELFYAML::Object>::mapping(IO, *ObjectFile.Elf);
     if (ObjectFile.Coff)
       MappingTraits<COFFYAML::Object>::mapping(IO, *ObjectFile.Coff);
+    if (ObjectFile.Goff)
+      MappingTraits<GOFFYAML::Object>::mapping(IO, *ObjectFile.Goff);
     if (ObjectFile.MachO)
       MappingTraits<MachOYAML::Object>::mapping(IO, *ObjectFile.MachO);
     if (ObjectFile.FatMachO)
@@ -46,6 +48,9 @@ void MappingTraits<YamlObjectFile>::mapping(IO &IO,
     } else if (IO.mapTag("!COFF")) {
       ObjectFile.Coff.reset(new COFFYAML::Object());
       MappingTraits<COFFYAML::Object>::mapping(IO, *ObjectFile.Coff);
+    } else if (IO.mapTag("!GOFF")) {
+      ObjectFile.Goff.reset(new GOFFYAML::Object());
+      MappingTraits<GOFFYAML::Object>::mapping(IO, *ObjectFile.Goff);
     } else if (IO.mapTag("!mach-o")) {
       ObjectFile.MachO.reset(new MachOYAML::Object());
       MappingTraits<MachOYAML::Object>::mapping(IO, *ObjectFile.MachO);
diff --git a/llvm/lib/ObjectYAML/yaml2obj.cpp b/llvm/lib/ObjectYAML/yaml2obj.cpp
index 06050e246fbff32..b9a9ad639709956 100644
--- a/llvm/lib/ObjectYAML/yaml2obj.cpp
+++ b/llvm/lib/ObjectYAML/yaml2obj.cpp
@@ -38,6 +38,8 @@ bool convertYAML(yaml::Input &YIn, raw_ostream &Out, ErrorHandler ErrHandler,
       return yaml2elf(*Doc.Elf, Out, ErrHandler, MaxSize);
     if (Doc.Coff)
       return yaml2coff(*Doc.Coff, Out, ErrHandler);
+    if (Doc.Goff)
+      return yaml2goff(*Doc.Goff, Out, ErrHandler);
     if (Doc.MachO || Doc.FatMachO)
       return yaml2macho(Doc, Out, ErrHandler);
     if (Doc.Minidump)
diff --git a/llvm/test/ObjectYAML/GOFF/GOFF-Header-End.yaml b/llvm/test/ObjectYAML/GOFF/GOFF-Header-End.yaml
new file mode 100644
index 000000000000000..7f7cf21390c0ec4
--- /dev/null
+++ b/llvm/test/ObjectYAML/GOFF/GOFF-Header-End.yaml
@@ -0,0 +1,18 @@
+# RUN: yaml2obj %s | od -v -An -tx1 | FileCheck --ignore-case %s
+
+# Verify that GOFF Header is correct.
+# CHECK: 03 f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK: 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+
+# Verify GOFF Module end.
+# CHECK: 03 40 00 00 00 00 00 00 00 00 00 02 00 00 00 00
+# CHECK: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+--- !GOFF
+FileHeader:
+  ArchitectureLevel: 1

[jh7370]

As before, I've avoided looking at the GOFF details and just tried to focus on style and test comments, aside from some more (small) general issues too.

[jh7370]

There's nothing in this test that shows you've checked the whole file. I assume that is desirable. One way of doing this is adding --implicit-check-not={{.}} to the FileCheck command.

[jh7370]

This conversation was marked as resolved, but I don't see anything that's changed that would achieve this?

[ysyeda]

I resolved this accidentally.
When I try to add --implicit-check-not={{.}}, my test fails at the first line. The output of od shows exactly the same GOFF output. Not sure if I'm missing something obvious here.

[jh7370]

Hmm, that implies there is some text still printed to the output, but it could be as simple as whitespace, e.g. a blank line at the end or start of the output. Could you pipe the od to a file and attach it here, please, and I can then take a quick look?

[ysyeda]

Sure, I've attached it here.
output.txt

[jh7370]

Could you try adding a # CHECK-EMPTY: to the end of the output and see if that fixes it? That's about the only idea I've got, looking at that text file.

[ysyeda]

That seems to work fine.

[jh7370]

This conversation was marked as resolved, but I don't see anything that's changed that would achieve this?

[jh7370]

@ysyeda, please don't resolve conversations that I've initiated, as I use the "resolved" state to determine whether or not I have reviewed the change and am happy with it. For more context, see https://discourse.llvm.org/t/rfc-github-pr-resolve-conversation-button/73178.

For ELF and COFF at least, most of the tests are contained in test/tools/yaml2obj. It might make sense to move the tests there.

[jh7370]

Hmm, that implies there is some text still printed to the output, but it could be as simple as whitespace, e.g. a blank line at the end or start of the output. Could you pipe the od to a file and attach it here, please, and I can then take a quick look?

[jh7370]

Nit: test file names are usually all lower-case.

[jh7370]

LGTM, aside from a couple of remaining nits, but it would be worth getting a pair of eyes with real GOFF experience to sign off on it.

[jh7370]

There's no need to use --check-prefix as there is only one prefix in this file. Just use CHECK.

Also, I'd reorder the test a little as follows, as I think this aids readability:

# RUN: ...
# CHECK: ...

--- !GOFF
## X: [] is an extra field ...
X: []

[ysyeda]

Done

[redstar]

LGTM.
Only a default version of the END record is implemented. Currently, AMODE, record count, etc. cannot be specified; see End of module record for the possible fields of the END record. That can be easily added on top of this implementation.