builder: increase stack size margin for automatic stack allocation#5381
builder: increase stack size margin for automatic stack allocation#5381digitalentity wants to merge 1 commit into
Conversation
Adjust the stack size margin to account for the tinygo_swapTask overhead.
There was a problem hiding this comment.
Pull request overview
This PR improves TinyGo’s automatic goroutine stack size calculation by adding a safety margin for context switches, accounting for stack usage in tinygo_swapTask that is not visible to Go-level call graph analysis.
Changes:
- Look up
tinygo_swapTaskin the computed call graph/symbol map and capture its frame size as context-switch overhead. - Add the context-switch overhead to all goroutine stack sizes that are compile-time Bounded.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // on every context switch. The static analysis correctly traces Go calls, | ||
| // but it cannot see into the assembly-level register push. | ||
| var contextSwitchOverhead uint64 | ||
| if swapFuncs, ok := functions["tinygo_swapTask"]; ok && len(swapFuncs) == 1 { |
aykevl
left a comment
There was a problem hiding this comment.
This is the wrong fix. The stack size of the goroutine swap functions should be taken care of already, since the assembly lists it as part of the CFA:
If it isn't taken into account, that's a different bug.
|
@aykevl the bug is not that the |
|
In #5375 (comment) I've figured out that the crashes are actually happening because of the memory corruption which sometimes gets caught by the canary, but sometimes is silent. With the increase of the autodetected stack size by 36 bytes ( |
|
Ah sorry, I misread the PR. Will take another look later! |
When TinyGo calculates stack sizes automatically, it performs a static analysis of the call graph. While this analysis correctly identifies the stack requirements for Go functions, it cannot account for stack usage within assembly-level routines that are not visible to the high-level analysis.
Specifically,
tinygo_swapTaskpushes callee-saved registers onto the current goroutine's stack during every context switch. If this overhead is not accounted for, a goroutine might overflow its stack during a context switch even if its Go-level stack usage is within limits.With this change in
determineStackSizes, we now look up thetinygo_swapTaskfunction in the symbol table to determine its frame size. This context switch overhead is now added to the calculated stack size for all goroutines that have a bounded stack size.Fixes #5375