Merge pull request #4 from tashcan/refactor/relocators

Rework x64 relocation to use a generic code path

.github/workflows/benchmark.yaml

@@ -23,6 +23,10 @@ jobs:
         run: |
           echo "build-output-dir=${{ github.workspace }}/build" >> "$GITHUB_OUTPUT"
 
+      - uses: mjp41/workaround8649@c8550b715ccdc17f89c8d5c28d7a48eeff9c94a8
+        with:
+          os: ubuntu-latest
+
       - name: Configure CMake
         run: >
           cmake -B ${{ steps.strings.outputs.build-output-dir }}

.github/workflows/build-and-test.yaml

@@ -11,15 +11,7 @@ jobs:
     runs-on: ${{ matrix.os }}
 
     strategy:
-      # Set fail-fast to false to ensure that feedback is delivered for all matrix combinations. Consider changing this to true when your workflow is stable.
       fail-fast: false
-
-      # Set up a matrix to run the following 3 configurations:
-      # 1. <Windows, Release, latest MSVC compiler toolchain on the default runner image, default generator>
-      # 2. <Linux, Release, latest GCC compiler toolchain on the default runner image, default generator>
-      # 3. <Linux, Release, latest Clang compiler toolchain on the default runner image, default generator>
-      #
-      # To add more build types (Release, Debug, RelWithDebInfo, etc.) customize the build_type list.
       matrix:
         os: [ubuntu-latest, windows-latest, macos-latest]
         build_type: [Release, Debug]
@@ -54,15 +46,16 @@ jobs:
         submodules: 'recursive'
 
     - name: Set reusable strings
-      # Turn repeated input strings (such as the build output directory) into step outputs. These step outputs can be used throughout the workflow file.
       id: strings
       shell: bash
       run: |
         echo "build-output-dir=${{ github.workspace }}/build" >> "$GITHUB_OUTPUT"
-
+  
+    - uses: mjp41/workaround8649@c8550b715ccdc17f89c8d5c28d7a48eeff9c94a8
+      with:
+        os: ${{ matrix.os }}
+
     - name: Configure CMake
-      # Configure CMake in a 'build' subdirectory. `CMAKE_BUILD_TYPE` is only required if you are using a single-configuration generator such as make.
-      # See https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html?highlight=cmake_build_type
       run: >
         cmake -B ${{ steps.strings.outputs.build-output-dir }}
         -DCMAKE_CXX_COMPILER=${{ matrix.cpp_compiler }}
@@ -71,12 +64,9 @@ jobs:
         -S ${{ github.workspace }}
 
     - name: Build
-      # Build your program with the given configuration. Note that --config is needed because the default Windows generator is a multi-config generator (Visual Studio generator).
       run: cmake --build ${{ steps.strings.outputs.build-output-dir }} --config ${{ matrix.build_type }} -j
 
     - name: Test
       working-directory: ${{ steps.strings.outputs.build-output-dir }}
-      # Execute tests defined by the CMake configuration. Note that --build-config is needed because the default Windows generator is a multi-config generator (Visual Studio generator).
-      # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
       run: ctest --build-config ${{ matrix.build_type }} --output-on-failure -R spud.test
 

CMakeLists.txt

@@ -15,9 +15,18 @@ set(CXX_STANDARD_REQUIRED ON)
 # set(CMAKE_EXPORT_COMPILE_COMMANDS ON CACHE INTERNAL "")
 set(ASMJIT_STATIC TRUE)
 
+option(SPUD_COMPARE_LIBS "" OFF)
+if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+set(__SPUD_IS_SELF ON)
+else()
+set(__SPUD_IS_SELF OFF)
+endif()
+option(SPUD_BUILD_TESTS "" ${__SPUD_IS_SELF})
+
 if (NOT TARGET Zydis)
 option(ZYDIS_BUILD_TOOLS "" OFF)
 option(ZYDIS_BUILD_EXAMPLES "" OFF)
