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

[llvm-objdump] Print out xcoff load section of xcoff object file with option private-headers #121226

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

Conversation

diggerlin
Copy link
Contributor

@diggerlin diggerlin commented Dec 27, 2024

[llvm-objdump] Print out xcoff load section of xcoff object file with option private-headers

@diggerlin diggerlin changed the title [llvm-objdump] Print out xcoff file header for xcoff object file with option private-headers [llvm-objdump] Print out xcoff load header for xcoff object file with option private-headers Dec 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 27, 2024

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

Author: zhijian lin (diggerlin)

Changes

[llvm-objdump] Print out xcoff file header for xcoff object file with option private-headers


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

2 Files Affected:

  • (added) llvm/test/tools/llvm-objdump/XCOFF/private-headers-option.test (+83)
  • (modified) llvm/tools/llvm-objdump/XCOFFDump.cpp (+41)
diff --git a/llvm/test/tools/llvm-objdump/XCOFF/private-headers-option.test b/llvm/test/tools/llvm-objdump/XCOFF/private-headers-option.test
new file mode 100644
index 00000000000000..d3409810f0c250
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/XCOFF/private-headers-option.test
@@ -0,0 +1,83 @@
+## Test the --privated-headers option for XCOFF object files.
+
+# RUN: yaml2obj --docnum=1 %s -o %t_xcoff32.o
+# RUN: yaml2obj --docnum=2  %s -o %t_xcoff64.o
+# RUN: llvm-objdump --private-headers %t_xcoff32.o |\
+# RUN:   FileCheck %s --check-prefixes=CHECK32 --match-full-lines
+# RUN: llvm-objdump --private-headers %t_xcoff64.o |\
+# RUN:   FileCheck %s --check-prefixes=CHECK64 --match-full-lines
+
+--- !XCOFF
+FileHeader:
+  MagicNumber:       0x1DF
+Sections:
+  - Name:            .loader
+    Flags:           [ STYP_LOADER ]
+    SectionData:     "0000000100000003000000050000016D00000001000000A40000001800000211"
+##                    ^-------                                                           -Version=1
+##                            ^-------                                                   -NumberOfSymbolEntries=3
+##                                    ^-------                                           -NumberOfRelocationEntries=5
+##                                            ^-------                                   -LengthOfImportFileIDStringTable=365
+##                                                    ^-------                           -NumberOfImportFileIDs=1
+##                                                            ^-------                   -OffsetToImportFileIDs=0xA4
+##                                                                    ^-------           -LengthOfStringTable=24
+##                                                                            ^-------   -OffsetToStringTable=0x211
+
+
+--- !XCOFF
+FileHeader:
+  MagicNumber:       0x1F7
+Sections:
+  - Name:            .loader
+    Flags:           [ STYP_LOADER ]
+    SectionData:     "0000000200000003000000050000016D000000010000002300000000000000D0000000000000023D00000000000000380000000000000080"
+##                    ^-------                                                           -Version=2
+##                            ^-------                                                   -NumberOfSymbolEntries=3
+##                                    ^-------                                           -NumberOfRelocationEntries=5
+##                                            ^-------                                   -LengthOfImportFileIDStringTable=365
+##                                                    ^-------                           -NumberOfImportFileIDs=1
+##                                                            ^-------                   --LengthOfStringTable=0x23
+##                                                                    ^---------------   -OffsetToImportFileIDs=0xD0
+##                                                                                    ^---------------                                        -OffsetToStringTable=0x23D
+##                                                                                                    ^--------------                         -OffsetToSymbolTable=0x38
+##                                                                                                                    ^---------------        -OffsetToRelocationEntries=0x80
+
+# CHECK32:      ---File Header:
+# CHECK32-NEXT: Magic:              0x1df
+# CHECK32-NEXT: NumberOfSections:   1
+# CHECK32-NEXT: Timestamp:          None (0)
+# CHECK32-NEXT: SymbolTableOffset:  0x0
+# CHECK32-NEXT: SymbolTableEntries: 0
+# CHECK32-NEXT: OptionalHeaderSize: 0x0
+# CHECK32-NEXT: Flags:              0x0
+
+# CHECK32:      ---Loader Section Header:
+# CHECK32-NEXT: Version:                           1
+# CHECK32-NEXT: NumberOfSymbolEntries:             3
+# CHECK32-NEXT: NumberOfRelocationEntries:         5
+# CHECK32-NEXT: LengthOfImportFileIDStringTable:   365
+# CHECK32-NEXT: NumberOfImportFileIDs:             1
+# CHECK32-NEXT: OffsetToImportFileIDs:             0xa4
+# CHECK32-NEXT: LengthOfStringTable:               24
+# CHECK32-NEXT: OffsetToStringTable:               0x211
+
+# CHECK64:      ---File Header:
+# CHECK64-NEXT: Magic:              0x1f7
+# CHECK64-NEXT: NumberOfSections:   1
+# CHECK64-NEXT: Timestamp:          None (0)
+# CHECK64-NEXT: SymbolTableOffset:  0x0
+# CHECK64-NEXT: SymbolTableEntries: 0
+# CHECK64-NEXT: OptionalHeaderSize: 0x0
+# CHECK64-NEXT: Flags:              0x0
+
+# CHECK64:      ---Loader Section Header:
+# CHECK64-NEXT: Version:                           2
+# CHECK64-NEXT: NumberOfSymbolEntries:             3
+# CHECK64-NEXT: NumberOfRelocationEntries:         5
+# CHECK64-NEXT: LengthOfImportFileIDStringTable:   365
+# CHECK64-NEXT: NumberOfImportFileIDs:             1
+# CHECK64-NEXT: OffsetToImportFileIDs:             0xd0
+# CHECK64-NEXT: LengthOfStringTable:               35
+# CHECK64-NEXT: OffsetToStringTable:               0x23d
+# CHECK64-NEXT: OffsetToSymbolTable                0x38
+# CHECK64-NEXT: OffsetToRelocationEntries          0x80
diff --git a/llvm/tools/llvm-objdump/XCOFFDump.cpp b/llvm/tools/llvm-objdump/XCOFFDump.cpp
index 92f9ee4fb9f96d..61dd0b87139794 100644
--- a/llvm/tools/llvm-objdump/XCOFFDump.cpp
+++ b/llvm/tools/llvm-objdump/XCOFFDump.cpp
@@ -42,6 +42,7 @@ class XCOFFDumper : public objdump::Dumper {
   void printPrivateHeaders() override;
   void printFileHeader();
   void printAuxiliaryHeader();
+  void printLoaderSectionHeader();
   void printAuxiliaryHeader(const XCOFFAuxiliaryHeader32 *AuxHeader);
   void printAuxiliaryHeader(const XCOFFAuxiliaryHeader64 *AuxHeader);
   template <typename AuxHeaderMemberType, typename XCOFFAuxiliaryHeader>
@@ -66,6 +67,7 @@ class XCOFFDumper : public objdump::Dumper {
 void XCOFFDumper::printPrivateHeaders() {
   printFileHeader();
   printAuxiliaryHeader();
+  printLoaderSectionHeader();
 }
 
 FormattedString XCOFFDumper::formatName(StringRef Name) {
@@ -262,6 +264,45 @@ void XCOFFDumper::printAuxiliaryHeader(
                                    AuxSize, *AuxHeader);
 }
 
+void XCOFFDumper::printLoaderSectionHeader() {
+  Expected<uintptr_t> LoaderSectionAddrOrError =
+      Obj.getSectionFileOffsetToRawData(XCOFF::STYP_LOADER);
+  if (!LoaderSectionAddrOrError) {
+    reportUniqueWarning(LoaderSectionAddrOrError.takeError());
+    return;
+  }
+  uintptr_t LoaderSectionAddr = LoaderSectionAddrOrError.get();
+
+  if (LoaderSectionAddr == 0)
+    return;
+
+  auto PrintLoadSecHeaderCommon = [&](const auto *LDHeader) {
+    printNumber("Version:", LDHeader->Version);
+    printNumber("NumberOfSymbolEntries:", LDHeader->NumberOfSymTabEnt);
+    printNumber("NumberOfRelocationEntries:", LDHeader->NumberOfRelTabEnt);
+    printNumber("LengthOfImportFileIDStringTable:",
+                LDHeader->LengthOfImpidStrTbl);
+    printNumber("NumberOfImportFileIDs:", LDHeader->NumberOfImpid);
+    printHex("OffsetToImportFileIDs:", LDHeader->OffsetToImpid);
+    printNumber("LengthOfStringTable:", LDHeader->LengthOfStrTbl);
+    printHex("OffsetToStringTable:", LDHeader->OffsetToStrTbl);
+  };
+
+  Width = 35;
+  outs() << "\n---Loader Section Header:\n";
+  if (Obj.is64Bit()) {
+    const LoaderSectionHeader64 *LoaderSec64 =
+        reinterpret_cast<const LoaderSectionHeader64 *>(LoaderSectionAddr);
+    PrintLoadSecHeaderCommon(LoaderSec64);
+    printHex("OffsetToSymbolTable", LoaderSec64->OffsetToSymTbl);
+    printHex("OffsetToRelocationEntries", LoaderSec64->OffsetToRelEnt);
+  } else {
+    const LoaderSectionHeader32 *LoaderSec32 =
+        reinterpret_cast<const LoaderSectionHeader32 *>(LoaderSectionAddr);
+    PrintLoadSecHeaderCommon(LoaderSec32);
+  }
+}
+
 void XCOFFDumper::printFileHeader() {
   Width = 20;
   outs() << "\n---File Header:\n";

@diggerlin diggerlin changed the title [llvm-objdump] Print out xcoff load header for xcoff object file with option private-headers [llvm-objdump] Print out xcoff load of xcoff object file with option private-headers Dec 27, 2024
@diggerlin
Copy link
Contributor Author

the test contest of llvm/test/tools/llvm-objdump/XCOFF/private-headers-option.test
come from https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-readobj/XCOFF/loader-section-header.test

@diggerlin diggerlin changed the title [llvm-objdump] Print out xcoff load of xcoff object file with option private-headers [llvm-objdump] Print out xcoff load section of xcoff object file with option private-headers Dec 27, 2024
llvm/tools/llvm-objdump/XCOFFDump.cpp Show resolved Hide resolved
outs() << "\n---Loader Section Header:\n";
if (Obj.is64Bit()) {
const LoaderSectionHeader64 *LoaderSec64 =
reinterpret_cast<const LoaderSectionHeader64 *>(LoaderSectionAddr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a risk that the data won't be aligned in memory, such that on some architectures, reading the data will cause a failure?

Copy link
Contributor Author

@diggerlin diggerlin Jan 14, 2025

Choose a reason for hiding this comment

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

If I understand your question correctly. I wrote following test case and run , it do not crash. it looks the PowerPC do not need to be alignment on the memory in when reading the data.

bash> cat test.cpp
unsigned int var =10;
int main() {
 char *cvar = reinterpret_cast<char *>(&var)+1;

 int *us = reinterpret_cast<int *>(cvar);
 (*us)++;
 return *us;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think you understood the question correctly, but you've missed the fact that this code could be run on non-PowerPC architectures, which is where my concern lies. See issues like #95811 for examples.

In this case, perhaps we should check that LoaderSectionAddr is appropriately aligned and emit an error if it isn't. Alternatively, we can simply do a memcpy of the raw data into a local copy of LoaderSectionHeader[64|32], so that the data is guaranteed to be aligned. I think either approach is valid, since it's probably a requirement (either explicitly, or indirectly implied by other aspects of the spec) of the XCOFF spec that LoaderSectionAddr is aligned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it only happen when compile with option -mstrict-align on the AArch64, I do not think llvm compile the llvm-objdump with option -mstrict-align.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've seen issues fixed in LLVM because of alignment problems, flagged by build bots. The issue I filed was just one possible example. Another example: 8fbe1e7.

Copy link
Contributor Author

@diggerlin diggerlin Jan 16, 2025

Choose a reason for hiding this comment

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

There is a lot of reinterpret_cast or static_cast in the XCOFFObjectFile.cpp/Archive.cpp etc. it will have the same problem if there is.
I think if we want to fix the problem we need to have underlying API to deal with the problem for whole the llvm in a separate patch instead of using memcpy one by one.

@diggerlin diggerlin requested a review from jh7370 January 14, 2025 18:24
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.

3 participants