Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[obj2yaml] Add ability to dump GOFF header records #90871

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

redstar
Copy link
Member

@redstar redstar commented May 2, 2024

Add support for the GOFF file format to obj2yaml, beginning with dumping header records.
The GOFF file format does not fit well into the ELF model, so an iterator over the
logical records and a data extractor is used to retrieve the information.

Add support for the GOFF file format to obj2yaml, beginning with dumping header records.
The GOFF file format does not fit well into the ELF model, so an iterator over the
logical records and a data extractor is used to retrieve the information.
@llvmbot
Copy link
Collaborator

llvmbot commented May 2, 2024

@llvm/pr-subscribers-objectyaml

@llvm/pr-subscribers-llvm-binary-utilities

Author: Kai Nacke (redstar)

Changes

Add support for the GOFF file format to obj2yaml, beginning with dumping header records.
The GOFF file format does not fit well into the ELF model, so an iterator over the
logical records and a data extractor is used to retrieve the information.


Patch is 20.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90871.diff

11 Files Affected:

  • (modified) llvm/include/llvm/Object/GOFFObjectFile.h (+71-4)
  • (modified) llvm/include/llvm/ObjectYAML/GOFFYAML.h (+2-2)
  • (modified) llvm/lib/Object/GOFFObjectFile.cpp (+100-48)
  • (modified) llvm/lib/Object/ObjectFile.cpp (+2-1)
  • (modified) llvm/lib/ObjectYAML/GOFFEmitter.cpp (+2-1)
  • (added) llvm/test/ObjectYAML/GOFF/header-default.yaml (+13)
  • (added) llvm/test/ObjectYAML/GOFF/header-filled.yaml (+21)
  • (modified) llvm/tools/obj2yaml/CMakeLists.txt (+1)
  • (added) llvm/tools/obj2yaml/goff2yaml.cpp (+148)
  • (modified) llvm/tools/obj2yaml/obj2yaml.cpp (+3)
  • (modified) llvm/tools/obj2yaml/obj2yaml.h (+3)
diff --git a/llvm/include/llvm/Object/GOFFObjectFile.h b/llvm/include/llvm/Object/GOFFObjectFile.h
index 6871641e97ec8d..1d05bad9e83dde 100644
--- a/llvm/include/llvm/Object/GOFFObjectFile.h
+++ b/llvm/include/llvm/Object/GOFFObjectFile.h
@@ -20,6 +20,7 @@
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/ConvertEBCDIC.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/SubtargetFeature.h"
 #include "llvm/TargetParser/Triple.h"
