On Wed Dec 20 17:00:09 2023 +0000, Jinoh Kang wrote:
Hello there! Thanks for your contribution. Apropos commit "ntdll/tests: Move test_continue() to be used for all platforms": I think there is a better way to test `NtContinue()` across platforms. You're approach of sharing the `test_continue()` function header/footer works, but ends up branching on the architecture *twice*.
static void test_continue(void) { ... #ifdef __x86_64__ static const BYTE call_func[] = { ... }; #elif defined(__aarch64__) static const DWORD call_func[] = { ... }; #else ... #endif ... memcpy( func_ptr, call_func, sizeof(call_func) ); FlushInstructionCache( GetCurrentProcess(), func_ptr, sizeof(call_func) ); func_ptr( &contexts, FALSE, NtContinue, pRtlCaptureContext ); #ifdef __x86_64__ ... #elif defined(__aarch64__) ... #endif ... }Notice that there is little code shared between architectures, other than `NtContinue` and `RtlCaptureContext`. Instead, I suggest that you define the entire `test_continue()` per each architecture:
#ifdef __x86_64__ ... static void test_thread_context(void) { ... #undef COMPARE } static void test_continue(void) { ... static const BYTE call_func[] = { ... }; ... memcpy( func_ptr, call_func, sizeof(call_func) ); FlushInstructionCache( GetCurrentProcess(), func_ptr, sizeof(call_func) ); func_ptr( &contexts, FALSE, NtContinue, pRtlCaptureContext ); ... } static void test_wow64_context(void) { ... } ... #elif defined(__arch64__) ... static void test_continue(void) { ... }Even if this leads to code duplication, this allows us flexibility to extend `test_continue` with arch-specific logic in the future, and avoids interleaving of different test code for each architecture.
I don't think that's better because essentially you want to test same functionality across all architectures but having them completely independent will cause tests to diverge.
Anyway I updated MR.