[SystemZ][z/OS] Add support for TXT records in the GOFF reader

[yusra.syeda]

ping :)

[jhenderson]

In D103490#2806504, @yusra.syeda wrote:

ping :)

Sorry, I've been away, and this is pretty low on my review priority list, so I haven't had a chance to look at this until now.

llvm/include/llvm/Object/GOFF.h

95

103

What's the motivation for returning by updating an input parameter, rather than returning via the return type?

llvm/include/llvm/Object/GOFFObjectFile.h

33

There may be a perfectly valid reason for this, but why make this a friend class rather than just making the relevant functions public?

36

Any particular reason you've chosen the size 256 here and in various other places? It may deserve a comment or some sort of named constant.

70–71

What's the motivation for adding these two functions at this point? It seems like a bad idea to expand the interface without a use-case (and if there is a use-case, what is it)?

72

By name, this function sounds awfully like isSectionBSS (which for ELF at least means a section that is allocated space and zero-initialised at load time).

126

*Ref types are designed to be copied. Don't pass them as const &.

llvm/lib/Object/GOFFObjectFile.cpp

432

This TODO sounds a bit odd. What does the clang compiler have to do with this code?

478

Any particular reason you've used '\0' rather than simply 0? After all, you're storing it in a uint8_t (a number) rather than a char/unsigned char.

519–520

Why not simply return arrayRefFromStringRef(StringRef(Data));? In fact, could you get away with returning simply Data.data() or similar?

527

I'm pretty sure the right hand side of this function will return an int because that's the type of the literal 1 in this context. I think you want to do UINT64_C(1) to force the type of the literal (and therefore the result of the left-shift operation) to 64-bit.

539–540

Don't add unnecessary local variable. Same below for the other functions.

564–573

576–580

Why are you adding this function?

llvm/unittests/Object/GOFFObjectFileTest.cpp

507

You can omit the explicit 0 from here. This will zero-initialise the whole array.

587

Get rid of double blank line.

595

600–603

You should be able to just do EXPECT_EQ(Contents, "\x12\x34\x56\x78\x9a\xbc\xde\xf0");

Use EXPECT_EQ because one section's content not being right shouldn't impact other sections, same as with the SymbolName check above.

Additionally, you need checks that symbols() and sections() return the right number of symbols and sections somewhere. Your test would pass as things stand, if symbols() or sections() returned an empty list (and therefore no looping occurred).

[yusra.syeda]

Address comments in the unit test

[yusra.syeda]

Removed unused functions, and addressed some more review comments

[yusra.syeda]

Update the getElementEsdId function

[yusra.syeda]

llvm/include/llvm/Object/GOFF.h

103

There is no specific reason, I have changed it to return the type.

llvm/include/llvm/Object/GOFFObjectFile.h

33

It's also done this way in ELFObjectFile.h, so this follows

[yusra.syeda]

llvm/include/llvm/Object/GOFFObjectFile.h

36

An initial vector size of 256 guarantees that in most cases the vector does not need to be extended, and avoids a memory allocation in SmallVector.

[jhenderson]

llvm/include/llvm/Object/GOFFObjectFile.h

36

The trade-off is that this requires a significantly bigger stack allocation than might be necessary. How many TextPtrs might a typical file have?

llvm/lib/Object/GOFFObjectFile.cpp

519–520

In fact, could you get away with returning simply Data.data() or similar?

Any response to this part of the comment? I'm pretty sure you could just do return {Err.data(), Err.size()} without needing to jump through the StringRef constructor.

543–544

As previously requested in my above comment ("same below...") don't add unnecessary local variables like this.

Really, I'd expect this return line to just be return ESDREcord::getExecutable(EsdRecord) == GOFF::ESD_EXE_DATA;. I don't know why you have several functions that return via input parameter rather than normal C++ way of using the return type...

llvm/unittests/Object/GOFFObjectFileTest.cpp

587

size_t seems more appropriate for counting the number of something.

593

Prefer pre to postincrement. However, I think I have a beter suggestion, if you only expect one symbol:

