Guarded the stack analyze binary search against a highest stack pointer that is not above the start of the stack, so a stack overflow or a corrupted control block no longer hangs or crashes the caller - #727
Open
fdesbiens wants to merge 1 commit into
Conversation
…er that is not above the start of the stack, so a stack overflow or a corrupted control block no longer hangs or crashes the caller _tx_thread_stack_analyze computed the midpoint of the remaining stack with TX_ULONG_POINTER_DIF, which casts the pointer difference to ULONG. When tx_thread_stack_highest_ptr had already moved below tx_thread_stack_start, the difference was negative and wrapped to a huge unsigned value, so the binary search probed far outside the stack and never converged. The highest stack pointer is now required to be above the start of the stack before the search begins, and the final scan for the first used word is bounded by the initial highest stack pointer so it cannot run past the end of the region either. Added regression coverage for an inverted and for an equal pair of stack pointers. Without the fix the new test case crashes the suite. Assisted-by: Copilot (Opus 5) <noreply@github.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
1 participant
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.
Fixes #460
_tx_thread_stack_analyzecomputes the midpoint of the remaining stack withTX_ULONG_POINTER_DIF, which casts the pointer difference toULONG. Whentx_thread_stack_highest_ptrhas already moved belowtx_thread_stack_start, that difference is negative and wraps to a huge unsigned value. The probe pointer then lands far outside the stack, and the search never converges: the caller either hangs or faults, depending on what the out-of-range address maps to.PR #464 stopped
TX_THREAD_STACK_CHECKfrom calling into the analysis in that state, which covers the normal path. This change adds the safeguard inside_tx_thread_stack_analyzeitself, as suggested by @billlamiework in the issue, so the function is also safe when it is called directly.The highest stack pointer is now required to be strictly above the start of the stack before the binary search begins. The final scan for the first used word is also bounded by the initial highest stack pointer, so it cannot run past the end of the region if the fill pattern is intact all the way up.
Regression coverage was added for an inverted pair of stack pointers and for an equal pair. Without the source change the new test case crashes the suite, so it genuinely reproduces the defect.
This does not address why the pointers end up inconsistent in the first place. That is either a real stack overflow or memory corruption of the
TX_THREADcontrol block, and it remains the application's problem to diagnose. What changes is that ThreadX now reports it through the stack error handler instead of hanging.PR checklist