Merge pull request #10997 from SandstromPL/fix/relinker-backup-copies-in-conversion

fix(relinker): skip backup copies of guest modules in the online converter too

core/relinker/cli/src/NativeModules.cpp

@@ -115,31 +115,9 @@ NativeModules ReadModules(const std::filesystem::path& inputPath, const std::fil
     result.Directories = std::move(directories);
     result.RuntimeDirectories = std::move(runtimeDirectories);
     for (const auto& path : paths) result.Modules.push_back({path, Io::FileReader().Read(Io::Utf8Path(path))});
-    std::map<std::size_t, std::string> sonames;
-    const auto soname = [&](std::size_t index) -> const std::string& {
-        const auto [found, inserted] = sonames.try_emplace(index);
-        if (inserted) found->second = GuestImageReader().Read(result.Modules[index].Path, result.Modules[index].Bytes, true).Soname;
-        return found->second;
-    };
-    std::vector<bool> copies(result.Modules.size());
-    for (std::size_t copy = 0; copy < result.Modules.size(); ++copy) {
-        const auto& path = result.Modules[copy].Path;
-        const auto name = Io::Utf8Path(path.filename());
-        for (std::size_t original = 0; original < result.Modules.size(); ++original) {
-            const auto& kept = result.Modules[original].Path;
-            const auto prefix = Io::Utf8Path(kept.filename());
-            if (kept.parent_path() != path.parent_path() || prefix.size() >= name.size() || !name.starts_with(prefix)) continue;
-            if (soname(copy).empty() || soname(copy) != soname(original)) continue;
-            copies[copy] = true;
-            std::cout << "Skipping backup copy: " << path.lexically_relative(root).generic_string() << " (same SONAME as "
-                      << kept.lexically_relative(root).generic_string() << ")\n";
-            break;
-        }
-    }
-    std::vector<GuestModuleInput> modules;
-    for (std::size_t index = 0; index < result.Modules.size(); ++index)
-        if (!copies[index]) modules.push_back(std::move(result.Modules[index]));
-    result.Modules = std::move(modules);
+    for (const auto& [copy, kept] : RemoveBackupCopies(result.Modules))
+        std::cout << "Skipping backup copy: " << copy.lexically_relative(root).generic_string() << " (same SONAME as "
+                  << kept.lexically_relative(root).generic_string() << ")\n";
     return result;
 }
 

core/relinker/relinker/include/relinker/guest/GuestImage.hpp

@@ -9,6 +9,7 @@
 #include <domain/IProgress.hpp>
 #include <set>
 #include <string>
