Merge pull request #11299 from p3n1cillin/claude/relinker-duplicate-relocations

fix(relinker): accept an identical duplicate relocation in Windows executables

core/relinker/CMakeLists.txt

@@ -550,6 +550,9 @@ if(BUILD_TESTING)
         add_test(NAME overlapping_relocations COMMAND ${Python3_EXECUTABLE}
             ${CMAKE_CURRENT_SOURCE_DIR}/relinker/tests/test_overlapping_relocations.py $<TARGET_FILE:relinker>)
         set_tests_properties(overlapping_relocations PROPERTIES TIMEOUT 30)
+        add_test(NAME duplicate_relocations COMMAND ${Python3_EXECUTABLE}
+            ${CMAKE_CURRENT_SOURCE_DIR}/relinker/tests/test_duplicate_relocations.py $<TARGET_FILE:relinker>)
+        set_tests_properties(duplicate_relocations PROPERTIES TIMEOUT 30)
         add_test(NAME guest_own_tls_relocations COMMAND ${Python3_EXECUTABLE}
             ${CMAKE_CURRENT_SOURCE_DIR}/relinker/tests/test_guest_own_tls_relocations.py $<TARGET_FILE:relinker>)
         set_tests_properties(guest_own_tls_relocations PROPERTIES TIMEOUT 30)

core/relinker/elfpatcher/src/windows/WindowsRelocationBuilder.cpp

@@ -6,6 +6,7 @@
 #include <limits>
 #include <map>
 #include <string>
+#include <utility>
 
 namespace Elfpatcher::Windows {
 
@@ -15,6 +16,7 @@ PeRelocations WindowsRelocationBuilder::Apply(WindowsLoadImage& image, const Dom
         throw Domain::RelinkerException("Invalid ELF dynamic symbol table size");
     PeRelocations result;
     std::map<std::uint64_t, std::uint64_t> ranges;
+    std::map<std::uint64_t, std::pair<std::uint64_t, std::uint64_t>> entries;
     const auto applyTable = [&](const std::vector<std::uint8_t>& table, const bool plt) {
         if (table.size() % 24 != 0)
             throw Domain::RelinkerException("Invalid ELF RELA table size");
@@ -29,10 +31,13 @@ PeRelocations WindowsRelocationBuilder::Apply(WindowsLoadImage& image, const Dom
             if (target > std::numeric_limits<std::uint64_t>::max() - 8)
                 throw Domain::RelinkerException("Relocation target address overflow", target);
             const auto rva = image.GetRva(target, 8);
+            const auto applied = entries.find(target);
+            if (applied != entries.end() && applied->second == std::pair{info, addend}) continue;
             const auto next = ranges.lower_bound(target);
             if ((next != ranges.end() && next->first < target + 8) || (next != ranges.begin() && std::prev(next)->second > target))
                 throw Domain::RelinkerException("Overlapping ELF relocation targets", target);
             ranges.emplace(target, target + 8);
+            entries.emplace(target, std::pair{info, addend});
             if (type == 8) {
                 if (symbol != 0)
                     throw Domain::RelinkerException("RELATIVE relocation has a symbol index", target);

core/relinker/relinker/src/analysis/CodeInstructionCollector.cpp

@@ -310,7 +310,8 @@ std::set<Domain::VirtualAddress> CodeInstructionCollector::Collect(const std::ve
                 const auto matchingDefinedSymbol = hasSymbol && Io::ReadU16(bytes, symbol + 6) != 0
                     && addend <= static_cast<std::uint64_t>(std::numeric_limits<std::int64_t>::max())
                     && Io::ReadU64(bytes, symbol + 8) == addend;
-                if ((index != 0 && !zeroSectionSymbol && !matchingDefinedSymbol) || !pointers.Values.emplace(slot, addend).second) throw Domain::RelinkerException("Code analysis: invalid relative relocation", slot);
+                const auto [recorded, inserted] = pointers.Values.emplace(slot, addend);
+                if ((index != 0 && !zeroSectionSymbol && !matchingDefinedSymbol) || (!inserted && recorded->second != addend)) throw Domain::RelinkerException("Code analysis: invalid relative relocation", slot);
                 addPointerRoot(addend);
             } else if (type == 1 || type == 6 || type == 7) {
                 if (index != 0 && index >= symbolCount) throw Domain::RelinkerException("Code analysis: invalid relocation symbol", slot);
@@ -318,7 +319,8 @@ std::set<Domain::VirtualAddress> CodeInstructionCollector::Collect(const std::ve
                 if (index == 0 || Io::ReadU16(bytes, symbol + 6) == 0) importSlots.insert(slot);
                 else {
                     const auto target = Io::ReadU64(bytes, symbol + 8) + addend;
-                    if (!pointers.Values.emplace(slot, target).second) throw Domain::RelinkerException("Code analysis: duplicate pointer relocation", slot);
+                    const auto [recorded, inserted] = pointers.Values.emplace(slot, target);
+                    if (!inserted && recorded->second != target) throw Domain::RelinkerException("Code analysis: duplicate pointer relocation", slot);
                     addPointerRoot(target);
                 }
             }

core/relinker/relinker/tests/test_duplicate_relocations.py

@@ -0,0 +1,55 @@
+import struct
+import subprocess
+import sys
+import tempfile
+from pathlib import Path
+
+from test_guest_none_relocations import module
+from test_overlapping_relocations import TARGETS, check_output, fixture
+
+
+def with_second_entry(entry):
+    image, local, plt = fixture("disjoint", False, False)
+    struct.pack_into("<QQq", image, 0x700 + 24, *entry)
+    for offset in range(0x400, 0x500, 16):
+        tag, value = struct.unpack_from("<qQ", image, offset)
+        if tag == 8:
+            struct.pack_into("<qQ", image, offset, tag, value + 24)
+            return image, local, plt
+    raise AssertionError("no DT_RELASZ tag")
+
+
+def main():
+    relinker = Path(sys.argv[1]).resolve()
+    with tempfile.TemporaryDirectory(prefix="anyps5-relocation-duplicate-") as directory:
+        work = Path(directory)
+
+        def convert(name, target, options, image):
+            case = work / f"{name}-{target}"
+            modules = case / "sce_module"
+            modules.mkdir(parents=True)
+            (modules / "module.prx").write_bytes(module())
+            source = case / "input.elf"
+            source.write_bytes(image)
+            output = case / ("output.exe" if target == "windows" else "output.out")
+            result = subprocess.run([str(relinker), "--registry", *options, str(source), str(output)],
+                                    capture_output=True, text=True, timeout=10)
+            return result, output
+
+        _, local, _ = fixture("disjoint", False, False)
+        duplicate, _, plt = with_second_entry(local[0])
+        for target, options in TARGETS:
+            result, output = convert("duplicate", target, options, duplicate)
+            assert result.returncode == 0, (target, result.stdout, result.stderr)
+            if target != "linux":
+                check_output(output.read_bytes(), target, False, local, plt)
+        offset, info, addend = local[0]
+        conflicting, _, _ = with_second_entry((offset, info, addend + 8))
+        result, output = convert("conflicting", "windows", ["--windows"], conflicting)
+        assert result.returncode == 2 and "Overlapping ELF relocation targets" in result.stderr and not output.exists(), (
+            result.stdout, result.stderr)
+    print("Duplicate relocation tests passed")
+
+
+if __name__ == "__main__":
+    main()