Merge pull request #8845 from rooklz/fix/refs-file-identity

fix(relinker): compare Windows file identities by their 128-bit file ID

core/relinker/CMakeLists.txt

@@ -88,6 +88,9 @@ target_include_directories(relinker_core PUBLIC
 
 target_compile_features(relinker_core PUBLIC cxx_std_20)
 set_target_properties(relinker_core PROPERTIES CXX_EXTENSIONS OFF)
+if(WIN32)
+    set_source_files_properties(${CMAKE_CURRENT_SOURCE_DIR}/io/src/FileIdentity.cpp PROPERTIES COMPILE_DEFINITIONS _WIN32_WINNT=0x0602)
+endif()
 
 if(BUILD_TESTING)
     add_test_executable(andn_lowering_tests ${CMAKE_CURRENT_SOURCE_DIR}/codegen/tests/AndnLoweringTests.cpp)
@@ -297,6 +300,14 @@ if(BUILD_TESTING)
     target_include_directories(buffer_bounds_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/io/include)
     add_test(NAME buffer_bounds COMMAND buffer_bounds_tests)
 
+    add_test_executable(file_identity_tests
+        ${CMAKE_CURRENT_SOURCE_DIR}/io/tests/FileIdentityTests.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/io/src/FileIdentity.cpp
+    )
+    target_include_directories(file_identity_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/io/include)
+    add_test(NAME file_identity COMMAND file_identity_tests)
+    set_tests_properties(file_identity PROPERTIES TIMEOUT 20)
+
     add_test_executable(param_json_utf8_tests
         ${CMAKE_CURRENT_SOURCE_DIR}/../libs/tests/ParamJsonParser.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/../libs/prx/libkernel/AppMetadata/src/ParamJsonParser.cpp)

core/relinker/io/src/FileIdentity.cpp

@@ -1,9 +1,13 @@
 #include <io/FileIdentity.hpp>
 #include <system_error>
-#include <tuple>
 
 #ifdef _WIN32
 #include <windows.h>
+#include <algorithm>
+#include <array>
+#include <cstdint>
+#include <cstring>
+#include <utility>
 #endif
 
 namespace Io {
@@ -11,16 +15,29 @@ namespace Io {
 #ifdef _WIN32
 namespace {
 
-std::tuple<DWORD, DWORD, DWORD> FileIdentity(const std::filesystem::path& path) {
+using Identity = std::pair<std::uint64_t, std::array<std::uint8_t, 16>>;
+
+Identity FileIdentity(const std::filesystem::path& path) {
     const auto handle = CreateFileW(path.c_str(), 0, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr);
     if (handle == INVALID_HANDLE_VALUE)
         throw std::filesystem::filesystem_error("Cannot open file identity", path, std::error_code(static_cast<int>(GetLastError()), std::system_category()));
+    Identity identity{};
+    FILE_ID_INFO idInformation{};
     BY_HANDLE_FILE_INFORMATION information{};
-    const bool read = GetFileInformationByHandle(handle, &information) != 0;
+    bool read = GetFileInformationByHandleEx(handle, FileIdInfo, &idInformation, sizeof(idInformation)) != 0;
+    if (read) {
+        identity.first = idInformation.VolumeSerialNumber;
+        std::copy(std::begin(idInformation.FileId.Identifier), std::end(idInformation.FileId.Identifier), identity.second.begin());
+    } else {
+        read = GetFileInformationByHandle(handle, &information) != 0;
+        identity.first = information.dwVolumeSerialNumber;
+        const std::uint64_t index = (static_cast<std::uint64_t>(information.nFileIndexHigh) << 32) | information.nFileIndexLow;
+        std::memcpy(identity.second.data(), &index, sizeof(index));
+    }
     const auto error = GetLastError();
     CloseHandle(handle);
     if (!read) throw std::filesystem::filesystem_error("Cannot read file identity", path, std::error_code(static_cast<int>(error), std::system_category()));
-    return {information.dwVolumeSerialNumber, information.nFileIndexHigh, information.nFileIndexLow};
+    return identity;
 }
 
 }

core/relinker/io/tests/FileIdentityTests.cpp

@@ -0,0 +1,64 @@
+#include <io/FileIdentity.hpp>
+#include <chrono>
+#include <filesystem>
+#include <fstream>
+#include <iostream>
+#include <stdexcept>
+#include <string>
+
+namespace {
+
+void require(bool condition, const std::string& message) {
+    if (!condition) throw std::runtime_error(message);
+}
+
+void write(const std::filesystem::path& path, const std::string& text) {
+    std::ofstream(path, std::ios::binary) << text;
+    require(std::filesystem::is_regular_file(path), "Cannot create " + path.string());
+}
+
+void requireMissing(const std::filesystem::path& first, const std::filesystem::path& second) {
+    try {
+        Io::SameFile(first, second);
+    } catch (const std::filesystem::filesystem_error&) {
+        return;
+    }
+    throw std::runtime_error("SameFile accepted a missing file");
+}
+
+void run(const std::filesystem::path& directory) {
+    const auto first = directory / "first.bin";
+    const auto link = directory / "link.bin";
+    const auto copy = directory / "copy.bin";
+    const auto folder = directory / "folder";
+    write(first, "same");
+    write(copy, "same");
+    std::filesystem::create_hard_link(first, link);
+    std::filesystem::create_directory(folder);
+    require(Io::SameFile(first, first), "A file differs from itself");
+    require(Io::SameFile(first, folder / ".." / "first.bin"), "Two spellings of one file differ");
+    require(Io::SameFile(first, link), "A hard link differs from its target");
+    require(!Io::SameFile(first, copy), "A copy with the same contents is the same file");
+    require(Io::SameFile(folder, directory / "folder" / "."), "Two spellings of one directory differ");
+    require(!Io::SameFile(first, folder), "A file is the same as a directory");
+    requireMissing(directory / "missing.bin", first);
+    requireMissing(first, directory / "missing.bin");
+}
+
+}
+
+int main() {
+    const auto directory = std::filesystem::temp_directory_path() /
+        ("anyps5-file-identity-" + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count()));
+    try {
+        std::filesystem::create_directories(directory);
+        run(directory);
+    } catch (const std::exception& error) {
+        std::cerr << error.what() << '\n';
+        std::error_code ignored;
+        std::filesystem::remove_all(directory, ignored);
+        return 1;
+    }
+    std::filesystem::remove_all(directory);
+    return 0;
+}

docs/dev/debt/relinker.md

@@ -4,7 +4,6 @@
 
 - [Executable guest imports](../../../core/relinker/relinker/src/guest/GuestModuleBuilder.cpp) - on Linux and macOS, a relocated executable import resolved to one guest provider requires that module and its dependencies at startup, even when it was discovered through `--module-dir` without a DT_NEEDED entry. Unreferenced symbols and no-op relocations do not require an otherwise deferred module. Console module-start ordering is unmeasured.
 
-- Windows [file identity checks](../../../core/relinker/io/src/FileIdentity.cpp) (input and output collisions, the input skipped during guest module discovery, executables converted twice) compare the volume serial number and the 64-bit file index from `GetFileInformationByHandle`. ReFS file IDs are 128-bit, so two distinct files on ReFS can in principle compare equal: a collision or a repeated executable is reported, or a module is skipped; `FILE_ID_INFO` is not used.
 - [Runtime-loaded modules](../../../core/relinker/relinker/src/guest/GuestModuleBuilder.cpp) (`--module-dir`) that another module needs are initialized at startup with no start arguments, before the module that needs them is loaded; the console is assumed to start them when that module is loaded.
 - The executable file that [relinker](../../../core/relinker/elfpatcher/src/windows/WindowsPeWriter.cpp) generates opens a console when launched, unless it is relinked with `--windows-gui`, which selects the GUI subsystem. A GUI build started from Explorer has no standard output or error handle; the [entry stub](../../../core/relinker/elfpatcher/src/windows/WindowsEntryStubBuilder.cpp) gives each missing one `anyps5.log` beside the executable, replacing the previous run's file (#3282). Errors about the executable path or about waiting for the previous LoadExec process are written before the log is opened and are lost when the standard error handle is missing. If the log cannot be created, output to the missing handles is lost.
 - [Guest import registries](../../../core/relinker/relinker/src/guest/GuestModuleBuilder.cpp) - declared host imports of bundled modules are recorded with their module name and relocation target, even when another guest module exports the same stripped NID. No-op relocations are omitted. Registries describe import bindings, not implementation correctness or runtime reachability.