Skip to content

[SystemZ][z/OS] yaml2obj GOFF symbols #75971

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.

Sign up for GitHub

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

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

[SystemZ][z/OS] yaml2obj GOFF symbols #75971

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
36be167
add sections & symbols
Dec 19, 2023
ae601e2
add tests
Dec 19, 2023
e53969c
fix formtting
Dec 19, 2023
22c968e
remove section related code
Jan 2, 2024
25a8034
address comments
Jan 18, 2024
2ce0e3b
remove std::option for Signature
Mar 12, 2024
ddca557
remove checks
Mar 14, 2024
c92c5c2
more comments
Mar 15, 2024
263bef6
add maxdatalength to message, define NameLengthOffset
Mar 15, 2024
85251f6
remove Records label if there are none
Apr 4, 2024
975462f
add more testing
Apr 9, 2024
980e0d1
address new set of comments
Apr 16, 2024
2861f97
fix ordering for tests + other various comments
Apr 25, 2024
97625ca
fix coding standard for error
Apr 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions llvm/include/llvm/BinaryFormat/GOFF.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ enum ESDAlignment : uint8_t {
ESD_ALIGN_4Kpage = 12,
};

enum TXTRecordStyle : uint8_t {
TXT_RS_Byte = 0,
TXT_RS_Structured = 1,
TXT_RS_Unstructured = 2,
};

enum ENDEntryPointRequest : uint8_t {
END_EPR_None = 0,
END_EPR_EsdidOffset = 1,
Expand Down
126 changes: 126 additions & 0 deletions llvm/include/llvm/ObjectYAML/GOFFYAML.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,104 @@
#include <vector>

namespace llvm {
namespace GOFF {

enum ESDFlags {
ESD_FillByteValuePresent = 1 << 7,
ESD_SymbolDisplayFlag = 1 << 6,
ESD_SymbolRenamingFlag = 1 << 5,
ESD_RemovableClass = 1 << 4
};

enum {
ESD_Mask_ERST = 0x07,
ESD_Mask_RQW = 0x07,
ESD_Mask_TextStyle = 0xf0,
ESD_Mask_BindingAlgorithm = 0x0f,
};

enum ESDBAFlags {
ESD_BA_Movable = 0x01,
ESD_BA_ReadOnly = 0x2,
ESD_BA_NoPrime = 0x4,
ESD_BA_COMMON = 0x8,
ESD_BA_Indirect = 0x10,
};

} // end namespace GOFF

// 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 {

LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDSYMBOLTYPE)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDNAMESPACEID)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDFlags)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDAMODE)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDRMODE)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDTEXTSTYLE)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDBINDINGALGORITHM)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDTASKINGBEHAVIOR)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDEXECUTABLE)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDLINKAGETYPE)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDBINDINGSTRENGTH)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDLOADINGBEHAVIOR)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDBINDINGSCOPE)
LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ESDALIGNMENT)
LLVM_YAML_STRONG_TYPEDEF(uint64_t, GOFF_BAFLAGS)

LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_TEXTRECORDSTYLE)

struct RecordBase {
enum RecordBaseKind { RBK_Symbol };

private:
const RecordBaseKind Kind;

protected:
RecordBase(RecordBaseKind Kind) : Kind(Kind) {}

public:
RecordBaseKind getKind() const { return Kind; }
};

using RecordPtr = std::unique_ptr<RecordBase>;

