Skip to content

[llvm-objdump] Implement decoding auxiliary header for xcoff with llvm-objdump --private-headers #105682

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

Merged
merged 8 commits into from
Nov 7, 2024
Merged

[llvm-objdump] Implement decoding auxiliary header for xcoff with llvm-objdump --private-headers #105682

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3a52dd2
implement decoding auxiliary header for xcoff with llvm-objdump --pri…
Aug 22, 2024
935775c
minor change
diggerlin Aug 22, 2024
e38e11f
add more FileCheck options
diggerlin Aug 22, 2024
8bfbee4
address comment
diggerlin Sep 16, 2024
4535a2f
reorder the run commands and add comments for test case
diggerlin Sep 16, 2024
de6dce5
implement decoding auxiliary header for xcoff with llvm-objdump --pri…
Aug 22, 2024
081f917
change python to %python
diggerlin Oct 21, 2024
00ce595
address comment
diggerlin Nov 5, 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
128 changes: 128 additions & 0 deletions llvm/test/tools/llvm-objdump/XCOFF/private-header-auxiliary.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
## Test that `the llvm-objdump --private-headers` print out the auxiliary header of XCOFF object file.
Copy link
Collaborator

@jh7370 jh7370 Aug 28, 2024

Choose a reason for hiding this comment

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

Please review what I've said in your past reviews about providing comments for individual cases within the test, ordering of RUN/YAML/CHECK lines and whitespace (especially blank lines), and act on them in this test file too.

Have you run this test locally at all?

Copy link
Contributor Author

@diggerlin diggerlin Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

yes, I run this test locally

Copy link
Collaborator

@jh7370 jh7370 Oct 11, 2024 •
edited by diggerlin
Loading

Choose a reason for hiding this comment

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

According to the pre-merge checks, this test is failing on Linux. Please review and fix.

I've also previously said that I'd prefer CHECK lines to be immediately after the corresponding test case, which you haven't addressed here.

Copy link
Collaborator

@jh7370 jh7370 Oct 11, 2024

Choose a reason for hiding this comment

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

Suggested change
## Test that `the llvm-objdump --private-headers` print out the auxiliary header of XCOFF object file.
## Test that `the llvm-objdump --private-headers` prints out the auxiliary header of an XCOFF object file.


# RUN: yaml2obj %s -o %t1
# RUN: llvm-objdump --private-headers %t1 | \
# RUN: FileCheck %s --check-prefix=CHECK32 --match-full-lines --strict-whitespace
# RUN: cp %t1 %t1_err1
# RUN: python -c "with open(r'%t1_err1', 'r+b') as input: input.seek(17); input.write(b'\x45')"
# RUN: python -c "with open(r'%t1_err1', 'r+b') as input: input.seek(4); input.write(b'\x00')"
Copy link
Collaborator

@jh7370 jh7370 Aug 28, 2024

Choose a reason for hiding this comment

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

Is there a reason these two lines can't be folded together into one python statement? You may also wish to look at using split-file, to allow you to have a proper python script, rather than these inline snippets.

Copy link
Contributor Author

@diggerlin diggerlin Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

thanks, combine to one line. This a one line python script, I do not think it need a split-file.

# RUN: llvm-objdump --private-headers %t1_err1 2>&1 | FileCheck %s --check-prefix=WARN1 --match-full-lines
# RUN: cp %t1 %t1_extra
# RUN: python -c "with open(r'%t1_extra', 'r+b') as input: input.seek(4); input.write(b'\x00')"
# RUN: python -c "with open(r'%t1_extra', 'r+b') as input: input.seek(17); input.write(b'\x4f')"
# RUN: llvm-objdump --private-headers %t1_extra 2>&1 | FileCheck %s --check-prefix=EXTRA --match-full-lines
# RUN: yaml2obj %s -DMAGIC=0x1F7 -DFLAG64=0x2 -o %t2
# RUN: llvm-objdump --private-headers %t2 | \
# RUN: FileCheck %s --check-prefix=CHECK64 --match-full-lines --strict-whitespace


# CHECK32:---Auxiliary Header:
Copy link
Collaborator

@jh7370 jh7370 Oct 11, 2024

Choose a reason for hiding this comment

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

There's a lot of duplication between the 32-bit and 64-bit checks here. Could you use a pattern of something like CHECK for common parts, and CHECK32/CHECK64 for parts that are not common, to reduce the duplication?

