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

[LLD][COFF] Sort base relocations #121699

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

cjacek
Copy link
Contributor

@cjacek cjacek commented Jan 5, 2025

This change ensures that base relocations are sorted in the output, aligning with MSVC linker behavior. While input files typically provide sorted relocations, this update guarantees correct sorting even if the input relocations are unordered.

This change ensures that base relocations are sorted in the output, aligning with MSVC linker behavior.
While input files typically provide sorted relocations, this update guarantees correct sorting even if
the input relocations are unordered.
@cjacek
Copy link
Contributor Author

cjacek commented Jan 5, 2025

Noticed while looking at #121500.

@llvmbot
Copy link
Member

llvmbot commented Jan 5, 2025

@llvm/pr-subscribers-lld-coff

@llvm/pr-subscribers-lld

Author: Jacek Caban (cjacek)

Changes

This change ensures that base relocations are sorted in the output, aligning with MSVC linker behavior. While input files typically provide sorted relocations, this update guarantees correct sorting even if the input relocations are unordered.


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

2 Files Affected:

  • (modified) lld/COFF/Writer.cpp (+2)
  • (added) lld/test/COFF/baserel-sort.yaml (+77)
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index a5b2b6238db1a9..138112f109fd43 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -2560,6 +2560,8 @@ void Writer::addBaserelBlocks(std::vector<Baserel> &v) {
   const uint32_t mask = ~uint32_t(pageSize - 1);
   uint32_t page = v[0].rva & mask;
   size_t i = 0, j = 1;
+  llvm::sort(v,
+             [](const Baserel &x, const Baserel &y) { return x.rva < y.rva; });
   for (size_t e = v.size(); j < e; ++j) {
     uint32_t p = v[j].rva & mask;
     if (p == page)
diff --git a/lld/test/COFF/baserel-sort.yaml b/lld/test/COFF/baserel-sort.yaml
new file mode 100644
index 00000000000000..d5affe11b2189b
--- /dev/null
+++ b/lld/test/COFF/baserel-sort.yaml
@@ -0,0 +1,77 @@
+# Verify that lld-link outputs sorted base relocations, even if the input file has unsorted entries.
+
+# RUN: yaml2obj %s -o %t.obj
+# RUN: lld-link -dll -entry:sym -machine:amd64 -out:%t.dll %t.obj
+# RUN: llvm-readobj --coff-basereloc %t.dll | FileCheck %s
+
+# CHECK:      BaseReloc [
+# CHECK-NEXT:   Entry {
+# CHECK-NEXT:     Type: DIR64
+# CHECK-NEXT:     Address: 0x2000
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Entry {
+# CHECK-NEXT:     Type: DIR64
+# CHECK-NEXT:     Address: 0x2008
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Entry {
+# CHECK-NEXT:     Type: DIR64
+# CHECK-NEXT:     Address: 0x2010
+# CHECK-NEXT:   }
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     C3
+    SizeOfRawData:   1
+  - Name:            .test
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     '000000000000000000000000000000000000000000000000'
+    SizeOfRawData:   24
+    Relocations:
+      - VirtualAddress:  16
+        SymbolName:      sym
+        Type:            IMAGE_REL_AMD64_ADDR64
+      - VirtualAddress:  0
+        SymbolName:      sym
+        Type:            IMAGE_REL_AMD64_ADDR64
+      - VirtualAddress:  8
+        SymbolName:      sym
+        Type:            IMAGE_REL_AMD64_ADDR64
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          1
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        40735498
+      Number:          1
+  - Name:            .test
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          24
+      NumberOfRelocations: 3
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          2
+  - Name:            sym
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

@llvmbot
Copy link
Member

llvmbot commented Jan 5, 2025

@llvm/pr-subscribers-platform-windows

Author: Jacek Caban (cjacek)

Changes

This change ensures that base relocations are sorted in the output, aligning with MSVC linker behavior. While input files typically provide sorted relocations, this update guarantees correct sorting even if the input relocations are unordered.


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

2 Files Affected:

  • (modified) lld/COFF/Writer.cpp (+2)
  • (added) lld/test/COFF/baserel-sort.yaml (+77)
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index a5b2b6238db1a9..138112f109fd43 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -2560,6 +2560,8 @@ void Writer::addBaserelBlocks(std::vector<Baserel> &v) {
   const uint32_t mask = ~uint32_t(pageSize - 1);
   uint32_t page = v[0].rva & mask;
   size_t i = 0, j = 1;
+  llvm::sort(v,
+             [](const Baserel &x, const Baserel &y) { return x.rva < y.rva; });
   for (size_t e = v.size(); j < e; ++j) {
     uint32_t p = v[j].rva & mask;
     if (p == page)
diff --git a/lld/test/COFF/baserel-sort.yaml b/lld/test/COFF/baserel-sort.yaml
new file mode 100644
index 00000000000000..d5affe11b2189b
--- /dev/null
+++ b/lld/test/COFF/baserel-sort.yaml
@@ -0,0 +1,77 @@
+# Verify that lld-link outputs sorted base relocations, even if the input file has unsorted entries.
+
+# RUN: yaml2obj %s -o %t.obj
+# RUN: lld-link -dll -entry:sym -machine:amd64 -out:%t.dll %t.obj
+# RUN: llvm-readobj --coff-basereloc %t.dll | FileCheck %s
+
+# CHECK:      BaseReloc [
+# CHECK-NEXT:   Entry {
+# CHECK-NEXT:     Type: DIR64
+# CHECK-NEXT:     Address: 0x2000
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Entry {
+# CHECK-NEXT:     Type: DIR64
+# CHECK-NEXT:     Address: 0x2008
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Entry {
+# CHECK-NEXT:     Type: DIR64
+# CHECK-NEXT:     Address: 0x2010
+# CHECK-NEXT:   }
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     C3
+    SizeOfRawData:   1
+  - Name:            .test
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     '000000000000000000000000000000000000000000000000'
+    SizeOfRawData:   24
+    Relocations:
+      - VirtualAddress:  16
+        SymbolName:      sym
+        Type:            IMAGE_REL_AMD64_ADDR64
+      - VirtualAddress:  0
+        SymbolName:      sym
+        Type:            IMAGE_REL_AMD64_ADDR64
+      - VirtualAddress:  8
+        SymbolName:      sym
+        Type:            IMAGE_REL_AMD64_ADDR64
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          1
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        40735498
+      Number:          1
+  - Name:            .test
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          24
+      NumberOfRelocations: 3
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          2
+  - Name:            sym
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

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.

2 participants