@@ -28,6 +29,66 @@ namespace llvm {
 
 namespace object {
 
+// The GOFF file format consists of 2 layers. The data is organized in logical
+// records, which are divided into physical records. A RecordRef object
+// represents a logical record, with the data still divided in physical record.
+class RecordRef {
+  const ObjectFile *OwningObject = nullptr;
+  const uint8_t *Data = nullptr;
+  uint64_t Size = 0U;
+  Error *Err = nullptr;
+
+  const uint8_t *base() const {
+    return reinterpret_cast<const uint8_t *>(OwningObject->getData().data());
+  }
+
+  // Create a string error, and clear the position.
+  void createError(std::error_code EC, const Twine &S) {
+    if (Err)
+      *Err = createStringError(EC, S);
+    Data = nullptr;
+    Size = 0;
+  }
+
+  // Determine the size of the logical record. Also makes sure that the physical
+  // records are correctly linked.
+  void determineSize();
+
+public:
+  RecordRef() = default;
+  RecordRef(const ObjectFile *Owner) : OwningObject(Owner) {}
+  RecordRef(const ObjectFile *Owner, Error *Err)
+      : OwningObject(Owner), Data(base()), Err(Err) {
+    determineSize();
+  };
+  RecordRef(const RecordRef &Other) {
+    OwningObject = Other.OwningObject;
+    Data = Other.Data;
+    Size = Other.Size;
+    Err = Other.Err;
+  };
+
+  bool operator==(const RecordRef &Other) const;
+  void moveNext();
+
+  /// Returns the type of the record.
+  llvm::GOFF::RecordType getRecordType() const;
+
+  /// Returns the size of the logical record. This is a multiply of the size of
+  /// a physical record.
+  uint64_t getSize() const;
+
+  /// Data of the logical record, still divided in physical records.
+  const ArrayRef<uint8_t> getContents() const;
+};
+
+inline bool RecordRef::operator==(const RecordRef &Other) const {
+  return OwningObject == Other.OwningObject && Data == Other.Data &&
+         Size == Other.Size;
+}
+
+using record_iterator = content_iterator<RecordRef>;
+
 class GOFFObjectFile : public ObjectFile {
   friend class GOFFSymbolRef;
 
@@ -44,6 +105,12 @@ class GOFFObjectFile : public ObjectFile {
   mutable DenseMap<uint32_t, SmallVector<uint8_t>> SectionDataCache;
 
 public:
+  record_iterator record_begin(Error *Err) const;
+  record_iterator record_end() const;
+  iterator_range<record_iterator> records(Error *Err) const {
+    return make_range(record_begin(Err), record_end());
+  }
+
   Expected<StringRef> getSymbolName(SymbolRef Symbol) const;
 
   GOFFObjectFile(MemoryBufferRef Object, Error &Err);
@@ -57,7 +124,9 @@ class GOFFObjectFile : public ObjectFile {
 
   Triple::ArchType getArch() const override { return Triple::systemz; }
 
-  Expected<SubtargetFeatures> getFeatures() const override { return SubtargetFeatures(); }
+  Expected<SubtargetFeatures> getFeatures() const override {
+    return SubtargetFeatures();
+  }
 
   bool isRelocatableObject() const override { return true; }
 
@@ -65,9 +134,7 @@ class GOFFObjectFile : public ObjectFile {
   basic_symbol_iterator symbol_begin() const override;
   basic_symbol_iterator symbol_end() const override;
 
-  bool is64Bit() const override {
-    return true;
-  }
+  bool is64Bit() const override { return true; }
 
   bool isSectionNoLoad(DataRefImpl Sec) const;
   bool isSectionReadOnlyData(DataRefImpl Sec) const;
diff --git a/llvm/include/llvm/ObjectYAML/GOFFYAML.h b/llvm/include/llvm/ObjectYAML/GOFFYAML.h
index f9bf45e95bd3a4..7715053e19ff9c 100644
--- a/llvm/include/llvm/ObjectYAML/GOFFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/GOFFYAML.h
@@ -29,8 +29,8 @@ struct FileHeader {
   uint32_t TargetEnvironment = 0;
   uint32_t TargetOperatingSystem = 0;
   uint16_t CCSID = 0;
-  StringRef CharacterSetName;
-  StringRef LanguageProductIdentifier;
+  std::string CharacterSetName;
+  std::string LanguageProductIdentifier;
   uint32_t ArchitectureLevel = 0;
   std::optional<uint16_t> InternalCCSID;
   std::optional<uint8_t> TargetSoftwareEnvironment;
diff --git a/llvm/lib/Object/GOFFObjectFile.cpp b/llvm/lib/Object/GOFFObjectFile.cpp
index 3b8704f28fdbb7..20be66c758fb9c 100644
--- a/llvm/lib/Object/GOFFObjectFile.cpp
+++ b/llvm/lib/Object/GOFFObjectFile.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/raw_ostream.h"
+#include <cstddef>
 
 #ifndef DEBUG_TYPE
 #define DEBUG_TYPE "goff"
@@ -24,6 +25,94 @@
 using namespace llvm::object;
 using namespace llvm;
 
+namespace {
+// Return the type of the record.
+GOFF::RecordType getRecordType(const uint8_t *PhysicalRecord) {
+  return GOFF::RecordType((PhysicalRecord[1] & 0xF0) >> 4);
+}
+
+// Return true if the record is a continuation record.
+bool isContinuation(const uint8_t *PhysicalRecord) {
+  return PhysicalRecord[1] & 0x02;
+}
+
+// Return true if the record has a continuation.
+bool isContinued(const uint8_t *PhysicalRecord) {
+  return PhysicalRecord[1] & 0x01;
+}
+} // namespace
+
+void RecordRef::determineSize() {
+  GOFF::RecordType CurrRecordType = ::getRecordType(Data);
+  bool PrevWasContinued = isContinued(Data);
+  const uint8_t *It = Data + GOFF::RecordLength;
+  const uint8_t *End = reinterpret_cast<const uint8_t *>(
+      base() + OwningObject->getData().size());
+  for (; It < End;
+       PrevWasContinued = isContinued(It), It += GOFF::RecordLength) {
+    GOFF::RecordType RecordType = ::getRecordType(It);
+    bool IsContinuation = isContinuation(It);
+    size_t RecordNum = (It - base()) / GOFF::RecordLength;
+    // If the previous record was continued, the current record should be a
+    // continuation.
+    if (PrevWasContinued && !IsContinuation) {
+      createError(object_error::parse_failed,
+                  "record " + std::to_string(RecordNum) +
+                      " is not a continuation record but the "
+                      "preceding record is continued");
+      return;
+    }
+    // If the current record is a continuation, then the previous record should
+    // be continued, and have the same record type.
+    if (IsContinuation) {
+      if (RecordType != CurrRecordType) {
+        createError(object_error::parse_failed,
+                    "record " + std::to_string(RecordNum) +
+                        " is a continuation record that does not "
+                        "match the type of the previous record");
+        return;
+      }
+      if (!PrevWasContinued) {
+        createError(object_error::parse_failed,
+                    "record " + std::to_string(RecordNum) +
+                        " is a continuation record that is not "
+                        "preceded by a continued record");
+        return;
+      }
+    }
+
+    // Break out of loop when we reached a new record.
+    if (!IsContinuation)
+      break;
+  }
+  Size = It - Data;
+}
+
+void RecordRef::moveNext() {
+  if (Data == nullptr)
+    return;
+  const uint8_t *Base = base();
+  std::ptrdiff_t Offset = Data - Base;
+  uint64_t NewOffset = Offset + Size;
+  if (NewOffset > OwningObject->getData().size()) {
+    Data = nullptr;
+    Size = 0;
+  } else {
+    Data = &Base[NewOffset];
+    determineSize();
+  }
+}
+
+GOFF::RecordType RecordRef::getRecordType() const {
+  return ::getRecordType(Data);
+}
+
+uint64_t RecordRef::getSize() const { return Size; }
+
+const ArrayRef<uint8_t> RecordRef::getContents() const {
+  return ArrayRef<uint8_t>(Data, Size);
+}
+
 Expected<std::unique_ptr<ObjectFile>>
 ObjectFile::createGOFFObjectFile(MemoryBufferRef Object) {
   Error Err = Error::success();
@@ -64,52 +153,9 @@ GOFFObjectFile::GOFFObjectFile(MemoryBufferRef Object, Error &Err)
   SectionEntryImpl DummySection;
   SectionList.emplace_back(DummySection); // Dummy entry at index 0.
 
-  uint8_t PrevRecordType = 0;
-  uint8_t PrevContinuationBits = 0;
-  const uint8_t *End = reinterpret_cast<const uint8_t *>(Data.getBufferEnd());
-  for (const uint8_t *I = base(); I < End; I += GOFF::RecordLength) {
-    uint8_t RecordType = (I[1] & 0xF0) >> 4;
-    bool IsContinuation = I[1] & 0x02;
-    bool PrevWasContinued = PrevContinuationBits & 0x01;
-    size_t RecordNum = (I - base()) / GOFF::RecordLength;
-
-    // If the previous record was continued, the current record should be a
-    // continuation.
-    if (PrevWasContinued && !IsContinuation) {
-      if (PrevRecordType == RecordType) {
-        Err = createStringError(object_error::parse_failed,
-                                "record " + std::to_string(RecordNum) +
-                                    " is not a continuation record but the "
-                                    "preceding record is continued");
-        return;
-      }
-    }
-    // Don't parse continuations records, only parse initial record.
-    if (IsContinuation) {
-      if (RecordType != PrevRecordType) {
-        Err = createStringError(object_error::parse_failed,
-                                "record " + std::to_string(RecordNum) +
-                                    " is a continuation record that does not "
-                                    "match the type of the previous record");
-        return;
-      }
-      if (!PrevWasContinued) {
-        Err = createStringError(object_error::parse_failed,
-                                "record " + std::to_string(RecordNum) +
-                                    " is a continuation record that is not "
-                                    "preceded by a continued record");
-        return;
-      }
-      PrevRecordType = RecordType;
-      PrevContinuationBits = I[1] & 0x03;
-      continue;
-    }
-    LLVM_DEBUG(for (size_t J = 0; J < GOFF::RecordLength; ++J) {
-      const uint8_t *P = I + J;
-      if (J % 8 == 0)
-        dbgs() << "  ";
-      dbgs() << format("%02hhX", *P);
-    });
+  for (auto &Rec : records(&Err)) {
+    uint8_t RecordType = Rec.getRecordType();
+    const uint8_t *I = Rec.getContents().data();
 
     switch (RecordType) {
     case GOFF::RT_ESD: {
@@ -179,11 +225,17 @@ GOFFObjectFile::GOFFObjectFile(MemoryBufferRef Object, Error &Err)
     default:
       llvm_unreachable("Unknown record type");
     }
-    PrevRecordType = RecordType;
-    PrevContinuationBits = I[1] & 0x03;
   }
 }
 
+record_iterator GOFFObjectFile::record_begin(Error *Err) const {
+  return record_iterator(RecordRef(this, Err));
+}
+
+record_iterator GOFFObjectFile::record_end() const {
+  return record_iterator(RecordRef(this));
+}
+
 const uint8_t *GOFFObjectFile::getSymbolEsdRecord(DataRefImpl Symb) const {
   const uint8_t *EsdRecord = EsdPtrs[Symb.d.a];
   return EsdRecord;
diff --git a/llvm/lib/Object/ObjectFile.cpp b/llvm/lib/Object/ObjectFile.cpp
index 6a226a3bbdbca3..831d3abed0954e 100644
--- a/llvm/lib/Object/ObjectFile.cpp
+++ b/llvm/lib/Object/ObjectFile.cpp
@@ -162,7 +162,6 @@ ObjectFile::createObjectFile(MemoryBufferRef Object, file_magic Type,
   case file_magic::windows_resource:
   case file_magic::pdb:
   case file_magic::minidump:
-  case file_magic::goff_object:
   case file_magic::cuda_fatbinary:
   case file_magic::offload_binary:
   case file_magic::dxcontainer_object:
@@ -178,6 +177,8 @@ ObjectFile::createObjectFile(MemoryBufferRef Object, file_magic Type,
   case file_magic::elf_shared_object:
   case file_magic::elf_core:
     return createELFObjectFile(Object, InitContent);
+  case file_magic::goff_object:
+    return createGOFFObjectFile(Object);
   case file_magic::macho_object:
   case file_magic::macho_executable:
   case file_magic::macho_fixed_virtual_memory_shared_lib:
diff --git a/llvm/lib/ObjectYAML/GOFFEmitter.cpp b/llvm/lib/ObjectYAML/GOFFEmitter.cpp
index 345904407e1d24..47550ac971980e 100644
--- a/llvm/lib/ObjectYAML/GOFFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/GOFFEmitter.cpp
@@ -219,7 +219,8 @@ void GOFFState::writeHeader(GOFFYAML::FileHeader &FileHdr) {
   }
 
   GW.makeNewRecord(GOFF::RT_HDR, GOFF::PayloadLength);
-  GW << binaryBe(FileHdr.TargetEnvironment)     // TargetEnvironment
+  GW << zeros(1)                                // Reserved
+     << binaryBe(FileHdr.TargetEnvironment)     // TargetEnvironment
      << binaryBe(FileHdr.TargetOperatingSystem) // TargetOperatingSystem
      << zeros(2)                                // Reserved
      << binaryBe(FileHdr.CCSID)                 // CCSID
diff --git a/llvm/test/ObjectYAML/GOFF/header-default.yaml b/llvm/test/ObjectYAML/GOFF/header-default.yaml
new file mode 100644
index 00000000000000..ed85aeb67840bf
--- /dev/null
+++ b/llvm/test/ObjectYAML/GOFF/header-default.yaml
@@ -0,0 +1,13 @@
+# RUN: yaml2obj %s | obj2yaml | FileCheck %s
+
+# CHECK: FileHeader:      {}
+
+--- !GOFF
+FileHeader:
+  TargetEnvironment: 0
+  TargetOperatingSystem: 0
+  CCSID:           0
+  CharacterSetName: "\0"
+  LanguageProductIdentifier: "\0"
+  ArchitectureLevel: 1
+...
diff --git a/llvm/test/ObjectYAML/GOFF/header-filled.yaml b/llvm/test/ObjectYAML/GOFF/header-filled.yaml
new file mode 100644
index 00000000000000..090b8e1c1b5ccc
--- /dev/null
+++ b/llvm/test/ObjectYAML/GOFF/header-filled.yaml
@@ -0,0 +1,21 @@
+# RUN: yaml2obj %s | obj2yaml | FileCheck %s
+
+# CHECK:      --- !GOFF
+# CHECK-NEXT: FileHeader:
+# CHECK-NEXT:   TargetEnvironment: 1
+# CHECK-NEXT:   TargetOperatingSystem: 2
+# CHECK-NEXT:   CCSID:           1047
+# CHECK-NEXT:   CharacterSetName: EBCDIC-1047
+# CHECK-NEXT:   LanguageProductIdentifier: Foo
+# CHECK-NEXT:   ArchitectureLevel: 2
+# CHECK: ...
+
+--- !GOFF
+FileHeader:
+  TargetEnvironment: 1
+  TargetOperatingSystem: 2
+  CCSID:           1047
+  CharacterSetName: EBCDIC-1047
+  LanguageProductIdentifier: Foo
+  ArchitectureLevel: 2
+...
diff --git a/llvm/tools/obj2yaml/CMakeLists.txt b/llvm/tools/obj2yaml/CMakeLists.txt
index ac8ff8c85dd9cc..ab0a04e617ca64 100644
--- a/llvm/tools/obj2yaml/CMakeLists.txt
+++ b/llvm/tools/obj2yaml/CMakeLists.txt
@@ -14,6 +14,7 @@ add_llvm_utility(obj2yaml
   dwarf2yaml.cpp
   dxcontainer2yaml.cpp
   elf2yaml.cpp
+  goff2yaml.cpp
   macho2yaml.cpp
   minidump2yaml.cpp
   offload2yaml.cpp
diff --git a/llvm/tools/obj2yaml/goff2yaml.cpp b/llvm/tools/obj2yaml/goff2yaml.cpp
new file mode 100644
index 00000000000000..35bfe993c1de10
--- /dev/null
+++ b/llvm/tools/obj2yaml/goff2yaml.cpp
@@ -0,0 +1,148 @@
+//===------ goff2yaml.cpp - obj2yaml conversion tool ------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "obj2yaml.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Object/GOFFObjectFile.h"
+#include "llvm/ObjectYAML/ObjectYAML.h"
+#include "llvm/Support/ConvertEBCDIC.h"
+#include "llvm/Support/DataExtractor.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
+
+using namespace llvm;
+
+namespace {
+std::string getFixedLengthEBCDICString(DataExtractor &Data,
+                                       DataExtractor::Cursor &C,
+                                       uint64_t Length,
+                                       StringRef TrimChars = {"\0", 1}) {
+  StringRef FixedLenStr = Data.getBytes(C, Length);
+  SmallString<16> Str;
+  ConverterEBCDIC::convertToUTF8(FixedLenStr, Str);
+  return Str.str().trim(TrimChars).str();
+}
+} // namespace
+
+class GOFFDumper {
+  const object::GOFFObjectFile &Obj;
+  GOFFYAML::Object YAMLObj;
+
+  Error dumpHeader(ArrayRef<uint8_t> Records);
+  Error dumpExternalSymbol(ArrayRef<uint8_t> Records);
+  Error dumpText(ArrayRef<uint8_t> Records);
+  Error dumpRelocationDirectory(ArrayRef<uint8_t> Records);
+  Error dumpDeferredLength(ArrayRef<uint8_t> Records);
+  Error dumpEnd(ArrayRef<uint8_t> Records);
+
+public:
+  GOFFDumper(const object::GOFFObjectFile &Obj);
+  Error dump();
+  GOFFYAML::Object &getYAMLObj();
+};
+
+GOFFDumper::GOFFDumper(const object::GOFFObjectFile &Obj) : Obj(Obj) {}
+
+Error GOFFDumper::dumpHeader(ArrayRef<uint8_t> Records) {
+  DataExtractor Data(Records, false, 0);
+  DataExtractor::Cursor C(4);
+  YAMLObj.Header.TargetEnvironment = Data.getU32(C);
+  YAMLObj.Header.TargetOperatingSystem = Data.getU32(C);
+  Data.skip(C, 2);
+  YAMLObj.Header.CCSID = Data.getU16(C);
+  YAMLObj.Header.CharacterSetName =
+      getFixedLengthEBCDICString(Data, C, 16);
+  YAMLObj.Header.LanguageProductIdentifier =
+      getFixedLengthEBCDICString(Data, C, 16);
+  YAMLObj.Header.ArchitectureLevel = Data.getU32(C);
+  uint16_t PropertiesLength = Data.getU16(C);
+  Data.skip(C, 6);
+  if (PropertiesLength) {
+    YAMLObj.Header.InternalCCSID = Data.getU16(C);
+    PropertiesLength -= 2;
+  }
+  if (PropertiesLength) {
+    YAMLObj.Header.TargetSoftwareEnvironment = Data.getU8(C);
+    PropertiesLength -= 1;
+  }
+  return C.takeError();
+}
+
+Error GOFFDumper::dumpExternalSymbol(ArrayRef<uint8_t> Records) {
+  // TODO Implement.
+  return Error::success();
+}
+
+Error GOFFDumper::dumpText(ArrayRef<uint8_t> Records) {
+  // TODO Implement.
+  return Error::success();
+}
+
+Error GOFFDumper::dumpRelocationDirectory(ArrayRef<uint8_t> Records) {
+  // TODO Implement.
+  return Error::success();
+}
+
+Error GOFFDumper::dumpDeferredLength(ArrayRef<uint8_t> Records) {
+  // TODO Implement.
+  return Error::success();
+}
+
+Error GOFFDumper::dumpEnd(ArrayRef<uint8_t> Records) {
+  // TODO Implement.
+  return Error::success();
+}
+
+Error GOFFDumper::dump() {
+  Error Err = Error::success();
+  for (auto &Rec : Obj.records(&Err)) {
+    if (Err)
+      return Err;
+    switch (Rec.getRecordType()) {
+    case GOFF::RT_ESD:
+      if (auto Err = dumpExternalSymbol(Rec.getContents()))
+        return Err;
+      break;
+    case GOFF::RT_TXT:
+      if (auto Err = dumpText(Rec.getContents()))
+        return Err;
+      break;
+    case GOFF::RT_RLD:
+      if (auto Err = dumpRelocationDirectory(Rec.getContents()))
+        return Err;
+      break;
+    case GOFF::RT_LEN:
+      if (auto Err = dumpDeferredLength(Rec.getContents()))
+        return Err;
+      break;
+    case GOFF::RT_END:
+      if (auto Err = dumpEnd(Rec.getContents()))
+        return Err;
+      break;
+    case GOFF::RT_HDR:
+      if (auto Err = dumpHeader(Rec.getContents()))
+        return Err;
+      break;
+    }
+  }
+  return Err;
+}
+
+GOFFYAML::Object &GOFFDumper::getYAMLObj() { return YAMLObj; }
+
+Error goff2yaml(raw_ostream &Out, const llvm::object::GOFFObjectFile &Obj) {
+  GOFFDumper Dumper(Obj);
+
+  if (auto Err = Dumper.dump())
+    return Err;
+
+  yaml::Output Yout(Out);
+  Yout << Dumper.getYAMLObj();
+
+  return Error::success();
+}
diff --git a/llvm/tools/obj2yaml/obj2yaml.cpp b/llvm/tools/obj2yaml/obj2yaml.cpp
index 63c8cc55ee2d4a..1b6badeb48fafb 100644
--- a/llvm/tools/obj2yaml/obj2yaml.cpp
+++ b/llvm/tools/obj2yaml/obj2yaml.cpp
@@ -45,6 +45,9 @@ static Error dumpObject(const ObjectFile &Obj, raw_ostream &OS) {
   if (Obj.isELF())
     return elf2yaml(OS, Obj);
 
+  if (Obj.isGOFF())
+    return goff2yaml(OS, cast<GOFFObjectFile>(Obj));
+
   if (Obj.isWasm())
     return errorCodeToError(wasm2yaml(OS, cast<WasmObjectFile>(Obj)));
 
diff --git a/llvm/tools/obj2yaml/obj2yaml.h b/llvm/tools/obj2yaml/obj2yaml.h
index 03d7db5317acde..f2247655f60099 100644
--- a/llvm/tools/obj2yaml/obj2yaml.h
+++ b/llvm/tools/obj2yaml/obj2yaml.h
@@ -13,6 +13,7 @@
 #define LLVM_TOOLS_OBJ2YAML_OBJ2YAML_H
 
 #include "llvm/Object/COFF.h"
+#include "llvm/Object/GOFFObjectFile.h"
 #include "llvm/Object/Minidump.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Object/XCOFFObjectFile.h"
@@ -25,6 +26,8 @@ std::error_code coff2yaml(llvm::raw_ostream &Out,
                           const llvm::object::COFFObjectFile &Obj);
 llvm::Error elf2yaml(llvm::raw_ostream &Out,
                      const llvm::object::ObjectFile &Obj);
+llvm::Error goff2yaml(llvm::raw_ostream &Out,
+                      const llvm::object::GOFFObjectFile &Obj);
 llvm::Error macho2yaml(llvm::raw_ostream &Out, const llvm::object::Binary &Obj...
[truncated]

Copy link

github-actions bot commented May 2, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@jh7370
Copy link
Collaborator

jh7370 commented May 3, 2024

Hi @redstar, I'm not going to have much time to look at this PR. Are you aware of #71445? I think we should land the yaml2obj side that @ysyeda has been working on before we really progress on this one.

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@redstar
Copy link
Member Author

redstar commented May 6, 2024

Hi @redstar, I'm not going to have much time to look at this PR. Are you aware of #71445? I think we should land the yaml2obj side that @ysyeda has been working on before we really progress on this one.

Yes, I am of that PR. I'll try to develop both in parallel because that really helps with testing.

- Use static instead of anonymous namespace for functions
- Use Twine instead of string concatenation
- Fix clang-format error
- Move test cases to test/obj2yaml since they are for the tool
if (RecordType != CurrRecordType) {
createError(object_error::parse_failed,
Twine("Record ")
.concat(std::to_string(RecordNum))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::to_string creates a temporary std::string. The overhead can be removed with Twine(RecordNum)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out! I was not aware of that appraoch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants