[LLD][COFF] Use EC load config for ARM64X relocations of load config directory #121337

cjacek · 2024-12-30T13:58:04Z

This change ensures the load config in the hybrid image view is handled correctly. It introduces a new Arm64XRelocVal class to abstract relocation values, allowing them to be relative to a symbol. This class will also be useful for managing ARM64X relocation offsets in the future.

llvmbot · 2024-12-30T13:58:37Z

@llvm/pr-subscribers-lld-coff
@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld

Author: Jacek Caban (cjacek)

Changes

This change ensures the load config in the hybrid image view is handled correctly. It introduces a new Arm64XRelocVal class to abstract relocation values, allowing them to be relative to a symbol. This class will also be useful for managing ARM64X relocation offsets in the future.

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

4 Files Affected:

(modified) lld/COFF/Chunks.cpp (+7-3)
(modified) lld/COFF/Chunks.h (+15-3)
(modified) lld/COFF/Writer.cpp (+3-4)
(modified) lld/test/COFF/arm64x-loadconfig.s (+33-4)

diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index ec0fdf0b67b38b..cac581ec0dbbe2 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -1150,6 +1150,10 @@ uint32_t ImportThunkChunkARM64EC::extendRanges() {
   return sizeof(arm64Thunk) - sizeof(uint32_t);
 }
 