struct Symbol : public RecordBase {
Symbol() : RecordBase(RBK_Symbol) {}

StringRef Name;
GOFF_ESDSYMBOLTYPE Type;
uint32_t ID;
uint32_t OwnerID;
uint32_t Address;
uint32_t Length;
uint32_t ExtAttrID;
uint32_t ExtAttrOffset;
GOFF_ESDNAMESPACEID NameSpace;
GOFF_ESDFlags Flags;
uint8_t FillByteValue;
uint32_t PSectID;
uint32_t Priority;
llvm::yaml::Hex64 Signature;
GOFF_ESDAMODE Amode;
GOFF_ESDRMODE Rmode;
GOFF_ESDTEXTSTYLE TextStyle;
GOFF_ESDBINDINGALGORITHM BindingAlgorithm;
GOFF_ESDTASKINGBEHAVIOR TaskingBehavior;
GOFF_ESDEXECUTABLE Executable;
GOFF_ESDLINKAGETYPE LinkageType;
GOFF_ESDBINDINGSTRENGTH BindingStrength;
GOFF_ESDLOADINGBEHAVIOR LoadingBehavior;
GOFF_ESDBINDINGSCOPE BindingScope;
GOFF_ESDALIGNMENT Alignment;
GOFF_BAFLAGS BAFlags;

static bool classof(const RecordBase *Rec) {
return Rec->getKind() == RBK_Symbol;
}
};

struct FileHeader {
uint32_t TargetEnvironment = 0;
uint32_t TargetOperatingSystem = 0;
Expand All @@ -38,12 +131,45 @@ struct FileHeader {

struct Object {
FileHeader Header;
std::vector<RecordPtr> Records;
Object();
};

} // end namespace GOFFYAML
} // end namespace llvm

LLVM_YAML_IS_SEQUENCE_VECTOR(GOFFYAML::RecordPtr)

LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDSYMBOLTYPE)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDNAMESPACEID)
LLVM_YAML_DECLARE_BITSET_TRAITS(GOFFYAML::GOFF_ESDFlags)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDAMODE)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDRMODE)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDTEXTSTYLE)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDBINDINGALGORITHM)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDTASKINGBEHAVIOR)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDEXECUTABLE)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDLINKAGETYPE)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDBINDINGSTRENGTH)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDLOADINGBEHAVIOR)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDBINDINGSCOPE)
LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ESDALIGNMENT)
LLVM_YAML_DECLARE_BITSET_TRAITS(GOFFYAML::GOFF_BAFLAGS)

LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_TEXTRECORDSTYLE)

LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::Symbol)
LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::FileHeader)
LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::Object)