auto Symbols  = GOFFObj->symbols();
ASSERT_EQ(std::distance(Symbols.begin(), Symbols.end(), 1);
SymbolRef Symbol = *Symbols.begin();
Expected<StringRef> SymbolNameOrErr = GOFFObj->getSymbolName(Symbol);
ASSERT_THAT_EXPECTED(SymbolNameOrErr, Succeeded());
StringRef SymbolName = SymbolNameOrErr.get();
EXPECT_EQ(SymbolName, "var#c");

A similar change would work for the section check below.

[yusra.syeda]

Updated the unit test and cleaned up return statements

[yusra.syeda]

llvm/lib/Object/GOFFObjectFile.cpp

519–520

Seems I can do something like this to avoid the StringRef constructor:

ArrayRef<uint8_t> temp(reinterpret_cast<const unsigned char *>(Data.data()), Data.size());
return temp;

If you prefer this, I can update it.

[jhenderson]

llvm/lib/Object/GOFFObjectFile.cpp

519–520

Ah, I see what some of the problem is now - didn't realise this returned an ArrayRef<uint8_t> rather than an ArrayRef<char>. Seems to me like this might be because your interfaces are inconsistent - some are using strings (effectively arrays of char), and others are using arrays of uint8_t). Is it possible to change your methods to canonicalise on one? To me it would be better to be the uint8_t option, since that is less likely to fall foul of signed conversion issues, and it's the closest thing to a true byte type that we have.

I think leaving as-is is fine, if it's not practical to canonicalise. For reference, you shouldn't need the temporary in your example, and should be able to just return it directly.

[yusra.syeda]

llvm/include/llvm/Object/GOFFObjectFile.h

36

The amount of TXT records can vary quite a bit. In an analysis of about 3390 object files, the average was about 1200 with the minimum being 3 and maximum being 376780. So 256 is a reasonable value.

llvm/lib/Object/GOFFObjectFile.cpp

519–520

Thanks, I will leave this as is for this patch.

[jhenderson]

Okay, no more comments from me at this time. Someone with GOFF knowledge needs to review the functionality for correctness.

[kpn]

Say, I can't find any file in the llvm repository named "GOFF*". They don't show up on Github, either, so I'm not sure what's going on. Where is the GOFF support in the LLVM tree currently? There's no "https://github.com/llvm/llvm-project/tree/main/llvm/lib/Object/GOFFObjectFile.cpp", for example.

llvm/include/llvm/BinaryFormat/GOFF.h

152 ↗

(On Diff #352251)

This appears unused currently. Don't include it until the patch where you use it.

llvm/lib/Object/GOFFObjectFile.cpp

479

Is this check really needed? If a zero fill byte is wanted then the GOFF fill byte field will be zero.

[yusra.syeda]

Removed unused enum and a redundant if statement

[yusra.syeda]

In D103490#2825462, @kpn wrote:

Say, I can't find any file in the llvm repository named "GOFF*". They don't show up on Github, either, so I'm not sure what's going on. Where is the GOFF support in the LLVM tree currently? There's no "https://github.com/llvm/llvm-project/tree/main/llvm/lib/Object/GOFFObjectFile.cpp", for example.

This patch is based off of https://reviews.llvm.org/D98437 which is where files you mentioned are added. https://reviews.llvm.org/D98437 cannot be committed until https://reviews.llvm.org/D88741 is committed. So these files are not in the llvm repository yet.

[yusra.syeda]

llvm/lib/Object/GOFFObjectFile.cpp

479

No, it's not needed. I've updated this.

[kpn]

In D103490#2845377, @yusra.syeda wrote:

In D103490#2825462, @kpn wrote:

Say, I can't find any file in the llvm repository named "GOFF*". They don't show up on Github, either, so I'm not sure what's going on. Where is the GOFF support in the LLVM tree currently? There's no "https://github.com/llvm/llvm-project/tree/main/llvm/lib/Object/GOFFObjectFile.cpp", for example.

This patch is based off of https://reviews.llvm.org/D98437 which is where files you mentioned are added. https://reviews.llvm.org/D98437 cannot be committed until https://reviews.llvm.org/D88741 is committed. So these files are not in the llvm repository yet.

I don't think we should be getting so far ahead of ourselves. Get your prerequisites in tree first and then we can proceed with this patch. That's my take, anyway.

llvm/include/llvm/Object/GOFF.h

 //===- GOFF.h - GOFF object file 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
 //
 //===----------------------------------------------------------------------===//
 //
public:
 
   static uint16_t getPropertyModuleLength(const uint8_t *Record) {
     uint16_t Length;
     get<uint16_t>(Record, 52, Length);
     return Length;
   }
 };
 
+class TXTRecord : public Record {
+public:
+  /// \brief Maximum length of data. Any more must go in a continuation.
+  static const uint8_t TXTMaxDataLength = 56;
+
+public:
+  TXTRecord() {}
+
+  static Error getData(const uint8_t *Record, SmallString<256> &CompleteData);
+
+  static uint32_t getElementEsdId(const uint8_t *Record) {
+    uint32_t EsdId;
+    get<uint32_t>(Record, 4, EsdId);
+    return EsdId;
+  }
+
+  static void getOffset(const uint8_t *Record, uint32_t &Offset) {
+    get<uint32_t>(Record, 12, Offset);
+  }
+
+  static void getDataLength(const uint8_t *Record, uint16_t &Length) {
+    get<uint16_t>(Record, 22, Length);
+  }
+};
+
 class ESDRecord : public Record {
 public:
   /// \brief Number of bytes for name; any more must go in continuation.
   /// This is the number of bytes that can fit into the data field of an ESD
   /// record.
   static const uint8_t ESDMaxUncontinuedNameLength = 8;
 
   /// \brief Maximum name length for ESD records and continuations.

llvm/include/llvm/Object/GOFFObjectFile.h

 //===- GOFF.h - GOFF object file 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
 //
 //===----------------------------------------------------------------------===//
 //

 #include "llvm/Support/Endian.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 
 namespace object {
 
 class GOFFObjectFile : public ObjectFile {
+  friend class GOFFSymbolRef;
+
   IndexedMap<const uint8_t *> EsdPtrs; // Indexed by EsdId.
+  SmallVector<const uint8_t *, 256> TextPtrs;
 
   mutable DenseMap<uint32_t, std::string> EsdNamesCache;
 
   typedef DataRefImpl SectionEntryImpl;
   // (EDID, 0)               code, r/o data section
   // (EDID,PRID)             r/w data section
   SmallVector<SectionEntryImpl, 256> SectionList;
   mutable DenseMap<uint32_t, std::string> SectionDataCache;
public:
   SubtargetFeatures getFeatures() const override { return SubtargetFeatures(); }
 
   bool isRelocatableObject() const override { return true; }
 
   void moveSymbolNext(DataRefImpl &Symb) const override;
   basic_symbol_iterator symbol_begin() const override;
   basic_symbol_iterator symbol_end() const override;
 
 private:
   // SymbolRef.
   Expected<StringRef> getSymbolName(DataRefImpl Symb) const override;
   Expected<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
   uint64_t getSymbolValueImpl(DataRefImpl Symb) const override;
   uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
   Expected<uint32_t> getSymbolFlags(DataRefImpl Symb) const override;
   Expected<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
+  uint64_t getSymbolSize(DataRefImpl Symb) const;
 
   const uint8_t *getSymbolEsdRecord(DataRefImpl Symb) const;
   bool isSymbolUnresolved(DataRefImpl Symb) const;
   bool isSymbolIndirect(DataRefImpl Symb) const;
 
   // SectionRef.
-  void moveSectionNext(DataRefImpl &Sec) const override{};
-  virtual Expected<StringRef> getSectionName(DataRefImpl Sec) const override {
-    return StringRef();
-  }
-  uint64_t getSectionAddress(DataRefImpl Sec) const override { return 0; }
-  uint64_t getSectionSize(DataRefImpl Sec) const override { return 0; }
+  void moveSectionNext(DataRefImpl &Sec) const override;
+  virtual Expected<StringRef> getSectionName(DataRefImpl Sec) const override;
+  uint64_t getSectionAddress(DataRefImpl Sec) const override;
+  uint64_t getSectionSize(DataRefImpl Sec) const override;
   virtual Expected<ArrayRef<uint8_t>>
-  getSectionContents(DataRefImpl Sec) const override {
-    return ArrayRef<uint8_t>();
-  }
-  uint64_t getSectionIndex(DataRefImpl Sec) const override { return 0; }
-  uint64_t getSectionAlignment(DataRefImpl Sec) const override { return 0; }
-  bool isSectionCompressed(DataRefImpl Sec) const override { return false; }
-  bool isSectionText(DataRefImpl Sec) const override { return false; }
-  bool isSectionData(DataRefImpl Sec) const override { return false; }
-  bool isSectionBSS(DataRefImpl Sec) const override { return false; }
-  bool isSectionVirtual(DataRefImpl Sec) const override { return false; }
+  getSectionContents(DataRefImpl Sec) const override;
+  uint64_t getSectionIndex(DataRefImpl Sec) const override;
+  uint64_t getSectionAlignment(DataRefImpl Sec) const override;
+  bool isSectionCompressed(DataRefImpl Sec) const override;
+  bool isSectionText(DataRefImpl Sec) const override;
+  bool isSectionData(DataRefImpl Sec) const override;
+  bool isSectionBSS(DataRefImpl Sec) const override;
+  bool isSectionVirtual(DataRefImpl Sec) const override;
   relocation_iterator section_rel_begin(DataRefImpl Sec) const override {
     return relocation_iterator(RelocationRef(Sec, this));
   }
   relocation_iterator section_rel_end(DataRefImpl Sec) const override {
     return relocation_iterator(RelocationRef(Sec, this));
   }
 
   const uint8_t *getSectionEdEsdRecord(DataRefImpl &Sec) const;
   const uint8_t *getSectionPrEsdRecord(DataRefImpl &Sec) const;
   const uint8_t *getSectionEdEsdRecord(uint32_t SectionIndex) const;
   const uint8_t *getSectionPrEsdRecord(uint32_t SectionIndex) const;
+  uint32_t getSectionDefEsdId(DataRefImpl &Sec) const;
 
   // RelocationRef.
   void moveRelocationNext(DataRefImpl &Rel) const override{};
   uint64_t getRelocationOffset(DataRefImpl Rel) const override { return 0; }
   symbol_iterator getRelocationSymbol(DataRefImpl Rel) const override {
     DataRefImpl Temp;
     return basic_symbol_iterator(SymbolRef(Temp, this));
   }
   uint64_t getRelocationType(DataRefImpl Rel) const override { return 0; }
   void getRelocationTypeName(DataRefImpl Rel,
                              SmallVectorImpl<char> &Result) const override{};
 };
 
+class GOFFSymbolRef : public SymbolRef {
+public:
+  GOFFSymbolRef(SymbolRef B) : SymbolRef(B) {
+    assert(isa<GOFFObjectFile>(SymbolRef::getObject()));
+  }
+
+  const GOFFObjectFile *getObject() const {
+    return cast<GOFFObjectFile>(BasicSymbolRef::getObject());
+  }
+
+  Expected<uint32_t> getSymbolGOFFFlags() const {
+    return getObject()->getSymbolFlags(getRawDataRefImpl());
+  }
+
+  Expected<SymbolRef::Type> getSymbolGOFFType() const {
+    return getObject()->getSymbolType(getRawDataRefImpl());
+  }
+
+  uint64_t getSize() const {
+    return getObject()->getSymbolSize(getRawDataRefImpl());
+  }
+};
+
 } // namespace object
 
 } // namespace llvm
 
 #endif

llvm/lib/Object/GOFFObjectFile.cpp

 //===- GOFFObjectFile.cpp - GOFF object file 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
 //
 //===----------------------------------------------------------------------===//
 //
case GOFF::RT_ESD: {
           // Add the section ED which was previously ignored.
           Section.d.a = SymEdId;
           SectionList.emplace_back(Section);
         }
       }
       LLVM_DEBUG(dbgs() << "  --  ESD " << EsdId << "\n");
       break;
     }
