Skip to content

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
+...

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

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

LGTM

@cjacek cjacek merged commit c57810a into llvm:main Jan 9, 2025
12 checks passed
@cjacek cjacek deleted the basereloc-sort branch January 9, 2025 14:40
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