+option(ZYDIS_BUILD_TOOLS "" OFF)
 FetchContent_Declare(
   zydis
   GIT_REPOSITORY https://github.com/zyantific/zydis.git
@@ -151,8 +160,7 @@ target_sources("spud"
     "${SPUD_AVX2_SRCS}"
 )
 
-
-if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+if(SPUD_BUILD_TESTS)
 
 if (WIN32)
 file(GLOB_RECURSE TEST_SRCS tests/x86_64/*.cc tests/x86_64/*.h tests/x86_64/*.asm)
@@ -178,7 +186,7 @@ FetchContent_Declare(
 FetchContent_MakeAvailable(Catch2)
 endif()
 
-if (COMPARE_LIBS)
+if (SPUD_COMPARE_LIBS)
 
 add_compile_definitions("SPUD_COMPARE_LIBS=1")
 
@@ -195,6 +203,16 @@ FetchContent_Declare(
 )
 
 FetchContent_MakeAvailable(minhook)
+
+set(ASMJIT_EXTERNAL ON)
+set(POLYHOOK_USE_EXTERNAL_ZYDIS ON)
+set(POLYHOOK_USE_EXTERNAL_ASMJIT ON)
+FetchContent_Declare(
+    PolyHook2
+    GIT_REPOSITORY "https://github.com/stevemk14ebr/PolyHook_2_0"
+)
+FetchContent_MakeAvailable(PolyHook2)
+
 endif()
 
 add_executable("spud.test" ${TEST_SRCS} "tests/test_util.h" "tests/pattern.cc")
@@ -204,8 +222,8 @@ add_dependencies("spud.test" "Catch2" "spud")
 
 add_executable("spud.benchmark" ${BENCHMARK_SRCS})
 target_include_directories("spud.benchmark" PRIVATE "benchmark")
-if (COMPARE_LIBS)
-target_link_libraries("spud.benchmark" PRIVATE "Catch2" "Catch2::Catch2WithMain" "spud" "lime" "minhook")
+if (SPUD_COMPARE_LIBS)
+target_link_libraries("spud.benchmark" PRIVATE "Catch2" "Catch2::Catch2WithMain" "spud" "lime" "minhook" "PolyHook2")
 else()
 target_link_libraries("spud.benchmark" PRIVATE "Catch2" "Catch2::Catch2WithMain" "spud")
 endif()

src/detour/aarch64/aarch64.cc

@@ -51,32 +51,16 @@ std::tuple<RelocationInfo, size_t> collect_relocations(uintptr_t address,
 
   return {relocation_info, jump_size};
 }
-size_t get_trampoline_size(std::span<uint8_t> target,
-                           const RelocationInfo &relocation_info) {
-  //
-  size_t required_space = target.size();
-
-  auto target_start = reinterpret_cast<uintptr_t>(target.data());
-  // for (const auto &relot : relocation_info.relocations) {
-  //   auto &relo = std::get<RelocationEntry>(relot);
-
-  //   auto &r_meta = relo_meta.at(relo.instruction);
-  //   required_space += r_meta.size;
-  // }
-  required_space += kAbsoluteJumpSize;
-  return required_space;
-}
 
-Trampoline create_trampoline(uintptr_t trampoline_address,
-                             uintptr_t return_address,
+Trampoline create_trampoline(uintptr_t return_address,
                              std::span<uint8_t> target,
                              const RelocationInfo &relocation_infos) {
   //
   using namespace asmjit;
   using namespace asmjit::a64;
 
   CodeHolder code;
-  code.init(Environment{asmjit::Arch::kAArch64}, trampoline_address);
+  code.init(Environment{asmjit::Arch::kAArch64}, 0x0);
   Assembler assembler(&code);
 
   const auto target_start = reinterpret_cast<uintptr_t>(target.data());

src/detour/aarch64/aarch64.h

@@ -12,10 +12,7 @@ std::vector<uint8_t> create_absolute_jump(uintptr_t target,
                                           uintptr_t container);
 std::tuple<RelocationInfo, size_t> collect_relocations(uintptr_t address,
                                                        size_t jump_size);
-size_t get_trampoline_size(std::span<uint8_t> target,
-                           const RelocationInfo &relocation);
-Trampoline create_trampoline(uintptr_t trampoline_address,
-                             uintptr_t return_address,
+Trampoline create_trampoline(uintptr_t return_address,
                              std::span<uint8_t> target,
                              const RelocationInfo &relocation_infos);
 } // namespace spud::detail::arm64

src/detour/detour.cc

@@ -27,10 +27,7 @@ struct DetourImpl {
                                                uintptr_t data);
   std::tuple<RelocationInfo, size_t> (*collect_relocations)(uintptr_t address,
                                                             size_t jump_size);
-  size_t (*get_trampoline_size)(std::span<uint8_t> target,
-                                const RelocationInfo &relocation);
-  Trampoline (*create_trampoline)(uintptr_t trampoline_address,
-                                  uintptr_t return_address,
+  Trampoline (*create_trampoline)(uintptr_t return_address,
                                   std::span<uint8_t> target,
                                   const RelocationInfo &relocation_infos);
   uintptr_t (*maybe_resolve_jump)(uintptr_t) = [](auto v) { return v; };
@@ -40,15 +37,13 @@ const static std::array<DetourImpl, Arch::kCount> kDetourImpls = {
     // x86_64
     DetourImpl{.create_absolute_jump = x64::create_absolute_jump,
                .collect_relocations = x64::collect_relocations,
-               .get_trampoline_size = x64::get_trampoline_size,
                .create_trampoline = x64::create_trampoline,
                .maybe_resolve_jump = x64::maybe_resolve_jump},
     // x86
     DetourImpl{.create_absolute_jump = x64::create_absolute_jump},
     // Arm64
     DetourImpl{.create_absolute_jump = arm64::create_absolute_jump,
                .collect_relocations = arm64::collect_relocations,
-               .get_trampoline_size = arm64::get_trampoline_size,
                .create_trampoline = arm64::create_trampoline},
 };
 
@@ -70,15 +65,8 @@ detail::detour &detour::install(Arch arch) {
   // Required trampoline size is how many bytes we have to take up of the
   // original function This will then be used to calculate the expanded size,
   // since we do have some instruction replacement and expansion going on
-  auto trampoline_size = impl.get_trampoline_size(
-      {reinterpret_cast<uint8_t *>(address_), required_trampoline_size},
-      relocation_infos);
-
-  auto trampoline_address = alloc_executable_memory(trampoline_size);
-  assert(trampoline_address != nullptr);
 
   auto trampoline = impl.create_trampoline(
-      reinterpret_cast<uintptr_t>(trampoline_address),
       address_ + required_trampoline_size,
       {reinterpret_cast<uint8_t *>(address_), required_trampoline_size},
       relocation_infos);
@@ -87,6 +75,9 @@ detail::detour &detour::install(Arch arch) {
   // permissions
   disable_jit_write_protection();
 
+  auto trampoline_address = alloc_executable_memory(trampoline.data.size());
+  assert(trampoline_address != nullptr);
+
   std::memcpy(trampoline_address, trampoline.data.data(),
               trampoline.data.size());
   trampoline_ =

src/detour/detour_impl.h

@@ -19,7 +19,6 @@ struct Trampoline {
 using RelocationEntry = std::variant<x64::RelocationEntry>;
 
 struct RelocationInfo {
-  std::map<uintptr_t, uintptr_t> relocation_offset;
   std::vector<RelocationEntry> relocations;
 };