+    case GOFF::RT_TXT:
+      // Save TXT records.
+      TextPtrs.emplace_back(I);
+      LLVM_DEBUG(dbgs() << "  --  TXT\n");
+      break;
     case GOFF::RT_END:
       LLVM_DEBUG(dbgs() << "  --  END (GOFF record type) unhandled\n");
       break;
     case GOFF::RT_HDR:
       LLVM_DEBUG(dbgs() << "  --  HDR (GOFF record type) unhandled\n");
       break;
     default:
       llvm_unreachable("Unknown record type");
for (size_t I = 0, E = SectionList.size(); I < E; ++I) {
     }
   }
   return createStringError(llvm::errc::invalid_argument,
                            "symbol with ESD id " + std::to_string(Symb.d.a) +
                                " refers to invalid section with ESD id " +
                                std::to_string(SymEdId));
 }
 
+uint64_t GOFFObjectFile::getSymbolSize(DataRefImpl Symb) const {
+  const uint8_t *Record = getSymbolEsdRecord(Symb);
+  uint32_t Length;
+  ESDRecord::getLength(Record, Length);
+  return Length;
+}
+
 const uint8_t *GOFFObjectFile::getSectionEdEsdRecord(DataRefImpl &Sec) const {
   SectionEntryImpl EsdIds = SectionList[Sec.d.a];
   const uint8_t *EsdRecord = EsdPtrs[EsdIds.d.a];
   return EsdRecord;
 }
 
 const uint8_t *GOFFObjectFile::getSectionPrEsdRecord(DataRefImpl &Sec) const {
   SectionEntryImpl EsdIds = SectionList[Sec.d.a];

 const uint8_t *
 GOFFObjectFile::getSectionPrEsdRecord(uint32_t SectionIndex) const {
   DataRefImpl Sec;
   Sec.d.a = SectionIndex;
   const uint8_t *EsdRecord = getSectionPrEsdRecord(Sec);
   return EsdRecord;
 }
 
+uint32_t GOFFObjectFile::getSectionDefEsdId(DataRefImpl &Sec) const {
+  const uint8_t *EsdRecord = getSectionEdEsdRecord(Sec);
+  uint32_t Length;
+  ESDRecord::getLength(EsdRecord, Length);
+  if (Length == 0) {
+    const uint8_t *PrEsdRecord = getSectionPrEsdRecord(Sec);
+    if (PrEsdRecord)
+      EsdRecord = PrEsdRecord;
+  }
+
+  uint32_t DefEsdId;
+  ESDRecord::getEsdId(EsdRecord, DefEsdId);
+  LLVM_DEBUG(dbgs() << "Got def EsdId: " << DefEsdId << '\n');
+  return DefEsdId;
+}
+
+void GOFFObjectFile::moveSectionNext(DataRefImpl &Sec) const {
+  if ((Sec.d.a + 1) >= SectionList.size())
+    Sec.d.a = 0; // section_end
+  else
+    Sec.d.a++;
+}
+
+Expected<StringRef> GOFFObjectFile::getSectionName(DataRefImpl Sec) const {
+  DataRefImpl EdSym;
+  SectionEntryImpl EsdIds = SectionList[Sec.d.a];
+  EdSym.d.a = EsdIds.d.a;
+  Expected<StringRef> Name = getSymbolName(EdSym);
+  if (Name) {
+    StringRef Res = *Name;
+    LLVM_DEBUG(dbgs() << "Got section: " << Res << '\n');
+    if (Res.equals("C___clangast"))
+      Res = "__clangast";
+    else if (Res.equals("C_autolink"))
+      Res = ".swift1_autolink_entries";
+    LLVM_DEBUG(dbgs() << "Final section name: " << Res << '\n');
+    Name = Res;
+  }
+  return Name;
+}
+
+uint64_t GOFFObjectFile::getSectionAddress(DataRefImpl Sec) const {
+  uint32_t Offset;
+  const uint8_t *EsdRecord = getSectionEdEsdRecord(Sec);
+  ESDRecord::getOffset(EsdRecord, Offset);
+  return Offset;
+}
+
+uint64_t GOFFObjectFile::getSectionIndex(DataRefImpl Sec) const {
+  return Sec.d.a;
+}
+
+uint64_t GOFFObjectFile::getSectionSize(DataRefImpl Sec) const {
+  uint32_t Length;
+  uint32_t DefEsdId = getSectionDefEsdId(Sec);
+  const uint8_t *EsdRecord = EsdPtrs[DefEsdId];
+  ESDRecord::getLength(EsdRecord, Length);
+  LLVM_DEBUG(dbgs() << "Got section size: " << Length << '\n');
+  return static_cast<uint64_t>(Length);
+}
+
+// Unravel TXT records and expand fill characters to produce
+// a contiguous sequence of bytes.
+Expected<ArrayRef<uint8_t>>
+GOFFObjectFile::getSectionContents(DataRefImpl Sec) const {
+  if (SectionDataCache.count(Sec.d.a)) {
+    StringRef Res = StringRef(SectionDataCache[Sec.d.a]);
+    return arrayRefFromStringRef(Res);
+  }
+
+  uint64_t SectionSize = getSectionSize(Sec);
+  uint32_t DefEsdId = getSectionDefEsdId(Sec);
+
+  const uint8_t *EdEsdRecord = getSectionEdEsdRecord(Sec);
+  uint8_t FillByte = 0;
+  ESDRecord::getFillByteValue(EdEsdRecord, FillByte);
+
+  // Initialize section with fill byte.
+  std::string &Data = SectionDataCache[Sec.d.a];
+  Data.reserve(SectionSize);
+  Data.insert(0, SectionSize, FillByte);
+
+  // Replace section with context from text records.
+  std::vector<const uint8_t *> TxtRecords;
+  for (const uint8_t *TxtRecordInt : TextPtrs) {
+    const uint8_t *TxtRecordPtr = TxtRecordInt;
+    uint32_t TxtEsdId = TXTRecord::getElementEsdId(TxtRecordPtr);
+    LLVM_DEBUG(dbgs() << "Got txt EsdId: " << TxtEsdId << '\n');
+
+    if (TxtEsdId == DefEsdId)
+      TxtRecords.push_back(TxtRecordInt);
+  }
+  LLVM_DEBUG(dbgs() << "Found " << TxtRecords.size() << " matching records\n");
+
+  for (const uint8_t *TxtRecordInt : TxtRecords) {
+    const uint8_t *TxtRecordPtr = TxtRecordInt;
+    uint32_t TxtDataOffset;
+    TXTRecord::getOffset(TxtRecordPtr, TxtDataOffset);
+    LLVM_DEBUG(dbgs() << "Got record data offset: " << TxtDataOffset << '\n');
+
+    uint16_t TxtDataSize;
+    TXTRecord::getDataLength(TxtRecordPtr, TxtDataSize);
+    LLVM_DEBUG(dbgs() << "Got record data size: " << TxtDataSize << '\n');
+
+    char *ChrPtr =
+        const_cast<char *>(reinterpret_cast<const char *>(TxtRecordInt));
+
+    SmallString<256> CompleteData;
+    if (auto Err = TXTRecord::getData(TxtRecordPtr, CompleteData))
+      return std::move(Err);
+    Data.replace(TxtDataOffset, TxtDataSize, ChrPtr + 24, TxtDataSize);
+  }
+
+  return arrayRefFromStringRef(StringRef(Data));
+}
+
+uint64_t GOFFObjectFile::getSectionAlignment(DataRefImpl Sec) const {
+  const uint8_t *EsdRecord = getSectionEdEsdRecord(Sec);
+  GOFF::ESDAlignment Pow2Alignment;
+  ESDRecord::getAlignment(EsdRecord, Pow2Alignment);
+  return UINT64_C(1) << (uint64_t)Pow2Alignment;
+}
+
+bool GOFFObjectFile::isSectionCompressed(DataRefImpl Sec) const {
+  return false;
+}
+
+bool GOFFObjectFile::isSectionText(DataRefImpl Sec) const {
+  const uint8_t *EsdRecord = getSectionEdEsdRecord(Sec);
+  GOFF::ESDExecutable Executable;
+  ESDRecord::getExecutable(EsdRecord, Executable);
+  return Executable == GOFF::ESD_EXE_CODE;
+}
+
+bool GOFFObjectFile::isSectionData(DataRefImpl Sec) const {
+  const uint8_t *EsdRecord = getSectionEdEsdRecord(Sec);
+  GOFF::ESDExecutable Executable;
+  ESDRecord::getExecutable(EsdRecord, Executable);
+  return Executable == GOFF::ESD_EXE_DATA;
+}
+
+bool GOFFObjectFile::isSectionBSS(DataRefImpl Sec) const { return false; }
+
+bool GOFFObjectFile::isSectionVirtual(DataRefImpl Sec) const { return false; }
+
 section_iterator GOFFObjectFile::section_begin() const {
   DataRefImpl Sec;
   moveSectionNext(Sec);
   return section_iterator(SectionRef(Sec, this));
 }
 
 section_iterator GOFFObjectFile::section_end() const {
   DataRefImpl Sec;
   return section_iterator(SectionRef(Sec, this));
 }
 
 void GOFFObjectFile::moveSymbolNext(DataRefImpl &Symb) const {
   for (uint32_t I = Symb.d.a + 1, E = EsdPtrs.size(); I < E; ++I) {
     if (EsdPtrs[I]) {
       const uint8_t *EsdRecord = EsdPtrs[I];
       GOFF::ESDSymbolType SymbolType;
       ESDRecord::getSymbolType(EsdRecord, SymbolType);
       // Skip EDs - i.e. section symbols.
       bool IgnoreSpecialGOFFSymbols = true;
       bool SkipSymbol = ((SymbolType == GOFF::ESD_ST_ElementDefinition) ||
                          (SymbolType == GOFF::ESD_ST_SectionDefinition)) &&
                         IgnoreSpecialGOFFSymbols;
       if (!SkipSymbol) {
         Symb.d.a = I;
         return;
       }
     }
   }
   Symb.d.a = 0;
 }
 
 basic_symbol_iterator GOFFObjectFile::symbol_begin() const {
   DataRefImpl Symb;
   moveSymbolNext(Symb);
   return basic_symbol_iterator(SymbolRef(Symb, this));
 }
 
 basic_symbol_iterator GOFFObjectFile::symbol_end() const {
   DataRefImpl Symb;
   return basic_symbol_iterator(SymbolRef(Symb, this));
 }
 
 Error Record::getContinuousData(const uint8_t *Record, uint16_t DataLength,

 }
 
 Error ESDRecord::getData(const uint8_t *Record,
                          SmallString<256> &CompleteData) {
   uint16_t DataSize = getNameLength(Record);
   return getContinuousData(Record, DataSize, 72, CompleteData);
 }
 
+Error TXTRecord::getData(const uint8_t *Record,
+                         SmallString<256> &CompleteData) {
+  uint16_t Length;
+  getDataLength(Record, Length);
+  return getContinuousData(Record, Length, 24, CompleteData);
+}
+
 Error ENDRecord::getData(const uint8_t *Record,
                          SmallString<256> &CompleteData) {
   uint16_t Length = getNameLength(Record);
   return getContinuousData(Record, Length, 26, CompleteData);
 }

llvm/unittests/Object/GOFFObjectFileTest.cpp

 //===- GOFFObjectFileTest.cpp - Tests for GOFFObjectFile ------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
TEST(GOFFObjectFileTest, InvalidERSymbolType) {
 
   for (SymbolRef Symbol : GOFFObj->symbols()) {
     Expected<SymbolRef::Type> SymbolType = Symbol.getType();
     EXPECT_THAT_EXPECTED(
         SymbolType,
         FailedWithMessage("ESD record 1 has unknown Executable type 0x03"));
   }
 }
+
+TEST(GOFFObjectFileTest, TXTConstruct) {
+  char GOFFData[GOFF::RecordLength * 6] = {};
+
+  // HDR record.
+  GOFFData[0] = 0x03;
+  GOFFData[1] = 0xF0;
+  GOFFData[50] = 0x01;
+
+  // ESD record.
+  GOFFData[GOFF::RecordLength] = 0x03;
+  GOFFData[GOFF::RecordLength + 7] = 0x01;  // ESDID.
+  GOFFData[GOFF::RecordLength + 71] = 0x05; // Size of symbol name.
+  GOFFData[GOFF::RecordLength + 72] = 0xa5; // Symbol name is v.
+  GOFFData[GOFF::RecordLength + 73] = 0x81; // Symbol name is a.
+  GOFFData[GOFF::RecordLength + 74] = 0x99; // Symbol name is r.
+  GOFFData[GOFF::RecordLength + 75] = 0x7b; // Symbol name is #.
+  GOFFData[GOFF::RecordLength + 76] = 0x83; // Symbol name is c.
+
+  // ESD record.
+  GOFFData[GOFF::RecordLength * 2] = 0x03;
+  GOFFData[GOFF::RecordLength * 2 + 3] = 0x01;
+  GOFFData[GOFF::RecordLength * 2 + 7] = 0x02;  // ESDID.
+  GOFFData[GOFF::RecordLength * 2 + 11] = 0x01; // Parent ESDID.
+  GOFFData[GOFF::RecordLength * 2 + 27] = 0x08; // Length.
+  GOFFData[GOFF::RecordLength * 2 + 40] = 0x01; // Name Space ID.
+  GOFFData[GOFF::RecordLength * 2 + 41] = 0x80;
+  GOFFData[GOFF::RecordLength * 2 + 60] = 0x04; // Size of symbol name.
+  GOFFData[GOFF::RecordLength * 2 + 61] = 0x04; // Size of symbol name.
+  GOFFData[GOFF::RecordLength * 2 + 63] = 0x0a; // Size of symbol name.
+  GOFFData[GOFF::RecordLength * 2 + 66] = 0x03; // Size of symbol name.
+  GOFFData[GOFF::RecordLength * 2 + 71] = 0x08; // Size of symbol name.
+  GOFFData[GOFF::RecordLength * 2 + 72] = 0xc3; // Symbol name is c.
+  GOFFData[GOFF::RecordLength * 2 + 73] = 0x6d; // Symbol name is _.
+  GOFFData[GOFF::RecordLength * 2 + 74] = 0xc3; // Symbol name is c.
+  GOFFData[GOFF::RecordLength * 2 + 75] = 0xd6; // Symbol name is o.
+  GOFFData[GOFF::RecordLength * 2 + 76] = 0xc4; // Symbol name is D.
+  GOFFData[GOFF::RecordLength * 2 + 77] = 0xc5; // Symbol name is E.
+  GOFFData[GOFF::RecordLength * 2 + 78] = 0xf6; // Symbol name is 6.
+  GOFFData[GOFF::RecordLength * 2 + 79] = 0xf4; // Symbol name is 4.
+
+  // ESD record.
+  GOFFData[GOFF::RecordLength * 3] = 0x03;
+  GOFFData[GOFF::RecordLength * 3 + 3] = 0x02;
+  GOFFData[GOFF::RecordLength * 3 + 7] = 0x03;  // ESDID.
+  GOFFData[GOFF::RecordLength * 3 + 11] = 0x02; // Parent ESDID.
+  GOFFData[GOFF::RecordLength * 3 + 71] = 0x05; // Size of symbol name.
+  GOFFData[GOFF::RecordLength * 3 + 72] = 0xa5; // Symbol name is v.
+  GOFFData[GOFF::RecordLength * 3 + 73] = 0x81; // Symbol name is a.
+  GOFFData[GOFF::RecordLength * 3 + 74] = 0x99; // Symbol name is r.
+  GOFFData[GOFF::RecordLength * 3 + 75] = 0x7b; // Symbol name is #.
+  GOFFData[GOFF::RecordLength * 3 + 76] = 0x83; // Symbol name is c.
+
+  // TXT record.
+  GOFFData[GOFF::RecordLength * 4] = 0x03;
+  GOFFData[GOFF::RecordLength * 4 + 1] = 0x10;
+  GOFFData[GOFF::RecordLength * 4 + 7] = 0x02;
+  GOFFData[GOFF::RecordLength * 4 + 23] = 0x08; // Data Length.
+  GOFFData[GOFF::RecordLength * 4 + 24] = 0x12;
+  GOFFData[GOFF::RecordLength * 4 + 25] = 0x34;
+  GOFFData[GOFF::RecordLength * 4 + 26] = 0x56;
+  GOFFData[GOFF::RecordLength * 4 + 27] = 0x78;
+  GOFFData[GOFF::RecordLength * 4 + 28] = 0x9a;
+  GOFFData[GOFF::RecordLength * 4 + 29] = 0xbc;
+  GOFFData[GOFF::RecordLength * 4 + 30] = 0xde;
+  GOFFData[GOFF::RecordLength * 4 + 31] = 0xf0;
+
+  // END record.
+  GOFFData[GOFF::RecordLength * 5] = 0x03;
+  GOFFData[GOFF::RecordLength * 5 + 1] = 0x40;
+  GOFFData[GOFF::RecordLength * 5 + 11] = 0x06;
+
+  StringRef Data(GOFFData, GOFF::RecordLength * 6);
+
+  Expected<std::unique_ptr<ObjectFile>> GOFFObjOrErr =
+      object::ObjectFile::createGOFFObjectFile(
+          MemoryBufferRef(Data, "dummyGOFF"));
+
+  ASSERT_THAT_EXPECTED(GOFFObjOrErr, Succeeded());
+
+  GOFFObjectFile *GOFFObj = dyn_cast<GOFFObjectFile>((*GOFFObjOrErr).get());
+  auto Symbols  = GOFFObj->symbols();
+  ASSERT_EQ(std::distance(Symbols.begin(), Symbols.end()), 1);
+  SymbolRef Symbol = *Symbols.begin();
+  Expected<StringRef> SymbolNameOrErr = GOFFObj->getSymbolName(Symbol);
+  ASSERT_THAT_EXPECTED(SymbolNameOrErr, Succeeded());
+  StringRef SymbolName = SymbolNameOrErr.get();
+  EXPECT_EQ(SymbolName, "var#c");
+
+  auto Sections  = GOFFObj->sections();
+  ASSERT_EQ(std::distance(Sections.begin(), Sections.end()), 1);
+  SectionRef Section = *Sections.begin();
+  Expected<StringRef> SectionContent = Section.getContents();
+  ASSERT_THAT_EXPECTED(SectionContent, Succeeded());
+  StringRef Contents = SectionContent.get();
+  EXPECT_EQ(Contents, "\x12\x34\x56\x78\x9a\xbc\xde\xf0");
+}