Made the stack analyze binary search require several consecutive fill words, so unwritten holes in a used stack no longer cause the highest stack pointer to be under-reported - #720
Merged
Conversation
… words, so unwritten holes in a used stack no longer cause the highest stack pointer to be under-reported The binary search in _tx_thread_stack_analyze() accepted a probe location as unused as soon as a single word still held TX_STACK_FILL. A word inside an otherwise used region that simply was never written - the padding of a partially initialized local array, for example - therefore made the search move away from the real boundary and report far less stack usage than the thread had actually consumed, which in turn kept the stack guard from firing. The probe now walks down from the candidate location and requires TX_THREAD_STACK_ANALYZE_FILL_WORDS consecutive fill words before it treats the location as unused, stopping early at the lowest location already known to hold the fill pattern. The new macro defaults to eight words and can be overridden in tx_port.h; setting it to one restores the previous behavior. Holes shorter than the configured run no longer mislead the search, and the result is unchanged for stacks that contain no such holes. 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
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 #425
_tx_thread_stack_analyze()binary-searches the stack for the boundary between the untouched fill pattern and the region the thread has actually used. The probe accepted a location as unused as soon as a single word still heldTX_STACK_FILL. A word inside an otherwise used region that simply was never written — the padding of a partially initialized local array, for example — therefore sent the search away from the real boundary, andtx_thread_stack_highest_ptrended up reporting far less usage than the thread had really consumed. As the reporter observed, that also keeps the stack guard from firing when it should.The probe now walks down from the candidate location and requires
TX_THREAD_STACK_ANALYZE_FILL_WORDSconsecutive fill words before it treats the location as unused. It stops early at the lowest location already known to hold the fill pattern, so locations near the bottom of the stack behave correctly. The new macro defaults to eight words and can be overridden intx_port.h; setting it to one restores the previous behavior exactly.This is the approach @billlamiework proposed in the issue thread. It remains a heuristic — a hole at least as long as the configured run can still mislead the search — but it removes the common single-word and short-hole cases at a cost of a handful of extra reads per binary search iteration.
The same change is applied to the SMP copy of the file, which was byte-identical to the non-SMP one.
Validation: an offline model of both algorithms confirms that a six-word hole placed on a probe location makes the current code report 88 words less usage than the thread actually used, while the new code lands on the true boundary; results are identical for stacks with no holes. The regression suites pass unchanged: 98/98 for
test/txand 114/114 fortest/smp.