Skip to content

[ELFDumper] Always read AMD Code Object notes as little-endian #70775

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

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

Pierre-vh
Copy link
Contributor

Avoids issues on big-endian hosts.

Note that we use aligned types because primitive integers are also aligned. If we don't use aligned types, HSAILProperties ends up being 11 bytes instead of 12 (1 byte padding at the end of the struct added by the compiler).
Technically only the first type needs to be aligned, but I just used aligned types everywhere to be consistent.

Fixes #65280

@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2023

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

Author: Pierre van Houtryve (Pierre-vh)

Changes

Avoids issues on big-endian hosts.

Note that we use aligned types because primitive integers are also aligned. If we don't use aligned types, HSAILProperties ends up being 11 bytes instead of 12 (1 byte padding at the end of the struct added by the compiler).
Technically only the first type needs to be aligned, but I just used aligned types everywhere to be consistent.

Fixes #65280


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

1 Files Affected:

  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+11-11)
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 658c651add3b922..34e31e05cd8527f 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -5451,8 +5451,8 @@ static AMDNote getAMDNote(uint32_t NoteType, ArrayRef<uint8_t> Desc) {
     return {"", ""};
   case ELF::NT_AMD_HSA_CODE_OBJECT_VERSION: {
     struct CodeObjectVersion {
-      uint32_t MajorVersion;
-      uint32_t MinorVersion;
+      support::aligned_ulittle32_t MajorVersion;
+      support::aligned_ulittle32_t MinorVersion;
     };
     if (Desc.size() != sizeof(CodeObjectVersion))
       return {"AMD HSA Code Object Version",
@@ -5466,8 +5466,8 @@ static AMDNote getAMDNote(uint32_t NoteType, ArrayRef<uint8_t> Desc) {
   }
   case ELF::NT_AMD_HSA_HSAIL: {
     struct HSAILProperties {
-      uint32_t HSAILMajorVersion;
-      uint32_t HSAILMinorVersion;
+      support::aligned_ulittle32_t HSAILMajorVersion;
+      support::aligned_ulittle32_t HSAILMinorVersion;
       uint8_t Profile;
       uint8_t MachineModel;
       uint8_t DefaultFloatRound;
@@ -5487,11 +5487,11 @@ static AMDNote getAMDNote(uint32_t NoteType, ArrayRef<uint8_t> Desc) {
   }
   case ELF::NT_AMD_HSA_ISA_VERSION: {
     struct IsaVersion {
-      uint16_t VendorNameSize;
-      uint16_t ArchitectureNameSize;
-      uint32_t Major;
-      uint32_t Minor;
-      uint32_t Stepping;
+      support::aligned_ulittle16_t VendorNameSize;
+      support::aligned_ulittle16_t ArchitectureNameSize;
+      support::aligned_ulittle32_t Major;
+      support::aligned_ulittle32_t Minor;
+      support::aligned_ulittle32_t Stepping;
     };
     if (Desc.size() < sizeof(IsaVersion))
       return {"AMD HSA ISA Version", "Invalid AMD HSA ISA Version"};
@@ -5527,8 +5527,8 @@ static AMDNote getAMDNote(uint32_t NoteType, ArrayRef<uint8_t> Desc) {
   }
   case ELF::NT_AMD_PAL_METADATA: {
     struct PALMetadata {
-      uint32_t Key;
-      uint32_t Value;
+      support::aligned_ulittle32_t Key;
+      support::aligned_ulittle32_t Value;
     };
     if (Desc.size() % sizeof(PALMetadata) != 0)
       return {"AMD PAL Metadata", "Invalid AMD PAL Metadata"};

@Pierre-vh Pierre-vh merged commit d3876c5 into llvm:main Nov 14, 2023
@Pierre-vh Pierre-vh deleted the fix-bigendian-getAMDNote branch November 14, 2023 06:26
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…70775)

Should avoid issues on big-endian hosts.

Note that we use aligned types because primitive integers are also
aligned. If we don't use aligned types, `HSAILProperties` ends up being
11 bytes instead of 12 (1 byte padding at the end of the struct added by
the compiler).
Technically only the first type needs to be aligned, but I just used
aligned types everywhere to be consistent.

Fixes llvm#65280
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.

llvm-readobj reads incorrect values from AMD note property on big-endian hosts
4 participants