Stopped the SMP stack check analyzing a stack it just found broken - #648
Merged
fdesbiens merged 1 commit intoAug 20, 2026
Merged
Conversation
TX_THREAD_STACK_CHECK detects a broken stack, calls the error handler, and then
tests whether the word below the high-water mark still holds the fill pattern.
On the SMP side that second test is a plain if, so a thread whose stack has
just been reported as corrupt goes straight on into _tx_thread_stack_analyze().
Analyzing a stack that is known to be broken is what that function is least
able to do. It binary searches between stack_lowest and stack_highest for the
fill pattern and then scans forward with
while (*stack_ptr == TX_STACK_FILL)
which has no bound of its own and no reason to terminate once the pattern it is
looking for is no longer where the pointers say it should be. The non-SMP copy
was given an else for exactly this reason. The SMP copy never was, and the two
macros are otherwise identical, line for line, so this single keyword was the
whole of the divergence.
The path is live in CI rather than theoretical. Instrumenting the internal
handler and running all 110 binaries of stack_checking_build shows
threadx_thread_stack_checking_test reaching it four times per run, on a thread
whose stack the test corrupts on purpose. Every one of those four currently
falls through into the analyze it should be skipping.
This is not the timeout the SMP suite has been failing on.
threadx_thread_priority_change never reaches the error handler at all, so
whatever wedges it in teardown is something else. Worth closing regardless: a
runaway scan inside stack analysis would present as a test that stops producing
output and is eventually killed, which is the shape that has been costing this
suite whole runs, and it would be indistinguishable in the log from the hang
already being chased.
Verified on both configurations that define TX_ENABLE_STACK_CHECKING.
threadx_thread_stack_checking_test, the one test that exercises the changed
branch, passes 60 consecutive runs, and stack_checking_build and
stack_checking_rand_fill_build both pass 110 of 110, repeated at the
parallelism CI uses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TX_THREAD_STACK_CHECKdetects a broken stack, calls the error handler, and then tests whether the word below the high-water mark still holds the fill pattern. On the SMP side that second test is a plainif:So a thread whose stack has just been reported as corrupt goes straight on into
_tx_thread_stack_analyze().Why that is the wrong function to reach in that state
Analyzing a stack that is known to be broken is what
_tx_thread_stack_analyze()is least able to do. It binary searches betweenstack_lowestandstack_highestfor the fill pattern and then scans forward withwhich has no bound of its own and no reason to terminate once the pattern it is looking for is no longer where the pointers say it should be.
The non-SMP copy of the macro was given an
elsefor exactly this reason. The SMP copy never was, and the two macros are otherwise identical, line for line — this single keyword was the whole of the divergence.The path is live, not theoretical
Instrumenting the internal handler and running all 110 binaries of
stack_checking_buildshowsthreadx_thread_stack_checking_testreaching it four times per run, on a thread whose stack the test corrupts on purpose. Every one of those four currently falls through into the analyze it should be skipping.This is not the SMP timeout
Worth stating plainly, since it would be easy to file this as the cause of the red suite.
threadx_thread_priority_changenever reaches the error handler at all, so whatever wedges it in teardown is something else (#647).It is worth closing regardless. A runaway scan inside stack analysis would present as a test that stops producing output and is eventually killed — the shape that has been costing this suite whole runs — and it would be indistinguishable in the log from the hang already being chased.
Verification
On both configurations that define
TX_ENABLE_STACK_CHECKING:threadx_thread_stack_checking_test, the one test that exercises the changed branch, passes 60 consecutive runsstack_checking_buildandstack_checking_rand_fill_buildboth pass 110 of 110, repeated at the parallelism CI uses