# CHECK32-NEXT:Magic: 0x10b
# CHECK32-NEXT:Version: 0x1
# CHECK32-NEXT:Size of .text section: 0x8
# CHECK32-NEXT:Size of .data section: 0x9
# CHECK32-NEXT:Size of .bss section: 0x10
# CHECK32-NEXT:Entry point address: 0x1111
# CHECK32-NEXT:.text section start address: 0x2222
# CHECK32-NEXT:.data section start address; 0x3333
# CHECK32-NEXT:TOC anchor address: 0x4444
# CHECK32-NEXT:Section number of entryPoint: 1
# CHECK32-NEXT:Section number of .text: 2
# CHECK32-NEXT:Section number of .data: 3
# CHECK32-NEXT:Section number of TOC: 4
# CHECK32-NEXT:Section number of loader data: 5
# CHECK32-NEXT:Section number of .bss: 6
# CHECK32-NEXT:Maxium alignment of .text: 0x7
# CHECK32-NEXT:Maxium alignment of .data: 0x3
# CHECK32-NEXT:Module type; 0x0
# CHECK32-NEXT:CPU type of objects: 0x1
# CHECK32-NEXT:(Reserved): 0x0
Copy link
Collaborator

@jh7370 jh7370 Aug 28, 2024

Choose a reason for hiding this comment

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

Does a Reserved field really need printing?

Copy link
Contributor Author

@diggerlin diggerlin Sep 16, 2024

Choose a reason for hiding this comment

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

thank, I deleted it.

# CHECK32-NEXT:Maximum stack size: 0x0
# CHECK32-NEXT:Maximum data size: 0x0
# CHECK32-NEXT:Reserved for debugger: 0x0
# CHECK32-NEXT:Text page size: 0x1
# CHECK32-NEXT:Data page size: 0x1
# CHECK32-NEXT:Stack page size: 0x1
# CHECK32-NEXT:Flag: 0x0
# CHECK32-NEXT:Alignment of thread-local storage: 0x1
# CHECK32-NEXT:Section number for .tdata: 7
# CHECK32-NEXT:Section number for .tbss: 8

# CHECK64:---Auxiliary Header:
# CHECK64-NEXT:Magic: 0x10b
# CHECK64-NEXT:Version: 0x1
# CHECK64-NEXT:Reserved for debugger: 0x0
# CHECK64-NEXT:.text section start address: 0x2222
# CHECK64-NEXT:.data section start address: 0x3333
# CHECK64-NEXT:TOC anchor address: 0x4444
# CHECK64-NEXT:Section number of entryPoint: 1
# CHECK64-NEXT:Section number of .text: 2
# CHECK64-NEXT:Section number of .data: 3
# CHECK64-NEXT:Section number of TOC: 4
# CHECK64-NEXT:Section number of loader data: 5
# CHECK64-NEXT:Section number of .bss: 6
# CHECK64-NEXT:Maxium alignment of .text: 0x7
# CHECK64-NEXT:Maxium alignment of .data: 0x3
# CHECK64-NEXT:Module type: 0x0
# CHECK64-NEXT:CPU type of objects: 0x1
# CHECK64-NEXT:(Reserved): 0x0
# CHECK64-NEXT:Text page size: 0x1
# CHECK64-NEXT:Data page size: 0x1
# CHECK64-NEXT:Stack page size: 0x1
# CHECK64-NEXT:Flag: 0x0
# CHECK64-NEXT:Alignment of thread-local storage: 0x1
# CHECK64-NEXT:Size of .text section: 0x8
# CHECK64-NEXT:Size of .data section: 0x9
# CHECK64-NEXT:Size of .bss section: 0x10
# CHECK64-NEXT:Entry point address: 0x1111
# CHECK64-NEXT:Maximum stack size: 0x0
# CHECK64-NEXT:Maximum data size: 0x0
# CHECK64-NEXT:Section number for .tdata: 7
# CHECK64-NEXT:Section number for .tbss: 8
# CHECK64-NEXT:Additional flags 64-bit XCOFF: 0x2

# WARN1: {{.*}}: only partial field for Section number for .tdata: at offset (68)
# WARN1-NEXT: Raw data (00)

# EXTRA: Extra raw data (00000000 000000)

