[Object][COFF][llvm-readobj] Add support for ARM64X dynamic relocations. #97229
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-binary-utilities @llvm/pr-subscribers-objectyaml Author: Jacek Caban (cjacek) ChangesThis PR implements support for parsing and exposing ARM64X relocations in COFFObjectFile and uses it in llvm-readobj to dump them. ARM64X relocations is what makes hybrid executables possible, more information about it can be found here. On top of that, interpreting those relocations is quite straightforward, I plan to submit support for inspecting ARM64X binaries with llvm-readobj next, see cjacek@3c1baf4 for the draft. This will be useful to be able to write LLD tests (and also headers will be generally useful for the implementation). I hope it will also make potential LLDB support easier, where we currently treat ARM64X binaries as ARM64, while we should use the hybrid view when debugging x86_64 process (see https://reviews.llvm.org/D156268). Patch is 85.42 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97229.diff 7 Files Affected:
diff --git a/llvm/include/llvm/BinaryFormat/COFF.h b/llvm/include/llvm/BinaryFormat/COFF.h
index c8642864af63b..3fc543f73c49d 100644
--- a/llvm/include/llvm/BinaryFormat/COFF.h
+++ b/llvm/include/llvm/BinaryFormat/COFF.h
@@ -417,6 +417,21 @@ enum RelocationTypesARM64 : unsigned {
IMAGE_REL_ARM64_REL32 = 0x0011,
};
+enum DynamicRelocationType : unsigned {
+ IMAGE_DYNAMIC_RELOCATION_GUARD_RF_PROLOGUE = 1,
+ IMAGE_DYNAMIC_RELOCATION_GUARD_RF_EPILOGUE = 2,
+ IMAGE_DYNAMIC_RELOCATION_GUARD_IMPORT_CONTROL_TRANSFER = 3,
+ IMAGE_DYNAMIC_RELOCATION_GUARD_INDIR_CONTROL_TRANSFER = 4,
+ IMAGE_DYNAMIC_RELOCATION_GUARD_SWITCHTABLE_BRANCH = 5,
+ IMAGE_DYNAMIC_RELOCATION_ARM64X = 6,
+};
+
+enum Arm64XFixupType : uint8_t {
+ IMAGE_DVRT_ARM64X_FIXUP_TYPE_ZEROFILL = 0,
+ IMAGE_DVRT_ARM64X_FIXUP_TYPE_VALUE = 1,
+ IMAGE_DVRT_ARM64X_FIXUP_TYPE_DELTA = 2,
+};
+
enum COMDATType : uint8_t {
IMAGE_COMDAT_SELECT_NODUPLICATES = 1,
IMAGE_COMDAT_SELECT_ANY,
diff --git a/llvm/include/llvm/Object/COFF.h b/llvm/include/llvm/Object/COFF.h
index a548b2c15c5fd..16f0d469d9aa7 100644
--- a/llvm/include/llvm/Object/COFF.h
+++ b/llvm/include/llvm/Object/COFF.h
@@ -35,8 +35,10 @@ template <typename T> class ArrayRef;
namespace object {
+class Arm64XRelocRef;
class BaseRelocRef;
class DelayImportDirectoryEntryRef;
+class DynamicRelocRef;
class ExportDirectoryEntryRef;
class ImportDirectoryEntryRef;
class ImportedSymbolRef;
@@ -48,6 +50,8 @@ using delay_import_directory_iterator =
using export_directory_iterator = content_iterator<ExportDirectoryEntryRef>;
using imported_symbol_iterator = content_iterator<ImportedSymbolRef>;
using base_reloc_iterator = content_iterator<BaseRelocRef>;
+using dynamic_reloc_iterator = content_iterator<DynamicRelocRef>;
+using arm64x_reloc_iterator = content_iterator<Arm64XRelocRef>;
/// The DOS compatible header at the front of all PE/COFF executables.
struct dos_header {
@@ -832,6 +836,37 @@ struct debug_h_header {
support::ulittle16_t HashAlgorithm;
};
+struct coff_dynamic_reloc_table {
+ support::ulittle32_t Version;
+ support::ulittle32_t Size;
+};
+
+struct coff_dynamic_relocation32 {
+ support::ulittle32_t Symbol;
+ support::ulittle32_t BaseRelocSize;
+};
+
+struct coff_dynamic_relocation64 {
+ support::ulittle64_t Symbol;
+ support::ulittle32_t BaseRelocSize;
+};
+
+struct coff_dynamic_relocation32_v2 {
+ support::ulittle32_t HeaderSize;
+ support::ulittle32_t FixupInfoSize;
+ support::ulittle32_t Symbol;
+ support::ulittle32_t SymbolGroup;
+ support::ulittle32_t Flags;
+};
+
+struct coff_dynamic_relocation64_v2 {
+ support::ulittle32_t HeaderSize;
+ support::ulittle32_t FixupInfoSize;
+ support::ulittle64_t Symbol;
+ support::ulittle32_t SymbolGroup;
+ support::ulittle32_t Flags;
+};
+
class COFFObjectFile : public ObjectFile {
private:
COFFObjectFile(MemoryBufferRef Object);
@@ -861,6 +896,8 @@ class COFFObjectFile : public ObjectFile {
// Either coff_load_configuration32 or coff_load_configuration64.
const void *LoadConfig = nullptr;
const chpe_metadata *CHPEMetadata = nullptr;
+ const coff_dynamic_reloc_table *DynamicRelocTable = nullptr;
+ std::shared_ptr<WritableMemoryBuffer> HybridBufferRef;
Expected<StringRef> getString(uint32_t offset) const;
@@ -986,6 +1023,9 @@ class COFFObjectFile : public ObjectFile {
}
const chpe_metadata *getCHPEMetadata() const { return CHPEMetadata; }
+ const coff_dynamic_reloc_table *getDynamicRelocTable() const {
+ return DynamicRelocTable;
+ }
StringRef getRelocationTypeName(uint16_t Type) const;
@@ -1054,6 +1094,8 @@ class COFFObjectFile : public ObjectFile {
export_directory_iterator export_directory_end() const;
base_reloc_iterator base_reloc_begin() const;
base_reloc_iterator base_reloc_end() const;
+ dynamic_reloc_iterator dynamic_reloc_begin() const;
+ dynamic_reloc_iterator dynamic_reloc_end() const;
const debug_directory *debug_directory_begin() const {
return DebugDirectoryBegin;
}
@@ -1066,6 +1108,7 @@ class COFFObjectFile : public ObjectFile {
delay_import_directories() const;
iterator_range<export_directory_iterator> export_directories() const;
iterator_range<base_reloc_iterator> base_relocs() const;
+ iterator_range<dynamic_reloc_iterator> dynamic_relocs() const;
iterator_range<const debug_directory *> debug_directories() const {
return make_range(debug_directory_begin(), debug_directory_end());
}
@@ -1295,6 +1338,54 @@ class BaseRelocRef {
uint32_t Index;
};
+class DynamicRelocRef {
+public:
+ DynamicRelocRef() = default;
+ DynamicRelocRef(const void *Header, const COFFObjectFile *Owner)
+ : Obj(Owner), Header(reinterpret_cast<const uint8_t *>(Header)) {}
+
+ bool operator==(const DynamicRelocRef &Other) const;
+ void moveNext();
+ uint32_t getType() const;
+ void getContents(ArrayRef<uint8_t> &Ref) const;
+
+ arm64x_reloc_iterator arm64x_reloc_begin() const;
+ arm64x_reloc_iterator arm64x_reloc_end() const;
+ iterator_range<arm64x_reloc_iterator> arm64x_relocs() const;
+
+private:
+ const COFFObjectFile *Obj;
+ const uint8_t *Header;
+};
+
+class Arm64XRelocRef {
+public:
+ Arm64XRelocRef() = default;
+ Arm64XRelocRef(const coff_base_reloc_block_header *Header, uint32_t Index = 0)
+ : Header(Header), Index(Index) {}
+
+ bool operator==(const Arm64XRelocRef &Other) const;
+ void moveNext();
+
+ COFF::Arm64XFixupType getType() const {
+ return COFF::Arm64XFixupType((getReloc() >> 12) & 3);
+ }
+ uint32_t getRVA() const { return Header->PageRVA + (getReloc() & 0xfff); }
+ uint8_t getSize() const;
+ uint64_t getValue() const;
+
+ const coff_base_reloc_block_header *Header;
+
+private:
+ uint32_t Index;
+
+ const support::ulittle16_t &getReloc() const {
+ return reinterpret_cast<const support::ulittle16_t *>(Header + 1)[Index];
+ }
+
+ uint16_t getArg() const { return getReloc() >> 14; }
+};
+
class ResourceSectionRef {
public:
ResourceSectionRef() = default;
diff --git a/llvm/lib/Object/COFFObjectFile.cpp b/llvm/lib/Object/COFFObjectFile.cpp
index 5a85b8e00c633..cee11988ebdf6 100644
--- a/llvm/lib/Object/COFFObjectFile.cpp
+++ b/llvm/lib/Object/COFFObjectFile.cpp
@@ -800,6 +800,218 @@ Error COFFObjectFile::initLoadConfigPtr() {
}
}
+ // Interpret and validate dynamic relocations.
+ uint32_t DynamicRelocTableOffset = 0, DynamicRelocTableSection = 0;
+ if (is64()) {
+ auto Config = getLoadConfig64();
+ if (Config->Size >=
+ offsetof(coff_load_configuration64, DynamicValueRelocTableSection) +
+ sizeof(Config->DynamicValueRelocTableSection)) {
+ DynamicRelocTableSection = Config->DynamicValueRelocTableSection;
+ DynamicRelocTableOffset = Config->DynamicValueRelocTableOffset;
+ }
+ } else {
+ auto Config = getLoadConfig32();
+ if (Config->Size >=
+ offsetof(coff_load_configuration32, DynamicValueRelocTableSection) +
+ sizeof(Config->DynamicValueRelocTableSection)) {
+ DynamicRelocTableSection = Config->DynamicValueRelocTableSection;
+ DynamicRelocTableOffset = Config->DynamicValueRelocTableOffset;
+ }
+ }
+
+ Expected<const coff_section *> Section = getSection(DynamicRelocTableSection);
+ if (!Section)
+ return Section.takeError();
+ if (*Section) {
+ ArrayRef<uint8_t> Contents;
+ if (Error E = getSectionContents(*Section, Contents))
+ return E;
+
+ Contents = Contents.drop_front(DynamicRelocTableOffset);
+ if (Contents.size() < sizeof(coff_dynamic_reloc_table))
+ return createStringError(object_error::parse_failed,
+ "Too large DynamicValueRelocTableOffset (" +
+ Twine(DynamicRelocTableOffset) + ")");
+
+ DynamicRelocTable =
+ reinterpret_cast<const coff_dynamic_reloc_table *>(Contents.data());
+
+ if (DynamicRelocTable->Version != 1 && DynamicRelocTable->Version != 2) {
+ DynamicRelocTable = nullptr;
+ return Error::success();
+ }
+
+ Contents = Contents.drop_front(sizeof(*DynamicRelocTable));
+ if (DynamicRelocTable->Size > Contents.size())
+ return createStringError(object_error::parse_failed,
+ "Indvalid dynamic relocations directory size (" +
+ Twine(DynamicRelocTable->Size) + ")");
+ Contents = Contents.take_front(DynamicRelocTable->Size);
+
+ while (!Contents.empty()) {
+ uint32_t DynRelocSize;
+ uint64_t Symbol;
+
+ if (DynamicRelocTable->Version == 1) {
+ if (is64()) {
+ if (Contents.size() < sizeof(coff_dynamic_relocation64))
+ return createStringError(
+ object_error::parse_failed,
+ "Unexpected end of dynamic relocations data");
+
+ auto DynReloc = reinterpret_cast<const coff_dynamic_relocation64 *>(
+ Contents.data());
+ Symbol = DynReloc->Symbol;
+ DynRelocSize = DynReloc->BaseRelocSize;
+ Contents = Contents.drop_front(sizeof(*DynReloc));
+ } else {
+ if (Contents.size() < sizeof(coff_dynamic_relocation32))
+ return createStringError(
+ object_error::parse_failed,
+ "Unexpected end of dynamic relocations data");
+
+ auto DynReloc = reinterpret_cast<const coff_dynamic_relocation32 *>(
+ Contents.data());
+ Symbol = DynReloc->Symbol;
+ DynRelocSize = DynReloc->BaseRelocSize;
+ Contents = Contents.drop_front(sizeof(*DynReloc));
+ }
+ } else {
+ if (is64()) {
+ if (Contents.size() < sizeof(coff_dynamic_relocation64_v2))
+ return createStringError(
+ object_error::parse_failed,
+ "Unexpected end of dynamic relocations data");
+
+ auto DynReloc =
+ reinterpret_cast<const coff_dynamic_relocation64_v2 *>(
+ Contents.data());
+ if (DynReloc->HeaderSize < sizeof(*DynReloc) ||
+ Contents.size() < DynReloc->HeaderSize)
+ return createStringError(
+ object_error::parse_failed,
+ "Invalid dynamic relocation header size (" +
+ Twine(DynReloc->HeaderSize) + ")");
+
+ Symbol = DynReloc->Symbol;
+ DynRelocSize = DynReloc->FixupInfoSize;
+ Contents = Contents.drop_front(DynReloc->HeaderSize);
+ } else {
+ if (Contents.size() < sizeof(coff_dynamic_relocation32_v2))
+ return createStringError(
+ object_error::parse_failed,
+ "Unexpected end of dynamic relocations data");
+
+ auto DynReloc =
+ reinterpret_cast<const coff_dynamic_relocation32_v2 *>(
+ Contents.data());
+ if (DynReloc->HeaderSize < sizeof(*DynReloc) ||
+ Contents.size() < DynReloc->HeaderSize)
+ return createStringError(
+ object_error::parse_failed,
+ "Invalid dynamic relocation header size (" +
+ Twine(DynReloc->HeaderSize) + ")");
+
+ Symbol = DynReloc->Symbol;
+ DynRelocSize = DynReloc->FixupInfoSize;
+ Contents = Contents.drop_front(DynReloc->HeaderSize);
+ }
+ }
+ if (DynRelocSize > Contents.size())
+ return createStringError(object_error::parse_failed,
+ "Too large dynamic relocation size (" +
+ Twine(DynRelocSize) + ")");
+
+ ArrayRef<uint8_t> RelocContents = Contents.take_front(DynRelocSize);
+ Contents = Contents.drop_front(DynRelocSize);
+
+ switch (Symbol) {
+ case COFF::IMAGE_DYNAMIC_RELOCATION_ARM64X:
+ while (!RelocContents.empty()) {
+ if (RelocContents.size() < sizeof(coff_base_reloc_block_header))
+ return createStringError(
+ object_error::parse_failed,
+ "Unexpected end of ARM64X relocations data");
+
+ auto Header = reinterpret_cast<const coff_base_reloc_block_header *>(
+ RelocContents.data());
+ if (Header->BlockSize <= sizeof(*Header))
+ return createStringError(object_error::parse_failed,
+ "ARM64X relocations block size (" +
+ Twine(Header->BlockSize) +
+ ") is too small");
+ if (Header->BlockSize % sizeof(uint32_t))
+ return createStringError(
+ object_error::parse_failed,
+ "Unaligned ARM64X relocations block size (" +
+ Twine(Header->BlockSize) + ")");
+ if (Header->BlockSize > RelocContents.size())
+ return createStringError(object_error::parse_failed,
+ "ARM64X relocations block size (" +
+ Twine(Header->BlockSize) +
+ ") is too large");
+ if (Header->PageRVA & 0xfff)
+ return createStringError(object_error::parse_failed,
+ "Unaligned ARM64X relocations page RVA (" +
+ Twine(Header->PageRVA) + ")");
+
+ ArrayRef<uint16_t> Relocs(
+ reinterpret_cast<const uint16_t *>(RelocContents.data() +
+ sizeof(*Header)),
+ (Header->BlockSize - sizeof(*Header)) / sizeof(uint16_t));
+ RelocContents = RelocContents.drop_front(Header->BlockSize);
+
+ while (!Relocs.empty()) {
+ if (!Relocs[0]) {
+ if (Relocs.size() != 1)
+ return createStringError(
+ object_error::parse_failed,
+ "Unexpected ARM64X relocations terminator");
+ break;
+ }
+
+ uint16_t Arg = Relocs[0] >> 14, RelocSize = 1, Size;
+ switch ((Relocs[0] >> 12) & 3) {
+ case COFF::IMAGE_DVRT_ARM64X_FIXUP_TYPE_ZEROFILL:
+ Size = 1 << Arg;
+ break;
+ case COFF::IMAGE_DVRT_ARM64X_FIXUP_TYPE_VALUE:
+ if (!Arg)
+ return createStringError(
+ object_error::parse_failed,
+ "Invalid ARM64X relocation value size (0)");
+ Size = 1 << Arg;
+ RelocSize += Size / sizeof(Relocs[0]);
+ break;
+ case COFF::IMAGE_DVRT_ARM64X_FIXUP_TYPE_DELTA:
+ ++RelocSize;
+ Size = sizeof(uint32_t);
+ break;
+ default:
+ return createStringError(object_error::parse_failed,
+ "Invalid relocation type");
+ }
+ if (Header->PageRVA) {
+ uint64_t IntPtr;
+ uint16_t Offset = Relocs[0] & 0xfff;
+ if (Offset % Size)
+ return createStringError(object_error::parse_failed,
+ "Unaligned ARM64X relocation RVA (" +
+ Twine(Header->PageRVA + Offset) +
+ ")");
+ if (Error E = getRvaPtr(Header->PageRVA + Offset + Size, IntPtr,
+ "ARM64X reloc"))
+ return E;
+ }
+ Relocs = Relocs.drop_front(RelocSize);
+ }
+ }
+ break;
+ }
+ }
+ }
+
return Error::success();
}
@@ -1047,6 +1259,19 @@ base_reloc_iterator COFFObjectFile::base_reloc_end() const {
return base_reloc_iterator(BaseRelocRef(BaseRelocEnd, this));
}
+dynamic_reloc_iterator COFFObjectFile::dynamic_reloc_begin() const {
+ const void *Header = DynamicRelocTable ? DynamicRelocTable + 1 : nullptr;
+ return dynamic_reloc_iterator(DynamicRelocRef(Header, this));
+}
+
+dynamic_reloc_iterator COFFObjectFile::dynamic_reloc_end() const {
+ const void *Header = nullptr;
+ if (DynamicRelocTable)
+ Header = reinterpret_cast<const uint8_t *>(DynamicRelocTable + 1) +
+ DynamicRelocTable->Size;
+ return dynamic_reloc_iterator(DynamicRelocRef(Header, this));
+}
+
uint8_t COFFObjectFile::getBytesInAddress() const {
return getArch() == Triple::x86_64 || getArch() == Triple::aarch64 ? 8 : 4;
}
@@ -1100,6 +1325,10 @@ iterator_range<base_reloc_iterator> COFFObjectFile::base_relocs() const {
return make_range(base_reloc_begin(), base_reloc_end());
}
+iterator_range<dynamic_reloc_iterator> COFFObjectFile::dynamic_relocs() const {
+ return make_range(dynamic_reloc_begin(), dynamic_reloc_end());
+}
+
const data_directory *COFFObjectFile::getDataDirectory(uint32_t Index) const {
if (!DataDirectory)
return nullptr;
@@ -1789,6 +2018,164 @@ Error BaseRelocRef::getRVA(uint32_t &Result) const {
return Error::success();
}
+bool DynamicRelocRef::operator==(const DynamicRelocRef &Other) const {
+ return Header == Other.Header;
+}
+
+void DynamicRelocRef::moveNext() {
+ switch (Obj->getDynamicRelocTable()->Version) {
+ case 1:
+ if (Obj->is64()) {
+ auto H = reinterpret_cast<const coff_dynamic_relocation64 *>(Header);
+ Header += sizeof(*H) + H->BaseRelocSize;
+ } else {
+ auto H = reinterpret_cast<const coff_dynamic_relocation32 *>(Header);
+ Header += sizeof(*H) + H->BaseRelocSize;
+ }
+ break;
+ case 2:
+ if (Obj->is64()) {
+ auto H = reinterpret_cast<const coff_dynamic_relocation64_v2 *>(Header);
+ Header += H->HeaderSize + H->FixupInfoSize;
+ } else {
+ auto H = reinterpret_cast<const coff_dynamic_relocation32_v2 *>(Header);
+ Header += H->HeaderSize + H->FixupInfoSize;
+ }
+ break;
+ }
+}
+
+uint32_t DynamicRelocRef::getType() const {
+ switch (Obj->getDynamicRelocTable()->Version) {
+ case 1:
+ if (Obj->is64()) {
+ auto H = reinterpret_cast<const coff_dynamic_relocation64 *>(Header);
+ return H->Symbol;
+ } else {
+ auto H = reinterpret_cast<const coff_dynamic_relocation32 *>(Header);
+ return H->Symbol;
+ }
+ break;
+ case 2:
+ if (Obj->is64()) {
+ auto H = reinterpret_cast<const coff_dynamic_relocation64_v2 *>(Header);
+ return H->Symbol;
+ } else {
+ auto H = reinterpret_cast<const coff_dynamic_relocation32_v2 *>(Header);
+ return H->Symbol;
+ }
+ break;
+ default:
+ llvm_unreachable("invalid version");
+ }
+}
+
+void DynamicRelocRef::getContents(ArrayRef<uint8_t> &Ref) const {
+ switch (Obj->getDynamicRelocTable()->Version) {
+ case 1:
+ if (Obj->is64()) {
+ auto H = reinterpret_cast<const coff_dynamic_relocation64 *>(Header);
+ Ref = ArrayRef(Header + sizeof(*H), H->BaseRelocSize);
+ } else {
+ auto H = reinterpret_cast<const coff_dynamic_relocation32 *>(Header);
+ Ref = ArrayRef(Header + sizeof(*H), H->BaseRelocSize);
+ }
+ break;
+ case 2:
+ if (Obj->is64()) {
+ auto H = reinterpret_cast<const coff_dynamic_relocation64_v2 *>(Header);
+ Ref = ArrayRef(Header + H->HeaderSize, H->FixupInfoSize);
+ } else {
+ auto H = reinterpret_cast<const coff_dynamic_relocation32_v2 *>(Header);
+ Ref = ArrayRef(Header + H->HeaderSize, H->FixupInfoSize);
+ }
+ break;
+ }
+}
+
+arm64x_reloc_iterator DynamicRelocRef::arm64x_reloc_begin() const {
+ assert(getType() == COFF::IMAGE_DYNAMIC_RELOCATION_ARM64X);
+ ArrayRef<uint8_t> Content;
+ getContents(Content);
+ auto Header =
+ reinterpret_cast<const coff_base_reloc_block_header *>(Content.begin());
+ return arm64x_reloc_iterator(Arm64XRelocRef(Header));
+}
+
+arm64x_reloc_iterator DynamicRelocRef::arm64x_reloc_end() const {
+ assert(getType() == COFF::IMAGE_DYNAMIC_RELOCATION_ARM64X);
+ ArrayRef<uint8_t> Content;
+ getContents(Content);
+ auto Header =
+ reinterpret_cast<const coff_base_reloc_block_header *>(Content.end());
+ return arm64x_reloc_iterator(Arm64XRelocRef(Header, 0));
+}
+
+iterator_range<arm64x_reloc_iterator> DynamicRelocRef::arm64x_relocs() const {
+ return make_range(arm64x_reloc_begin(), arm64x_reloc_end());
+}
+
+bool Arm64XRelocRef::operator==(const Arm64XRelocRef &Other) const {
+ return Header == Other.Header && Index == Other.Index;
+}
+
+void Arm64XRelocRef::moveNext() {
+ switch (getType()) {
+ case COFF::IMAGE_DVRT_ARM64X_FIXUP_TYPE_VALUE:
+ Index += (1u << ge...
[truncated]
|
|
Sorry, going to have to bow out of this one: I can help with ARM64EC, but not ARM64X. |
llvm/lib/Object/COFFObjectFile.cpp
Outdated
|
|
||
| if (DynamicRelocTable->Version != 1 && DynamicRelocTable->Version != 2) { | ||
| DynamicRelocTable = nullptr; | ||
| return Error::success(); |
There was a problem hiding this comment.
Why is this not an error?
There was a problem hiding this comment.
I meant it for forward-compatibility. If there is a newer version in the future, we won't be able to read it, but the object could be otherwise still useful.
There was a problem hiding this comment.
For objdump, I can see an argument for continuing to dump (ideally with a warning), but other uses of object files likely want to bail out of they can't parse an important part of the object file. And the API doesn't really provide any way to tell what happened.
There was a problem hiding this comment.
I pushed a version that makes it an error, thanks.
There was a problem hiding this comment.
FWIW, this distinction between verifying all aspects, or just reading as much as possible with as little judgement as possible, is indeed quite frustrating at times.
If you have an object file that our tools reject, our tools also are quite unfriendly in helping you figure out what really is wrong. (I remember doing a WIP/throwaway branch sometime to make all the COFFObjectFile errors much more verbose, and to continue parsing as far as possible, to let me analyze some file.)
mstorsjo
reviewed
Jul 28, 2024
llvm/lib/Object/COFFObjectFile.cpp
Outdated
| Twine(DynamicRelocTable->Size) + ")"); | ||
| Contents = Contents.take_front(DynamicRelocTable->Size); | ||
|
|
||
| while (!Contents.empty()) { |
There was a problem hiding this comment.
This whole parsing o the dynamic reloc table feels like quite a lot of code, and it inflates the enclosing function, initLoadConfigPtr, quite significantly. I think this would be more readable if split out to a separate function for parsing the dynamic reloc table.
After seeing that some amount of the parsing code here is unused, I'm not quite convinced that this needs to be split, but if the dynamic reloc table handling stuff here is more than maybe 30-50 lines, I'd still favour splitting to a separate function.
llvm/include/llvm/Object/COFF.h
Outdated
| @@ -861,6 +896,8 @@ class COFFObjectFile : public ObjectFile { | |||
| // Either coff_load_configuration32 or coff_load_configuration64. | |||
| const void *LoadConfig = nullptr; | |||
| const chpe_metadata *CHPEMetadata = nullptr; | |||
| const coff_dynamic_reloc_table *DynamicRelocTable = nullptr; | |||
| std::shared_ptr<WritableMemoryBuffer> HybridBufferRef; | |||
There was a problem hiding this comment.
This seems like a leftover? A std::shared_ptr in these classes feels like a very unusual thing (we don't have any occurrances of that before), so that would require a bit of motivation - but it seems to be unused in this patch still?
There was a problem hiding this comment.
Yes, it's a leftover from experiments that I dropped, sorry about that.
mstorsjo
approved these changes
Aug 4, 2024
There was a problem hiding this comment.
LGTM, thank you!
I'd still wait a day or two, in case @efriedma-quic has further comments on it, but other than that, this looks good to me now!
|
Merged, thanks for the review! |
cjacek
added a commit
that referenced
this pull request
Aug 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements support for parsing and exposing ARM64X relocations in COFFObjectFile and uses it in llvm-readobj to dump them. ARM64X relocations is what makes hybrid executables possible, more information about it can be found here. On top of that, interpreting those relocations is quite straightforward, I plan to submit support for inspecting ARM64X binaries with llvm-readobj next, see cjacek@3c1baf4 for the draft.
This will be useful to be able to write LLD tests (and also headers will be generally useful for the implementation). I hope it will also make potential LLDB support easier, where we currently treat ARM64X binaries as ARM64, while we should use the hybrid view when debugging x86_64 process (see https://reviews.llvm.org/D156268).