+uint64_t Arm64XRelocVal::get() const {
+  return (sym ? sym->getRVA() : 0) + value;
+}
+
 size_t Arm64XDynamicRelocEntry::getSize() const {
   switch (type) {
   case IMAGE_DVRT_ARM64X_FIXUP_TYPE_VALUE:
@@ -1169,13 +1173,13 @@ void Arm64XDynamicRelocEntry::writeTo(uint8_t *buf) const {
     *out |= ((bit_width(size) - 1) << 14); // Encode the size.
     switch (size) {
     case 2:
-      out[1] = value;
+      out[1] = value.get();
       break;
     case 4:
-      *reinterpret_cast<ulittle32_t *>(out + 1) = value;
+      *reinterpret_cast<ulittle32_t *>(out + 1) = value.get();
       break;
     case 8:
-      *reinterpret_cast<ulittle64_t *>(out + 1) = value;
+      *reinterpret_cast<ulittle64_t *>(out + 1) = value.get();
       break;
     default:
       llvm_unreachable("invalid size");
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
index 0d2b2ac0f15ea9..6adc751c214ab7 100644
--- a/lld/COFF/Chunks.h
+++ b/lld/COFF/Chunks.h
@@ -835,18 +835,30 @@ class ECExportThunkChunk : public NonSectionCodeChunk {
   Defined *target;
 };
 
+// ARM64X relocation value, potentially relative to a symbol.
+class Arm64XRelocVal {
+public:
+  Arm64XRelocVal(int64_t value = 0) : value(value) {}
+  Arm64XRelocVal(Defined *sym, int32_t offset = 0) : sym(sym), value(offset) {}
+  uint64_t get() const;
+
+private:
+  Defined *sym = nullptr;
+  uint64_t value;
+};
+
 // ARM64X entry for dynamic relocations.
 class Arm64XDynamicRelocEntry {
 public:
   Arm64XDynamicRelocEntry(llvm::COFF::Arm64XFixupType type, uint8_t size,
-                          uint32_t offset, uint64_t value)
+                          uint32_t offset, Arm64XRelocVal value)
       : offset(offset), value(value), type(type), size(size) {}
 
   size_t getSize() const;
   void writeTo(uint8_t *buf) const;
 
   uint32_t offset;
-  uint64_t value;
+  Arm64XRelocVal value;
 
 private:
   llvm::COFF::Arm64XFixupType type;
@@ -862,7 +874,7 @@ class DynamicRelocsChunk : public NonSectionChunk {
   void finalize();
 
   void add(llvm::COFF::Arm64XFixupType type, uint8_t size, uint32_t offset,
-           uint64_t value) {
+           Arm64XRelocVal value) {
     arm64xRelocs.emplace_back(type, size, offset, value);
   }
 
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index b3dd5f6cf49265..156f051901c892 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -2582,18 +2582,17 @@ void Writer::createDynamicRelocs() {
                          coffHeaderOffset + offsetof(coff_file_header, Machine),
                          AMD64);
 
-  // Clear the load config directory.
-  // FIXME: Use the hybrid load config value instead.
+  // Set the hybrid load config to the EC load config.
   ctx.dynamicRelocs->add(IMAGE_DVRT_ARM64X_FIXUP_TYPE_VALUE, sizeof(uint32_t),
                          dataDirOffset64 +
                              LOAD_CONFIG_TABLE * sizeof(data_directory) +
                              offsetof(data_directory, RelativeVirtualAddress),
-                         0);
+                         ctx.hybridSymtab->loadConfigSym);
   ctx.dynamicRelocs->add(IMAGE_DVRT_ARM64X_FIXUP_TYPE_VALUE, sizeof(uint32_t),
                          dataDirOffset64 +
                              LOAD_CONFIG_TABLE * sizeof(data_directory) +
                              offsetof(data_directory, Size),
-                         0);
+                         ctx.hybridSymtab->loadConfigSize);
 }
 
 PartialSection *Writer::createPartialSection(StringRef name,
diff --git a/lld/test/COFF/arm64x-loadconfig.s b/lld/test/COFF/arm64x-loadconfig.s
index 8d2ab555546341..2b550864b77f15 100644
--- a/lld/test/COFF/arm64x-loadconfig.s
+++ b/lld/test/COFF/arm64x-loadconfig.s
@@ -87,20 +87,49 @@
 // LOADCFG-NEXT:       RVA: 0x150
 // LOADCFG-NEXT:       Type: VALUE
 // LOADCFG-NEXT:       Size: 0x4
-// LOADCFG-NEXT:       Value: 0x0
+// LOADCFG-NEXT:       Value: 0x1140
 // LOADCFG-NEXT:     ]
 // LOADCFG-NEXT:     Entry [
 // LOADCFG-NEXT:       RVA: 0x154
 // LOADCFG-NEXT:       Type: VALUE
 // LOADCFG-NEXT:       Size: 0x4
-// LOADCFG-NEXT:       Value: 0x0
+// LOADCFG-NEXT:       Value: 0x140
 // LOADCFG-NEXT:     ]
 // LOADCFG-NEXT:   ]
 // LOADCFG-NEXT: ]
 // LOADCFG-NEXT: HybridObject {
-// LOADCFG-NEXT:   Format: COFF-x86-64
-// LOADCFG-NEXT:   Arch: x86_64
+// LOADCFG-NEXT:   Format: COFF-ARM64EC
+// LOADCFG-NEXT:   Arch: aarch64
 // LOADCFG-NEXT:   AddressSize: 64bit
+// LOADCFG-NEXT:   LoadConfig [
+// LOADCFG-NEXT:     Size:   0x140
+// LOADCFG:        CHPEMetadata [
+// LOADCFG-NEXT:     Version:   0x2
+// LOADCFG:        ]
+// LOADCFG-NEXT:   DynamicRelocations [
+// LOADCFG-NEXT:     Version: 0x1
+// LOADCFG-NEXT:     Arm64X [
+// LOADCFG-NEXT:       Entry [
+// LOADCFG-NEXT:         RVA: 0x7C
+// LOADCFG-NEXT:         Type: VALUE
+// LOADCFG-NEXT:         Size: 0x2
+// LOADCFG-NEXT:         Value: 0x8664
+// LOADCFG-NEXT:       ]
+// LOADCFG-NEXT:       Entry [
+// LOADCFG-NEXT:         RVA: 0x150
+// LOADCFG-NEXT:         Type: VALUE
+// LOADCFG-NEXT:         Size: 0x4
+// LOADCFG-NEXT:         Value: 0x1140
+// LOADCFG-NEXT:       ]
+// LOADCFG-NEXT:       Entry [
+// LOADCFG-NEXT:         RVA: 0x154
+// LOADCFG-NEXT:         Type: VALUE
+// LOADCFG-NEXT:         Size: 0x4
+// LOADCFG-NEXT:         Value: 0x140
+// LOADCFG-NEXT:       ]
+// LOADCFG-NEXT:     ]
+// LOADCFG-NEXT:   ]
+// LOADCFG-NEXT: }
 
 // RUN: llvm-readobj --coff-basereloc out-hyb.dll | FileCheck --check-prefix=BASERELOC %s
 // BASERELOC:      BaseReloc [

cjacek · 2025-01-09T23:22:01Z

Rebased.

mstorsjo

LGTM, modulo the int64_t vs uint64_t detail.

(Sorry for losing track of this PR - feel free to ping me sooner if it looks like I've missed some PR.)

mstorsjo · 2025-01-10T08:41:04Z

lld/COFF/Chunks.h

+// ARM64X relocation value, potentially relative to a symbol.
+class Arm64XRelocVal {
+public:
+  Arm64XRelocVal(int64_t value = 0) : value(value) {}


This constructor takes an int64_t, while the value field actually is an uint64_t. Is this intended?

I changed argument type to uint64_t. Negative values are possible to delta values, but the cast may just as well happen on the call. Thanks!

…directory This change ensures the load config in the hybrid image view is handled correctly. It introduces a new Arm64XRelocVal class to abstract relocation values, allowing them to be relative to a symbol. This class will also be useful for managing ARM64X relocation offsets in the future.

llvm-ci · 2025-01-10T23:51:06Z

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building lld at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/11797

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Input file: /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM
Output file (aarch64): /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
Loaded 3 functions from DWARF.
Loaded 3 functions from symbol table.
warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000001
addr=0x0000000000000248, file=  1, line=  5
addr=0x0000000000000254, file=  1, line=  7
addr=0x0000000000000258, file=  1, line=  9
addr=0x000000000000025c, file=  1, line=  8
addr=0x0000000000000260, file=  1, line= 11
addr=0x0000000000000264, file=  1, line= 10
addr=0x0000000000000268, file=  1, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000030
addr=0x0000000000000248, file=  2, line=  5
addr=0x0000000000000254, file=  2, line=  7
addr=0x0000000000000258, file=  2, line=  9
addr=0x000000000000025c, file=  2, line=  8
addr=0x0000000000000260, file=  2, line= 11
addr=0x0000000000000264, file=  2, line= 10
...

…directory (llvm#121337) This change ensures the load config in the hybrid image view is handled correctly. It introduces a new Arm64XRelocVal class to abstract relocation values, allowing them to be relative to a symbol. This class will also be useful for managing ARM64X relocation offsets in the future.

cjacek requested a review from mstorsjo December 30, 2024 13:58

llvmbot added lld lld:COFF platform:windows labels Dec 30, 2024

cjacek mentioned this pull request Jan 5, 2025

[LLD][COFF] Emit base relocation for native CHPE metadata pointer on ARM64X #121500

Merged

cjacek force-pushed the loadcfg-reloc branch from 1c0185e to e10e7e2 Compare January 9, 2025 23:21

mstorsjo approved these changes Jan 10, 2025

View reviewed changes

cjacek force-pushed the loadcfg-reloc branch from e10e7e2 to 30d83b4 Compare January 10, 2025 17:09

cjacek merged commit 3b0daff into llvm:main Jan 10, 2025
8 checks passed

cjacek deleted the loadcfg-reloc branch January 10, 2025 20:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LLD][COFF] Use EC load config for ARM64X relocations of load config directory #121337

[LLD][COFF] Use EC load config for ARM64X relocations of load config directory #121337

cjacek commented Dec 30, 2024

llvmbot commented Dec 30, 2024 •

edited

Loading

cjacek commented Jan 9, 2025

mstorsjo left a comment

mstorsjo Jan 10, 2025

cjacek Jan 10, 2025

llvm-ci commented Jan 10, 2025

[LLD][COFF] Use EC load config for ARM64X relocations of load config directory #121337

[LLD][COFF] Use EC load config for ARM64X relocations of load config directory #121337

Conversation

cjacek commented Dec 30, 2024

llvmbot commented Dec 30, 2024 • edited Loading

cjacek commented Jan 9, 2025

mstorsjo left a comment

Choose a reason for hiding this comment

mstorsjo Jan 10, 2025

Choose a reason for hiding this comment

cjacek Jan 10, 2025

Choose a reason for hiding this comment

llvm-ci commented Jan 10, 2025

llvmbot commented Dec 30, 2024 •

edited

Loading