--- !XCOFF
FileHeader:
MagicNumber: [[MAGIC=0x1DF]]
AuxiliaryHeader:
Magic: 0x10B
Version: 0x1
TextSectionSize: 0x8
DataSectionSize: 0x9
BssSectionSize: 0x10
EntryPointAddr: 0x1111
TextStartAddr: 0x2222
DataStartAddr: 0x3333
TOCAnchorAddr: 0x4444
SecNumOfEntryPoint: 1
SecNumOfText: 2
SecNumOfData: 3
SecNumOfTOC: 4
SecNumOfLoader: 5
SecNumOfBSS: 6
MaxAlignOfText: 0x7
MaxAlignOfData: 0x3
ModuleType: 0x1
TextPageSize: 0x1
DataPageSize: 0x1
StackPageSize: 0x1
SecNumOfTData: 7
SecNumOfTBSS: 8
FlagAndTDataAlignment: 0x1
Flag: [[FLAG64=<none>]]
Sections:
- Flags: [ STYP_TEXT ]
SectionData: "1232"
- Flags: [ STYP_DATA ]
SectionData: "5678"
- Flags: [ STYP_BSS ]
SectionData: "9101"
- Flags: [ STYP_TDATA ]
SectionData: "1112"
- Flags: [ STYP_TBSS ]
SectionData: "1314"
187 changes: 184 additions & 3 deletions llvm/tools/llvm-objdump/XCOFFDump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,26 @@ class XCOFFDumper : public objdump::Dumper {

public:
XCOFFDumper(const object::XCOFFObjectFile &O) : Dumper(O), Obj(O) {}
void printBinary(StringRef Name, ArrayRef<uint8_t> B);
void printHex(StringRef Name, uint64_t Value);
void printNumber(StringRef Name, uint64_t Value);

private:
void printPrivateHeaders() override;
void printFileHeader();
void printAuxiliaryHeader();
void printAuxiliaryHeader(const XCOFFAuxiliaryHeader32 *AuxHeader);
void printAuxiliaryHeader(const XCOFFAuxiliaryHeader64 *AuxHeader);
FormattedString formatName(StringRef Name);
void printHex(StringRef Name, uint64_t Value);
void printNumber(StringRef Name, uint64_t Value);
void printStrHex(StringRef Name, StringRef Str, uint64_t Value);
void setWidth(unsigned W) { Width = W; };
unsigned getWidth() { return Width; }
};

void XCOFFDumper::printPrivateHeaders() { printFileHeader(); }
void XCOFFDumper::printPrivateHeaders() {
printFileHeader();
printAuxiliaryHeader();
}

