From 55728942007334a0dc9d81c779161cced7d057e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Desbiens?= Date: Tue, 25 Aug 2026 19:24:01 -0400 Subject: [PATCH] Covered the trace entry update paths and the misaligned stack adjustment Against the merged coverage report -- every build configuration instrumented and unioned -- eighteen lines of common/src were uncovered. Seventeen of them were one missing scenario rather than eighteen separate gaps. tx_block_allocate, tx_byte_allocate, tx_thread_system_suspend and tx_thread_system_resume each carry blocks under TX_ENABLE_EVENT_TRACE that go back and patch a trace entry once the call has done its work, all of the shape: if (entry_ptr != TX_NULL) { if (time_stamp == entry_ptr -> tx_trace_buffer_entry_time_stamp) entry_ptr comes from _tx_trace_buffer_current_ptr, which stays TX_NULL until tx_trace_enable is called at run time. Building with TX_ENABLE_EVENT_TRACE is not enough, and exactly one test in the suite enables tracing -- threadx_trace_basic_test -- which tests the enable API itself and never calls either allocator. So those blocks sat in the report's denominator and never in its covered set. threadx_trace_entry_update_test enables tracing and then drives both allocators twice each, once on the path that succeeds immediately and once through a suspension that a second thread satisfies, since each allocator carries one update block on either side. It then sleeps so that the last runnable thread suspends with nothing ready to take over: tx_thread_system_suspend lines 345 and 351 are on the branch that sets _tx_thread_execute_ptr to TX_NULL, and the two allocator suspensions never reach it because the other thread was always ready. The same test closes tx_trace_object_register's TX_NULL name break by creating a semaphore with no name. A semaphore and not a thread deliberately: for TX_TRACE_OBJECT_TYPE_THREAD the register function dereferences the pointer it is given to read the thread's priority, so that type needs a real TX_THREAD behind it. threadx_trace_basic_test makes the equivalent call only under ifndef TX_ENABLE_EVENT_TRACE, against the no-op stub. threadx_thread_misaligned_stack_test covers the remaining line, tx_thread_create.c:136, where a stack that does not begin on a ULONG boundary costs a ULONG of size so that rounding the start up cannot run past the end of the caller's buffer. Every other test hands tx_thread_create an aligned stack. That line is compiled only under TX_ENABLE_STACK_CHECKING, so it is absent from three of the five configurations' reports rather than uncovered in them, and it was verified under stack_checking_build. Measured on the merged report, all 480 tests passing and 5 of 5 configurations green: 4485 of 4503 covered before, 4502 of 4503 after, denominator unchanged. One line remains, tx_thread_system_resume.c:529, and it is the report's last flapping line rather than a standing gap -- two clean runs of the same tree gave 4503 of 4503 and 4502 of 4503. Reaching it by construction was tried twice and failed both times, so it is left alone here. It needs the preempt disable flag and the system state both clear, and tx_thread_resume raises the preempt disable flag before calling _tx_thread_system_resume, as do the put and send paths; creating a higher priority auto-start thread from thread context does not raise it but does not reach the check either, which a probe showed is executed only during initialisation, with the system state at TX_INITIALIZE_IN_PROGRESS. Assisted-by: Claude Opus 5 --- test/tx/cmake/regression/CMakeLists.txt | 2 + .../threadx_thread_misaligned_stack_test.c | 150 ++++++++ .../threadx_trace_entry_update_test.c | 331 ++++++++++++++++++ 3 files changed, 483 insertions(+) create mode 100644 test/tx/regression/threadx_thread_misaligned_stack_test.c create mode 100644 test/tx/regression/threadx_trace_entry_update_test.c diff --git a/test/tx/cmake/regression/CMakeLists.txt b/test/tx/cmake/regression/CMakeLists.txt index 34fba1b2c..7a0296dc1 100644 --- a/test/tx/cmake/regression/CMakeLists.txt +++ b/test/tx/cmake/regression/CMakeLists.txt @@ -80,6 +80,7 @@ set(regression_test_cases ${SOURCE_DIR}/threadx_thread_create_preemption_threshold_test.c ${SOURCE_DIR}/threadx_thread_delayed_suspension_test.c ${SOURCE_DIR}/threadx_thread_information_test.c + ${SOURCE_DIR}/threadx_thread_misaligned_stack_test.c ${SOURCE_DIR}/threadx_thread_multi_level_preemption_threshold_test.c ${SOURCE_DIR}/threadx_thread_multiple_non_current_test.c ${SOURCE_DIR}/threadx_thread_multiple_sleep_test.c @@ -109,6 +110,7 @@ set(regression_test_cases ${SOURCE_DIR}/threadx_timer_multiple_test.c ${SOURCE_DIR}/threadx_timer_simple_test.c ${SOURCE_DIR}/threadx_trace_basic_test.c + ${SOURCE_DIR}/threadx_trace_entry_update_test.c ${SOURCE_DIR}/threadx_initialize_kernel_setup_test.c) add_custom_command( diff --git a/test/tx/regression/threadx_thread_misaligned_stack_test.c b/test/tx/regression/threadx_thread_misaligned_stack_test.c new file mode 100644 index 000000000..63d64775a --- /dev/null +++ b/test/tx/regression/threadx_thread_misaligned_stack_test.c @@ -0,0 +1,150 @@ +/*************************************************************************** + * Copyright (c) 2026 Eclipse ThreadX contributors + * + * This program and the accompanying materials are made available under the + * terms of the MIT License which is available at + * https://opensource.org/licenses/MIT. + * + * AI Disclosure: This file was largely AI-generated by Claude Code (Opus 5). + * The AI-generated portions may be considered public domain (CC0-1.0) + * and not subject to the project's licence. The human contributor has + * reviewed and verified that the code is correct. + * + * SPDX-License-Identifier: MIT and CC0-1.0 + **************************************************************************/ + +/* This test creates a thread whose stack does not begin on a ULONG boundary. + + tx_thread_create rounds the starting address up to the next ULONG and then + takes a ULONG off the size, so that the rounding cannot push the end of the + stack past the memory the caller supplied: + + if (new_stack_start != updated_stack_start) + { + stack_size = stack_size - (sizeof(ULONG)); + } + + Every other test in the suite hands tx_thread_create an aligned stack, so + that subtraction was never executed -- the only uncovered line in + tx_thread_create.c across all five build configurations. A misaligned stack + is legitimate: the API takes a VOID * and the alignment fix-up exists + precisely to accept one. + + The thread is then run to completion rather than merely created, because the + point of the adjustment is that the resulting stack is still usable and still + inside the caller's buffer. */ + +#include +#include "tx_api.h" + + +void test_control_return(UINT status); + + +static TX_THREAD thread_0; +static TX_THREAD thread_1; + +/* Thread 1's stack is carved out of this by hand so that its start can be put + deliberately off a ULONG boundary. The extra ULONG is what the misaligned + start is offset into, so that the stack still ends inside the array after + tx_thread_create has rounded the start up. */ + +static UCHAR misaligned_area[TEST_STACK_SIZE_PRINTF + sizeof(ULONG)]; + +static ULONG thread_1_counter = 0; + + +static void thread_0_entry(ULONG thread_input); +static void thread_1_entry(ULONG thread_input); + + +/* Define what the initial system looks like. */ + +#ifdef CTEST +void test_application_define(void *first_unused_memory) +#else +void threadx_thread_misaligned_stack_application_define(void *first_unused_memory) +#endif +{ + +UINT status; +CHAR *pointer; +CHAR *misaligned_stack; + + + /* Setup a pointer. */ + pointer = (CHAR *) first_unused_memory; + + status = tx_thread_create(&thread_0, "thread 0", thread_0_entry, 0, + pointer, TEST_STACK_SIZE_PRINTF, + 16, 16, TX_NO_TIME_SLICE, TX_AUTO_START); + pointer = pointer + TEST_STACK_SIZE_PRINTF; + + if (status != TX_SUCCESS) + { + + printf("Running Thread Misaligned Stack Test................................ ERROR #1\n"); + test_control_return(1); + } + + /* Push the start one byte past a ULONG boundary. The array itself is + aligned, so adding one is enough to guarantee the fix-up runs whatever + the alignment of the array turned out to be. */ + misaligned_stack = ((CHAR *) misaligned_area) + 1; + + status = tx_thread_create(&thread_1, "thread 1", thread_1_entry, 0, + misaligned_stack, TEST_STACK_SIZE_PRINTF, + 15, 15, TX_NO_TIME_SLICE, TX_AUTO_START); + + if (status != TX_SUCCESS) + { + + printf("Running Thread Misaligned Stack Test................................ ERROR #2\n"); + test_control_return(1); + } +} + + +static void thread_0_entry(ULONG thread_input) +{ + + /* Thread 1 is the higher priority of the two, so this thread first gets the + processor while thread 1 is sleeping. Sleep past that, so the check below + is made against a thread that has actually finished. */ + tx_thread_sleep(5); + + /* Both increments must have happened: one before thread 1 slept and one + after it woke, which is the half that needed the stack to survive a + context switch. */ + if (thread_1_counter != ((ULONG) 2)) + { + + printf("Running Thread Misaligned Stack Test................................ ERROR #3\n"); + test_control_return(1); + } + + /* The thread that ran on the misaligned stack must have run to completion, + which is what says the adjusted stack was usable. */ + if (thread_1.tx_thread_state != TX_COMPLETED) + { + + printf("Running Thread Misaligned Stack Test................................ ERROR #4\n"); + test_control_return(1); + } + + printf("Running Thread Misaligned Stack Test................................ SUCCESS!\n"); + test_control_return(0); +} + + +static void thread_1_entry(ULONG thread_input) +{ + + /* Use the stack either side of a context switch rather than returning + straight away. */ + thread_1_counter++; + + tx_thread_sleep(1); + + thread_1_counter++; +} diff --git a/test/tx/regression/threadx_trace_entry_update_test.c b/test/tx/regression/threadx_trace_entry_update_test.c new file mode 100644 index 000000000..d827497af --- /dev/null +++ b/test/tx/regression/threadx_trace_entry_update_test.c @@ -0,0 +1,331 @@ +/*************************************************************************** + * Copyright (c) 2026 Eclipse ThreadX contributors + * + * This program and the accompanying materials are made available under the + * terms of the MIT License which is available at + * https://opensource.org/licenses/MIT. + * + * AI Disclosure: This file was largely AI-generated by Claude Code (Opus 5). + * The AI-generated portions may be considered public domain (CC0-1.0) + * and not subject to the project's licence. The human contributor has + * reviewed and verified that the code is correct. + * + * SPDX-License-Identifier: MIT and CC0-1.0 + **************************************************************************/ + +/* This test drives the trace entry update paths -- the blocks guarded by + TX_ENABLE_EVENT_TRACE that go back and patch a trace entry after the call + they belong to has finished its work. + + Those blocks all have the same shape: + + if (entry_ptr != TX_NULL) + { + if (time_stamp == entry_ptr -> tx_trace_buffer_entry_time_stamp) + { + entry_ptr -> tx_trace_buffer_entry_information_field_N = ...; + } + } + + entry_ptr comes from _tx_trace_buffer_current_ptr, which stays TX_NULL until + tx_trace_enable() is called at run time. Compiling with TX_ENABLE_EVENT_TRACE + is therefore not enough to reach them, and the trace_build configuration on + its own never does: the only other test that enables tracing -- + threadx_trace_basic_test -- exercises the enable API itself and never calls + tx_block_allocate or tx_byte_allocate at all. So the update blocks in + tx_block_allocate.c, tx_byte_allocate.c, tx_thread_system_suspend.c and + tx_thread_system_resume.c were absent from the coverage report's covered set + while appearing in its denominator. + + Both allocators are driven twice, because they carry two of these blocks + each and only one is on the path that succeeds immediately: the second sits + after a suspension, where the allocation completes in the releasing thread's + context. Hence the second thread, which exists only to release what + thread 0 is waiting on. + + The test is built in all five configurations, and four of them do not define + TX_ENABLE_EVENT_TRACE. There tx_trace_enable reports TX_FEATURE_NOT_ENABLED, + the update blocks do not exist, and the allocation sequence below is still a + valid exercise of both allocators' suspension paths -- so the test is + meaningful in every configuration rather than skipped in four. */ + +#include +#include "tx_api.h" +#define TX_SOURCE_CODE +#include "tx_trace.h" + + +void test_control_return(UINT status); + + +static TX_THREAD thread_0; +static TX_THREAD thread_1; + +static TX_BLOCK_POOL block_pool_0; +static TX_BYTE_POOL byte_pool_0; +static TX_SEMAPHORE semaphore_0; + +/* The trace buffer is sized generously on purpose. The inner test of each + update block compares the saved time stamp against the one in the entry, and + a buffer small enough to wrap between the call and its completion would + overwrite the entry and take the false branch every time -- the block would + be reached and its body still never run. */ + +static UCHAR trace_buffer[16384]; + +/* Four blocks of 20 bytes: 100 / (20 + sizeof(void *)) on a 32-bit build. */ + +static UCHAR block_pool_area[100]; +static UCHAR byte_pool_area[512]; + +/* Allocated by thread 0 and released by thread 1, which is what lets thread 0's + suspended allocations complete. */ + +static void *held_block; +static void *held_bytes; + +static UINT error = 0; + + +static void thread_0_entry(ULONG thread_input); +static void thread_1_entry(ULONG thread_input); + + +/* Define what the initial system looks like. */ + +#ifdef CTEST +void test_application_define(void *first_unused_memory) +#else +void threadx_trace_entry_update_application_define(void *first_unused_memory) +#endif +{ + +UINT status; +CHAR *pointer; + + + /* Setup a pointer. */ + pointer = (CHAR *) first_unused_memory; + + /* Create the pools before tracing is enabled, so that the events this test + cares about are the allocations rather than the creates. */ + status = tx_block_pool_create(&block_pool_0, "trace entry update block pool", + 20, block_pool_area, sizeof(block_pool_area)); + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #1\n"); + test_control_return(1); + } + + status = tx_byte_pool_create(&byte_pool_0, "trace entry update byte pool", + byte_pool_area, sizeof(byte_pool_area)); + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #2\n"); + test_control_return(1); + } + + /* Turn tracing on. This is the whole point of the test: without it + _tx_trace_buffer_current_ptr stays TX_NULL and every update block below + takes its guarding false branch. */ + _tx_trace_initialize(); + status = tx_trace_enable(trace_buffer, sizeof(trace_buffer), 16); + +#ifdef TX_ENABLE_EVENT_TRACE + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #3\n"); + test_control_return(1); + } +#else + + /* Four of the five build configurations reach this branch. The rest of the + test still runs; it just is not exercising any update block. */ + if (status != TX_FEATURE_NOT_ENABLED) + { + + printf("Running Trace Entry Update Test..................................... ERROR #4\n"); + test_control_return(1); + } +#endif + + /* Register an object with no name at all, now that tracing is on. The name + copy in _tx_trace_object_register guards against a TX_NULL name and + breaks out of the copy loop, and nothing else in the suite takes that + branch with tracing enabled -- threadx_trace_basic_test makes the + equivalent call only under #ifndef TX_ENABLE_EVENT_TRACE, against the + no-op stub. + + A semaphore rather than a thread, deliberately: for a thread object the + register function dereferences the pointer it is handed to read the + thread's priority, so that type needs a real TX_THREAD behind it. */ + status = tx_semaphore_create(&semaphore_0, TX_NULL, 1); + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #5\n"); + test_control_return(1); + } + + /* Thread 0 drives the test and must run first, so it takes the higher + priority -- the lower number. */ + status = tx_thread_create(&thread_0, "thread 0", thread_0_entry, 0, + pointer, TEST_STACK_SIZE_PRINTF, + 15, 15, TX_NO_TIME_SLICE, TX_AUTO_START); + pointer = pointer + TEST_STACK_SIZE_PRINTF; + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #6\n"); + test_control_return(1); + } + + /* Thread 1 releases what thread 0 waits on. It runs only while thread 0 is + suspended, which is exactly when it is needed. */ + status = tx_thread_create(&thread_1, "thread 1", thread_1_entry, 0, + pointer, TEST_STACK_SIZE_PRINTF, + 16, 16, TX_NO_TIME_SLICE, TX_AUTO_START); + pointer = pointer + TEST_STACK_SIZE_PRINTF; + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #7\n"); + test_control_return(1); + } +} + + +static void thread_0_entry(ULONG thread_input) +{ + +UINT status; +void *block_ptr; +void *byte_ptr; +UINT i; + + + /* Empty the block pool. The first of these takes the immediate-success path + through tx_block_allocate, which carries the first update block. */ + held_block = TX_NULL; + for (i = 0; i < 4; i++) + { + + status = tx_block_allocate(&block_pool_0, &block_ptr, TX_NO_WAIT); + + if (status != TX_SUCCESS) + { + + error++; + break; + } + + /* Keep the first one for thread 1 to give back. */ + if (held_block == TX_NULL) + { + + held_block = block_ptr; + } + } + + /* The pool is empty now, so this suspends. It completes in thread 1's + context when the block comes back, which is the second update block in + tx_block_allocate -- and the suspend and resume it goes through carry the + update blocks in tx_thread_system_suspend and tx_thread_system_resume. */ + status = tx_block_allocate(&block_pool_0, &block_ptr, TX_WAIT_FOREVER); + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #8\n"); + test_control_return(1); + } + + /* Now the same shape for the byte pool. Take most of it first, on the path + that succeeds immediately. */ + status = tx_byte_allocate(&byte_pool_0, &held_bytes, 300, TX_NO_WAIT); + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #9\n"); + test_control_return(1); + } + + /* Not enough left, so this one suspends and is satisfied by thread 1's + release. */ + status = tx_byte_allocate(&byte_pool_0, &byte_ptr, 300, TX_WAIT_FOREVER); + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #10\n"); + test_control_return(1); + } + + /* Sleep, so that thread 1 gets to run out its entry function and suspend + itself. That leaves nothing ready to execute, which is a third update + block in tx_thread_system_suspend -- the one on the path that sets + _tx_thread_execute_ptr to TX_NULL. The two suspensions above never reach + it, because the other thread was always ready to take over. */ + status = tx_thread_sleep(5); + + if (status != TX_SUCCESS) + { + + printf("Running Trace Entry Update Test..................................... ERROR #11\n"); + test_control_return(1); + } + + if (error) + { + + printf("Running Trace Entry Update Test..................................... ERROR #12\n"); + test_control_return(1); + } + else + { + + printf("Running Trace Entry Update Test..................................... SUCCESS!\n"); + test_control_return(0); + } +} + + +static void thread_1_entry(ULONG thread_input) +{ + +UINT status; + + + /* Reached only when thread 0 has suspended on the empty block pool. */ + status = tx_block_release(held_block); + + if (status != TX_SUCCESS) + { + + error++; + } + + /* And again when thread 0 has suspended on the byte pool. Thread 0 preempts + on the release above, runs until it suspends a second time, and control + comes back here. */ + status = tx_byte_release(held_bytes); + + if (status != TX_SUCCESS) + { + + error++; + } + + /* Thread 0 reports the result; this thread has nothing left to do. */ + tx_thread_suspend(&thread_1); +}