From e5f77fe98fc574038d9a511012be3f2471cf2b08 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Sun, 5 Oct 2025 23:59:48 +0200 Subject: [PATCH 1/5] test workflows --- .forgejo/workflows/demo.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .forgejo/workflows/demo.yaml diff --git a/.forgejo/workflows/demo.yaml b/.forgejo/workflows/demo.yaml new file mode 100644 index 0000000..efdc13f --- /dev/null +++ b/.forgejo/workflows/demo.yaml @@ -0,0 +1,6 @@ +on: [push] +jobs: + test: + runs-on: docker + steps: + - run: echo All good! From b62d10462c4d3f41cb0f07ca005805fa87dfe911 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Mon, 6 Oct 2025 00:03:06 +0200 Subject: [PATCH 2/5] test --- .forgejo/workflows/demo.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.forgejo/workflows/demo.yaml b/.forgejo/workflows/demo.yaml index efdc13f..ad6df49 100644 --- a/.forgejo/workflows/demo.yaml +++ b/.forgejo/workflows/demo.yaml @@ -4,3 +4,5 @@ jobs: runs-on: docker steps: - run: echo All good! + - uses: actions/checkout@v4 + - run: ls -la From 701d6c78f29fc626c0dfb3609acce635394da2a1 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Fri, 21 Nov 2025 13:16:01 +0100 Subject: [PATCH 3/5] do not optimize the test lib else localFunction in test_local_function is likely inlined and the tests fail --- tests/CMakeLists.txt | 4 +++- tests/test_local_function.cpp | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e572dbb..d0afa78 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -7,6 +7,7 @@ add_executable(mach_detours_tests test_threads.cpp test_transaction.cpp ) +target_compile_options(mach_detours_tests PRIVATE "-O0") # The target function must be in a shared library because otherwise it might be in the same code page as the test.cpp functions. # Between attach and commit the target function's code page is not executable, which can mean our test driver code would not @@ -15,10 +16,11 @@ add_library(mach_detours_tests_lib SHARED lib_function.c lib_function.h ) +target_compile_options(mach_detours_tests_lib PRIVATE "-O0") target_link_libraries(mach_detours_tests PRIVATE mach_detours Catch2::Catch2WithMain mach_detours_tests_lib -) \ No newline at end of file +) diff --git a/tests/test_local_function.cpp b/tests/test_local_function.cpp index 49b2d60..2f29e06 100644 --- a/tests/test_local_function.cpp +++ b/tests/test_local_function.cpp @@ -4,6 +4,9 @@ #include +// Note that this file must be compiled without optimizations (or at least without inlining) +// else localFunction() calls below are likely going to be inlined at the call site, and detours will not work. + static int localFunctionCounter = 0; int localFunction() { From 21962fb04de31d0836cf7d77fcbaaa4a3ec12df9 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Fri, 21 Nov 2025 13:16:01 +0100 Subject: [PATCH 4/5] intellij housekeeping --- .idea/vcs.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/.idea/vcs.xml b/.idea/vcs.xml index 94a25f7..f0f41f0 100644 --- a/.idea/vcs.xml +++ b/.idea/vcs.xml @@ -2,5 +2,6 @@ + \ No newline at end of file From 92e1d84fc73358120f303e22a05b261db948e849 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Fri, 21 Nov 2025 13:16:01 +0100 Subject: [PATCH 5/5] fix tests (oops) and also make them more robust --- tests/test_threads.cpp | 54 +++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/tests/test_threads.cpp b/tests/test_threads.cpp index 11af631..f6d109f 100644 --- a/tests/test_threads.cpp +++ b/tests/test_threads.cpp @@ -1,5 +1,6 @@ // Copyright (c) Lysann Tranvouez. All rights reserved. +#include #include #include @@ -29,33 +30,62 @@ static const char* localFunctionDetour() } static const char* (*realLocalFunction)() = localFunction; +struct threadData +{ + std::atomic loopCounter; + bool cancel = false; +}; + static void* threadFunc(void* pUserArg) { - const bool* pCancel = static_cast(pUserArg); - while (!*pCancel) { + auto& data = *static_cast(pUserArg); + while (!data.cancel) { localFunction(); localFunction(); localFunction(); localFunction(); localFunction(); + ++data.loopCounter; } return nullptr; } +template +bool resetLoopCountersAndWaitForLoop(std::array& threadDatas, std::optional timeout = std::nullopt) +{ + for (auto i = 0; i < numThreads; i++) { + threadDatas[i].loopCounter = 0; + } + + const auto endTime = timeout ? std::make_optional(std::chrono::steady_clock::now() + *timeout) : std::nullopt; + while (true) { + // we check for >= 2 to be sure we did a full loop - we don't want to execute just the line incrementing the counter! + const bool anyLooped = std::ranges::any_of(threadDatas, [](const threadData& data){ return data.loopCounter >= 2; }); + if (anyLooped) { + return true; + } + if (endTime && std::chrono::steady_clock::now() > *endTime) { + return false; + } + usleep( 100 ); + } +} + TEST_CASE( "Handling other threads", "[attach][local][threads]" ) { - bool cancelThreads = false; + using namespace std::chrono_literals; constexpr auto numThreads = 150; pthread_t threads[numThreads]; + std::array threadDatas{}; for (auto i = 0; i < numThreads; i++) { - pthread_create(&threads[i], nullptr, threadFunc, (void*)&cancelThreads); + pthread_create(&threads[i], nullptr, threadFunc, (void*)&threadDatas[i]); } SECTION( "running threads modify a value" ) { const int saved_b = g_b; - usleep( 1000 ); // let the threads update g_b a bit + resetLoopCountersAndWaitForLoop(threadDatas); CHECK( saved_b != g_b ); } @@ -79,13 +109,13 @@ TEST_CASE( "Handling other threads", "[attach][local][threads]" ) } int saved_b = g_b; - usleep( 1000 ); // let any running thread update g_b a bit + resetLoopCountersAndWaitForLoop(threadDatas, 1s); CHECK( saved_b == g_b ); CHECK( detour_transaction_commit() == err_none ); saved_b = g_b; - usleep( 1000 ); // let the threads update g_b a bit + resetLoopCountersAndWaitForLoop(threadDatas); CHECK( saved_b != g_b ); } @@ -95,7 +125,7 @@ TEST_CASE( "Handling other threads", "[attach][local][threads]" ) CHECK( detour_transaction_abort() == err_none ); const int saved_b = g_b; - usleep( 1000 ); // let the threads update g_b a bit + resetLoopCountersAndWaitForLoop(threadDatas); CHECK( saved_b != g_b ); } @@ -108,15 +138,17 @@ TEST_CASE( "Handling other threads", "[attach][local][threads]" ) CHECK( detour_attach_and_commit(reinterpret_cast(&realLocalFunction), reinterpret_cast(localFunctionDetour)) == err_none ); const int saved_b = g_b; - usleep( 1000 ); // let the threads update g_b a bit - CHECK( saved_b != g_b ); + resetLoopCountersAndWaitForLoop(threadDatas); + CHECK( saved_b == g_b ); // clean up CHECK( detour_transaction_begin_managed() == err_none ); CHECK( detour_detach_and_commit(reinterpret_cast(&realLocalFunction), reinterpret_cast(localFunctionDetour)) == err_none ); } - cancelThreads = true; + for (auto i = 0; i < numThreads; i++) { + threadDatas[i].cancel = true; + } for (auto i = 0; i < numThreads; i++) { pthread_join(threads[i], nullptr); }