namespace llvm {
namespace yaml {

template <> struct CustomMappingTraits<GOFFYAML::RecordPtr> {
static void inputOne(IO &IO, StringRef Key, GOFFYAML::RecordPtr &Elem);
static void output(IO &IO, GOFFYAML::RecordPtr &Elem);
};

} // end namespace yaml
} // end namespace llvm
#endif // LLVM_OBJECTYAML_GOFFYAML_H
57 changes: 57 additions & 0 deletions llvm/lib/ObjectYAML/GOFFEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ class GOFFOstream : public raw_ostream {
class GOFFState {
void writeHeader(GOFFYAML::FileHeader &FileHdr);
void writeEnd();
void writeSymbol(GOFFYAML::Symbol Sym);

void reportError(const Twine &Msg) {
ErrHandler(Msg);
Expand Down Expand Up @@ -245,6 +246,53 @@ void GOFFState::writeHeader(GOFFYAML::FileHeader &FileHdr) {
}
}

void GOFFState::writeSymbol(GOFFYAML::Symbol Sym) {
SmallString<80> SymName;
if (std::error_code EC = ConverterEBCDIC::convertToEBCDIC(Sym.Name, SymName))
reportError("conversion error on " + Sym.Name + ": " + EC.message());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Coming back to this fresh, I think we can reword this:

Suggested change
reportError("conversion error on " + Sym.Name + ": " + EC.message());
reportError("cannot convert '" + Sym.Name + "' to EBCDIC: " + EC.message());

You also should have a test case which triggers this error.

size_t SymNameLength = SymName.size();
if (SymNameLength > GOFF::MaxDataLength)
reportError("symbol name is too long: " + Twine(SymNameLength) +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test case for this error, please.

". Max length is: " + Twine(GOFF::MaxDataLength));

const unsigned NameLengthOffset = 69;
GW.makeNewRecord(GOFF::RT_ESD, NameLengthOffset + SymNameLength);
GW << binaryBe(Sym.Type) // Symbol type
<< binaryBe(Sym.ID) // ESDID
<< binaryBe(Sym.OwnerID) // Owner ESDID
<< binaryBe(uint32_t(0)) // Reserved
<< binaryBe(Sym.Address) // Offset/Address
<< binaryBe(uint32_t(0)) // Reserved
<< binaryBe(Sym.Length) // Length
<< binaryBe(Sym.ExtAttrID) // Extended attributes
<< binaryBe(Sym.ExtAttrOffset) // Extended attributes data offset
<< binaryBe(uint32_t(0)) // Reserved
<< binaryBe(Sym.NameSpace) // Namespace ID
<< binaryBe(Sym.Flags) // Flags
<< binaryBe(Sym.FillByteValue) // Fill byte value
<< binaryBe(uint8_t(0)) // Reserved
<< binaryBe(Sym.PSectID) // PSECT ID
<< binaryBe(Sym.Priority); // Priority
if (Sym.Signature)
GW << Sym.Signature; // Signature
else
GW << zeros(8);
#define BIT(E, N) (Sym.BAFlags & GOFF::E ? 1 << (7 - N) : 0)
GW << binaryBe(Sym.Amode) // Behavioral attributes - Amode
<< binaryBe(Sym.Rmode) // Behavioral attributes - Rmode
<< binaryBe(uint8_t(Sym.TextStyle << 4 | Sym.BindingAlgorithm))
<< binaryBe(uint8_t(Sym.TaskingBehavior << 5 | BIT(ESD_BA_Movable, 3) |
BIT(ESD_BA_ReadOnly, 4) | Sym.Executable))
<< binaryBe(uint8_t(BIT(ESD_BA_NoPrime, 1) | Sym.BindingStrength))
<< binaryBe(uint8_t(Sym.LoadingBehavior << 6 | BIT(ESD_BA_COMMON, 2) |
BIT(ESD_BA_Indirect, 3) | Sym.BindingScope))
<< binaryBe(uint8_t(Sym.LinkageType << 5 | Sym.Alignment))
<< zeros(3) // Behavioral attributes - Reserved
<< binaryBe(static_cast<uint16_t>(SymNameLength)) // Name length
<< SymName.str();
#undef BIT
}

void GOFFState::writeEnd() {
GW.makeNewRecord(GOFF::RT_END, GOFF::PayloadLength);
GW << binaryBe(uint8_t(0)) // No entry point
Expand All @@ -259,6 +307,15 @@ bool GOFFState::writeObject() {
writeHeader(Doc.Header);
if (HasError)
return false;
// Iterate over all records.
unsigned RecordNum = 0;
for (const std::unique_ptr<llvm::GOFFYAML::RecordBase> &Rec : Doc.Records) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should look up the enumerate function to allow iterating over the range whilst also producing an index.

if (const auto *Sym = dyn_cast<GOFFYAML::Symbol>(Rec.get()))
writeSymbol(*Sym);
else
reportError("unknown record type on record index" + Twine(RecordNum));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
reportError("unknown record type on record index" + Twine(RecordNum));
reportError("record with index " + Twine(RecordNum) + " has unknown record type " + Twine(Rec.Type));

This is clearer, I think. Rec.Type may not be the right thing - I just used this as a placeholder so that I didn't have to look up the actual member that is used to determine what kind of Record it is.

You should also have a test case for this error.

Copy link
Member

Choose a reason for hiding this comment

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

That is running in the wrong direction. The if is over the discriminator used for the inheritance hierarchy. There is no user input involved. The error message made no sense in the first place. The whole loop should be:

  for (const auto &Rec: Doc.Records) {
    GOFFYAML::RecordBase *RecBase = Rec.get();
    switch (RecBase->getKind()) {
      case GOFFYAML::RecordBase::RBK_Symbol:
        writeSymbol(*static_cast<GOFFYAML::Symbol *>(RecBase));
    }
  }

There is no default case because the switch covers all cases. I put this into the updated PR.

RecordNum++;
}
writeEnd();
return true;
}
Expand Down
Loading
Loading