FormattedString XCOFFDumper::formatName(StringRef Name) {
return FormattedString(Name, Width, FormattedString::JustifyLeft);
Expand All @@ -67,6 +75,179 @@ void XCOFFDumper::printStrHex(StringRef Name, StringRef Str, uint64_t Value) {
<< ")\n";
}

void XCOFFDumper::printBinary(StringRef Name, ArrayRef<uint8_t> B) {
unsigned OrgWidth = getWidth();
setWidth(0);
outs() << formatName(Name) << " (" << format_bytes(B) << ")\n";
setWidth(OrgWidth);
}

void XCOFFDumper::printAuxiliaryHeader() {
setWidth(36);
if (Obj.is64Bit())
printAuxiliaryHeader(Obj.auxiliaryHeader64());
else
printAuxiliaryHeader(Obj.auxiliaryHeader32());
}

enum PrintStyle { Hex, Number };
template <typename T, typename V>
Copy link
Collaborator

@jh7370 jh7370 Oct 11, 2024

Choose a reason for hiding this comment

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

I'd suggest naming these template parameters to more clearly indicate what the type is supposed to be.

I'm also wondering whether this and checkAndPrintAuxHeaderParseError should be private members of the XCOFFDumper class, so that you don't need to make methods like printHex public.

static void printAuxMemberHelper(PrintStyle Style, const char *MemberName,
const T &Member, const V *AuxHeader,
uint16_t AuxSize, uint16_t &PartialFieldOffset,
const char *&PartialFieldName,
XCOFFDumper *Dumper) {
ptrdiff_t Offset = reinterpret_cast<const char *>(&Member) -
reinterpret_cast<const char *>(AuxHeader);
if (Offset + sizeof(Member) <= AuxSize)
Style == Hex ? Dumper->printHex(MemberName, Member)
: Dumper->printNumber(MemberName, Member);
else if (Offset < AuxSize) {
PartialFieldOffset = Offset;
PartialFieldName = MemberName;
}
}

template <class T>
Copy link
Collaborator

@jh7370 jh7370 Oct 11, 2024

Choose a reason for hiding this comment

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

Same comment as above re. clearer name. Also, be consistent with using typename/class for type parameters in templates.

void checkAndPrintAuxHeaderParseError(const char *PartialFieldName,
uint16_t PartialFieldOffset,
uint16_t AuxSize, T &AuxHeader,
XCOFFDumper *Dumper) {
if (PartialFieldOffset < AuxSize) {
Dumper->reportUniqueWarning(Twine("only partial field for ") +
PartialFieldName + " at offset (" +
Twine(PartialFieldOffset) + ")");
Dumper->printBinary(
Copy link
Collaborator

@jh7370 jh7370 Aug 28, 2024

Choose a reason for hiding this comment

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

You are presumably aware that this will print to stdout, whereas the warning will be printed to stderr, so they may not line up together?

"Raw data",
ArrayRef<uint8_t>(reinterpret_cast<const uint8_t *>(&AuxHeader) +
PartialFieldOffset,
AuxSize - PartialFieldOffset));
} else if (sizeof(AuxHeader) < AuxSize)
Copy link
Collaborator

@jh7370 jh7370 Aug 28, 2024

Choose a reason for hiding this comment

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

Please review the coding standards here: https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements. Note in particular the need for uniformity in an if/else-if chain.

Dumper->printBinary(
"Extra raw data",
ArrayRef<uint8_t>(reinterpret_cast<const uint8_t *>(&AuxHeader) +
sizeof(AuxHeader),
AuxSize - sizeof(AuxHeader)));
}

void XCOFFDumper::printAuxiliaryHeader(
const XCOFFAuxiliaryHeader32 *AuxHeader) {
if (AuxHeader == nullptr)
return;
outs() << "\n---Auxiliary Header:\n";
uint16_t AuxSize = Obj.getOptionalHeaderSize();
uint16_t PartialFieldOffset = AuxSize;
const char *PartialFieldName = nullptr;

auto PrintAuxMember = [&](PrintStyle Style, const char *MemberName,
auto &Member) {
printAuxMemberHelper(Style, MemberName, Member, AuxHeader, AuxSize,
PartialFieldOffset, PartialFieldName, this);
};

PrintAuxMember(Hex, "Magic:", AuxHeader->AuxMagic);
PrintAuxMember(Hex, "Version:", AuxHeader->Version);
PrintAuxMember(Hex, "Size of .text section:", AuxHeader->TextSize);
PrintAuxMember(Hex, "Size of .data section:", AuxHeader->InitDataSize);
PrintAuxMember(Hex, "Size of .bss section:", AuxHeader->BssDataSize);
PrintAuxMember(Hex, "Entry point address:", AuxHeader->EntryPointAddr);
PrintAuxMember(Hex, ".text section start address:", AuxHeader->TextStartAddr);
PrintAuxMember(Hex, ".data section start address;", AuxHeader->DataStartAddr);
PrintAuxMember(Hex, "TOC anchor address:", AuxHeader->TOCAnchorAddr);
PrintAuxMember(
Number, "Section number of entryPoint:", AuxHeader->SecNumOfEntryPoint);
PrintAuxMember(Number, "Section number of .text:", AuxHeader->SecNumOfText);
PrintAuxMember(Number, "Section number of .data:", AuxHeader->SecNumOfData);
PrintAuxMember(Number, "Section number of TOC:", AuxHeader->SecNumOfTOC);
PrintAuxMember(Number,
"Section number of loader data:", AuxHeader->SecNumOfLoader);
PrintAuxMember(Number, "Section number of .bss:", AuxHeader->SecNumOfBSS);
PrintAuxMember(Hex, "Maxium alignment of .text:", AuxHeader->MaxAlignOfText);
PrintAuxMember(Hex, "Maxium alignment of .data:", AuxHeader->MaxAlignOfData);
PrintAuxMember(Hex, "Module type;", AuxHeader->ModuleType);
PrintAuxMember(Hex, "CPU type of objects:", AuxHeader->CpuFlag);
PrintAuxMember(Hex, "(Reserved):", AuxHeader->CpuType);
PrintAuxMember(Hex, "Maximum stack size:", AuxHeader->MaxStackSize);
PrintAuxMember(Hex, "Maximum data size:", AuxHeader->MaxDataSize);
PrintAuxMember(Hex, "Reserved for debugger:", AuxHeader->ReservedForDebugger);
PrintAuxMember(Hex, "Text page size:", AuxHeader->TextPageSize);
PrintAuxMember(Hex, "Data page size:", AuxHeader->DataPageSize);
PrintAuxMember(Hex, "Stack page size:", AuxHeader->StackPageSize);
if (offsetof(XCOFFAuxiliaryHeader32, FlagAndTDataAlignment) +
sizeof(XCOFFAuxiliaryHeader32::FlagAndTDataAlignment) <=
AuxSize) {
printHex("Flag:", AuxHeader->getFlag());
printHex("Alignment of thread-local storage:",
AuxHeader->getTDataAlignment());
}

PrintAuxMember(Number,
"Section number for .tdata:", AuxHeader->SecNumOfTData);
PrintAuxMember(Number, "Section number for .tbss:", AuxHeader->SecNumOfTBSS);

checkAndPrintAuxHeaderParseError(PartialFieldName, PartialFieldOffset,
AuxSize, *AuxHeader, this);
}

void XCOFFDumper::printAuxiliaryHeader(
const XCOFFAuxiliaryHeader64 *AuxHeader) {
if (AuxHeader == nullptr)
return;
uint16_t AuxSize = Obj.getOptionalHeaderSize();
outs() << "\n---Auxiliary Header:\n";
uint16_t PartialFieldOffset = AuxSize;
const char *PartialFieldName = nullptr;

auto PrintAuxMember = [&](PrintStyle Style, const char *MemberName,
auto &Member) {
printAuxMemberHelper(Style, MemberName, Member, AuxHeader, AuxSize,
PartialFieldOffset, PartialFieldName, this);
};

PrintAuxMember(Hex, "Magic:", AuxHeader->AuxMagic);
PrintAuxMember(Hex, "Version:", AuxHeader->Version);
PrintAuxMember(Hex, "Reserved for debugger:", AuxHeader->ReservedForDebugger);
PrintAuxMember(Hex, ".text section start address:", AuxHeader->TextStartAddr);
PrintAuxMember(Hex, ".data section start address:", AuxHeader->DataStartAddr);
PrintAuxMember(Hex, "TOC anchor address:", AuxHeader->TOCAnchorAddr);
PrintAuxMember(
Number, "Section number of entryPoint:", AuxHeader->SecNumOfEntryPoint);
PrintAuxMember(Number, "Section number of .text:", AuxHeader->SecNumOfText);
PrintAuxMember(Number, "Section number of .data:", AuxHeader->SecNumOfData);
PrintAuxMember(Number, "Section number of TOC:", AuxHeader->SecNumOfTOC);
PrintAuxMember(Number,
"Section number of loader data:", AuxHeader->SecNumOfLoader);
PrintAuxMember(Number, "Section number of .bss:", AuxHeader->SecNumOfBSS);
PrintAuxMember(Hex, "Maxium alignment of .text:", AuxHeader->MaxAlignOfText);
PrintAuxMember(Hex, "Maxium alignment of .data:", AuxHeader->MaxAlignOfData);
PrintAuxMember(Hex, "Module type:", AuxHeader->ModuleType);
PrintAuxMember(Hex, "CPU type of objects:", AuxHeader->CpuFlag);
PrintAuxMember(Hex, "(Reserved):", AuxHeader->CpuType);
PrintAuxMember(Hex, "Text page size:", AuxHeader->TextPageSize);
PrintAuxMember(Hex, "Data page size:", AuxHeader->DataPageSize);
PrintAuxMember(Hex, "Stack page size:", AuxHeader->StackPageSize);
if (offsetof(XCOFFAuxiliaryHeader64, FlagAndTDataAlignment) +
sizeof(XCOFFAuxiliaryHeader64::FlagAndTDataAlignment) <=
AuxSize) {
printHex("Flag:", AuxHeader->getFlag());
printHex("Alignment of thread-local storage:",
AuxHeader->getTDataAlignment());
}
PrintAuxMember(Hex, "Size of .text section:", AuxHeader->TextSize);
PrintAuxMember(Hex, "Size of .data section:", AuxHeader->InitDataSize);
PrintAuxMember(Hex, "Size of .bss section:", AuxHeader->BssDataSize);
PrintAuxMember(Hex, "Entry point address:", AuxHeader->EntryPointAddr);
PrintAuxMember(Hex, "Maximum stack size:", AuxHeader->MaxStackSize);
PrintAuxMember(Hex, "Maximum data size:", AuxHeader->MaxDataSize);
PrintAuxMember(Number,
"Section number for .tdata:", AuxHeader->SecNumOfTData);
PrintAuxMember(Number, "Section number for .tbss:", AuxHeader->SecNumOfTBSS);
PrintAuxMember(Hex, "Additional flags 64-bit XCOFF:", AuxHeader->XCOFF64Flag);

checkAndPrintAuxHeaderParseError(PartialFieldName, PartialFieldOffset,
AuxSize, *AuxHeader, this);
}

void XCOFFDumper::printFileHeader() {
setWidth(20);
outs() << "\n---File Header:\n";
Expand Down