+#include <utility>
 #include <vector>
 
 namespace Relinker {
@@ -79,6 +80,8 @@ struct GuestModuleInput {
     std::vector<GamePatch::PatchedRange> PatchedRanges;
 };
 
+std::vector<std::pair<std::filesystem::path, std::filesystem::path>> RemoveBackupCopies(std::vector<GuestModuleInput>& modules);
+
 class GuestModuleBuilder {
 public:
     std::vector<GuestArtifact> Build(const std::filesystem::path& inputPath, const std::filesystem::path& outputPath, Domain::SysVDynamicSection& dynamic, bool windows, bool macos, bool toIntel, bool toRosetta, ISyscallScanner& syscallScanner, bool lazyBinding, const std::string& runPath, const std::set<std::string>& excludedModules, const std::filesystem::path& root, const std::vector<std::filesystem::path>& runtimeDirectories, std::vector<GuestModuleInput> modules, Domain::IProgress& progress) const;

core/relinker/relinker/src/guest/GuestModuleBuilder.cpp

@@ -489,4 +489,33 @@ std::vector<GuestArtifact> GuestModuleBuilder::Build(const std::filesystem::path
     return artifacts;
 }
 
+
+std::vector<std::pair<std::filesystem::path, std::filesystem::path>> RemoveBackupCopies(std::vector<GuestModuleInput>& modules) {
+    std::map<std::size_t, std::string> sonames;
+    const auto soname = [&](std::size_t index) -> const std::string& {
+        const auto [found, inserted] = sonames.try_emplace(index);
+        if (inserted) found->second = GuestImageReader().Read(modules[index].Path, modules[index].Bytes, true).Soname;
+        return found->second;
+    };
+    std::vector<std::pair<std::filesystem::path, std::filesystem::path>> skipped;
+    std::vector<bool> copies(modules.size());
+    for (std::size_t copy = 0; copy < modules.size(); ++copy) {
+        const auto& path = modules[copy].Path;
+        const auto name = Io::Utf8Path(path.filename());
+        for (std::size_t original = 0; original < modules.size(); ++original) {
+            const auto& kept = modules[original].Path;
+            const auto prefix = Io::Utf8Path(kept.filename());
+            if (kept.parent_path() != path.parent_path() || prefix.size() >= name.size() || !name.starts_with(prefix)) continue;
+            if (soname(copy).empty() || soname(copy) != soname(original)) continue;
+            copies[copy] = true;
+            skipped.emplace_back(path, kept);
+            break;
+        }
+    }
+    std::vector<GuestModuleInput> remaining;
+    for (std::size_t index = 0; index < modules.size(); ++index)
+        if (!copies[index]) remaining.push_back(std::move(modules[index]));
+    modules = std::move(remaining);
+    return skipped;
+}
 }

core/relinker/relinker/src/pipeline/Conversion.cpp

@@ -52,6 +52,9 @@ ConversionOutput Conversion::Convert(ConversionInput input, const ConversionOpti
     if (options.MacosFixedBase && options.Target != TargetSystem::MacOs) throw Domain::RelinkerException("A fixed macOS base requires the macOS target");
     const bool windows = options.Target == TargetSystem::Windows;
     const bool macos = options.Target == TargetSystem::MacOs;
+    for (const auto& [copy, kept] : RemoveBackupCopies(input.Modules))
+        inputProgress.Message("Skipping backup copy: " + copy.lexically_relative(input.ModuleRoot).generic_string() + " (same SONAME as "
+                              + kept.lexically_relative(input.ModuleRoot).generic_string() + ")\n");
     inputProgress.Report({Domain::ProgressStage::Executable, Io::Utf8Path(input.InputPath), 0, 1});
     auto sourceBytes = std::move(input.Executable);
     std::vector<GamePatch::AppliedPatch> appliedPatches;

core/relinker/relinker/tests/ConversionTests.cpp

@@ -25,7 +25,7 @@ void reject(TAction action) {
     throw std::runtime_error("Invalid conversion was accepted");
 }
 
-std::vector<std::uint8_t> fixture(const std::string& needed = {}) {
+std::vector<std::uint8_t> fixture(const std::string& needed = {}, const std::string& soname = {}) {
     std::vector<std::uint8_t> bytes(0x3000);
     const std::array<std::uint8_t, 7> magic{0x7f, 'E', 'L', 'F', 2, 1, 1};
     std::copy(magic.begin(), magic.end(), bytes.begin());
@@ -53,11 +53,16 @@ std::vector<std::uint8_t> fixture(const std::string& needed = {}) {
     header(1, 1, 6, 0x2000, 0x1000);
     for (std::size_t index = 3; index < 8; ++index) header(index, 0x6fffff01, 0, 0, 0);
     bytes[0x1000] = 0xc3;
-    std::vector<std::pair<std::uint64_t, std::uint64_t>> tags{{5, 0x2600}, {10, needed.empty() ? 1 : needed.size() + 2}, {6, 0x2620}, {11, 24}, {7, 0x2700}, {8, 0}, {9, 24}, {4, 0x2680}};
+    const std::size_t sonameOffset = needed.empty() ? 1 : needed.size() + 2;
+    std::vector<std::pair<std::uint64_t, std::uint64_t>> tags{{5, 0x2600}, {10, soname.empty() ? sonameOffset : sonameOffset + soname.size() + 1}, {6, 0x2620}, {11, 24}, {7, 0x2700}, {8, 0}, {9, 24}, {4, 0x2680}};
     if (!needed.empty()) {
         std::copy(needed.begin(), needed.end(), bytes.begin() + 0x2601);
         tags.push_back({1, 1});
     }
+    if (!soname.empty()) {
+        std::copy(soname.begin(), soname.end(), bytes.begin() + 0x2600 + static_cast<std::ptrdiff_t>(sonameOffset));
+        tags.push_back({14, sonameOffset});
+    }
     tags.push_back({0, 0});
     header(2, 2, 6, 0x2000, tags.size() * 16);
     for (std::size_t index = 0; index < tags.size(); ++index) {
@@ -127,6 +132,27 @@ void convert() {
     require(progress.Events.back().Completed != 1 || progress.Events.back().Stage != Domain::ProgressStage::Executable, "Failed conversion reported completion");
 }
 
+void backupCopies() {
+    for (const auto target : {Relinker::TargetSystem::Windows, Relinker::TargetSystem::Linux}) {
+        TestProgress progress;
+        Relinker::ConversionOptions options;
+        options.Target = target;
+        auto source = input();
+        for (const auto* name : {"libSceFace.prx", "libSceFace.prx.esbak", "libSceFace.prxbak"})
+            source.Modules.push_back({std::filesystem::path("virtual/title/sce_module") / name, fixture({}, "libSceFace.prx")});
+        const auto output = Relinker::Conversion(progress).Convert(std::move(source), options);
+        require(output.Modules.size() == 1 && output.Modules.front().Path.generic_string() == "virtual/output/app0/sce_module/libSceFace.prx.guest.prx", "A backup copy was converted");
+        for (const auto* name : {"libSceFace.prx.esbak", "libSceFace.prxbak"})
+            require(progress.Messages.find(std::string("Skipping backup copy: sce_module/") + name + " (same SONAME as sce_module/libSceFace.prx)\n") != std::string::npos, "A skipped backup copy was not reported");
+    }
+    TestProgress progress;
+    auto source = input();
+    source.Modules.push_back({"virtual/title/sce_module/libSceFace.prx", fixture({}, "libSceFace.prx")});
+    source.Modules.push_back({"virtual/title/sce_module/libSceFace.prx.esbak", fixture({}, "libSceFaceOld.prx")});
+    const auto output = Relinker::Conversion(progress).Convert(std::move(source), {});
+    require(output.Modules.size() == 2 && progress.Messages.find("Skipping backup copy") == std::string::npos, "A module with another SONAME was skipped as a backup copy");
+}
+
 void runtimeLifecyclePolicy() {
     TestProgress progress;
     const Relinker::Conversion converter(progress);
@@ -377,6 +403,7 @@ void relocationValidation() {
 int main() {
     try {
         convert();
+        backupCopies();
         runtimeLifecyclePolicy();
         fixedBase();
         scanning();

docs/user/USAGE.md

@@ -14,7 +14,7 @@ Select or drop multiple app ZIP archives to convert them into one ZIP bundle. Th
 
 The SELF unwrapper refuses containers requiring more than 64 MiB of total alignment padding before allocating the output buffer. This limit counts padding across file-backed and empty segments, not segment data; a refusal preserves the input and any existing output file.
 
-Use a clean ELF executable. Place its bundled ELF modules in `sce_module/`, `sce_modules/`, or `prx/` beside the input executable, or under the parent directory selected with `--sce-module-path <path>`. The option accepts absolute paths and paths relative to the current working directory; do not include the `sce_module` directory itself. Only files directly inside the module directories are processed. SELF/fSELF containers are skipped with a warning; unwrap them before relinking if they need to be included. `tools/unwrap_self.py <input.self> <output.elf>` does that for a decrypted container: it places the segments at the offsets of the SELF segment records and restores `p_offset % p_align == p_vaddr % p_align`, and it refuses a container whose load segments all look encrypted. The command-line relinker skips an ELF module as a backup copy, with a message, when its file name extends the name of another ELF module in the same directory and both have the same SONAME, such as a dumper's `libX.prx.esbak` or `libX.prxbak` beside an ELF `libX.prx`; a single backup beside a SELF container is still relinked. `prx/` can coexist with either `sce_module/` or `sce_modules/`. Both `sce_module/` and `sce_modules/` present is an error. All three absent is an error unless an explicit `--module-dir` is supplied.
+Use a clean ELF executable. Place its bundled ELF modules in `sce_module/`, `sce_modules/`, or `prx/` beside the input executable, or under the parent directory selected with `--sce-module-path <path>`. The option accepts absolute paths and paths relative to the current working directory; do not include the `sce_module` directory itself. Only files directly inside the module directories are processed. SELF/fSELF containers are skipped with a warning; unwrap them before relinking if they need to be included. `tools/unwrap_self.py <input.self> <output.elf>` does that for a decrypted container: it places the segments at the offsets of the SELF segment records and restores `p_offset % p_align == p_vaddr % p_align`, and it refuses a container whose load segments all look encrypted. The relinker, on the command line and in the online converter, skips an ELF module as a backup copy, with a message, when its file name extends the name of another ELF module in the same directory and both have the same SONAME, such as a dumper's `libX.prx.esbak` or `libX.prxbak` beside an ELF `libX.prx`; a single backup beside a SELF container is still relinked. `prx/` can coexist with either `sce_module/` or `sce_modules/`. Both `sce_module/` and `sce_modules/` present is an error. All three absent is an error unless an explicit `--module-dir` is supplied.
 
 ```text
 source/