diff options
author | Ayke van Laethem <[email protected]> | 2021-07-05 01:50:52 +0200 |
---|---|---|
committer | Ron Evans <[email protected]> | 2021-07-07 09:25:47 +0200 |
commit | 0565b7c0e050f0cccbccdcbc1a9cee43462603d9 (patch) | |
tree | 09664d17ef72c5cb79cb76055fb585af86cb75c6 | |
parent | 444dded92cfaf870be3a590e13419af49f1192b0 (diff) | |
download | tinygo-0565b7c0e050f0cccbccdcbc1a9cee43462603d9.tar.gz tinygo-0565b7c0e050f0cccbccdcbc1a9cee43462603d9.zip |
cortexm: fix stack overflow because of unaligned stacks
On ARM, the stack has to be aligned to 8 bytes on function calls, but
not necessarily within a function. Leaf functions can take advantage of
this by not keeping the stack aligned so they can avoid pushing one
register. However, because regular functions might expect an aligned
stack, the interrupt controller will forcibly re-align the stack when an
interrupt happens in such a leaf function (controlled by the STKALIGN
flag, defaults to on). This means that stack size calculation (as used
in TinyGo) needs to make sure this extra space for stack re-alignment is
available.
This commit fixes this by aligning the stack size that will be used for
new goroutines.
Additionally, it increases the stack canary size from 4 to 8 bytes, to
keep the stack aligned. This is not strictly necessary but is required
by the AAPCS so let's do it anyway just to be sure.
-rw-r--r-- | builder/build.go | 24 |
1 files changed, 18 insertions, 6 deletions
diff --git a/builder/build.go b/builder/build.go index 8a433ca24..f17d26c72 100644 --- a/builder/build.go +++ b/builder/build.go @@ -953,15 +953,19 @@ func modifyStackSizes(executable string, stackSizeLoads []string, stackSizes map if fn.stackSizeType == stacksize.Bounded { stackSize := uint32(fn.stackSize) - // Adding 4 for the stack canary. Even though the size may be - // automatically determined, stack overflow checking is still - // important as the stack size cannot be determined for all - // goroutines. - stackSize += 4 - // Add stack size used by interrupts. switch fileHeader.Machine { case elf.EM_ARM: + if stackSize%8 != 0 { + // If the stack isn't a multiple of 8, it means the leaf + // function with the biggest stack depth doesn't have an aligned + // stack. If the STKALIGN flag is set (which it is by default) + // the interrupt controller will forcibly align the stack before + // storing in-use registers. This will thus overwrite one word + // past the end of the stack (off-by-one). + stackSize += 4 + } + // On Cortex-M (assumed here), this stack size is 8 words or 32 // bytes. This is only to store the registers that the interrupt // may modify, the interrupt will switch to the interrupt stack @@ -969,6 +973,14 @@ func modifyStackSizes(executable string, stackSizeLoads []string, stackSizes map // Some background: // https://interrupt.memfault.com/blog/cortex-m-rtos-context-switching stackSize += 32 + + // Adding 4 for the stack canary, and another 4 to keep the + // stack aligned. Even though the size may be automatically + // determined, stack overflow checking is still important as the + // stack size cannot be determined for all goroutines. + stackSize += 8 + default: + return fmt.Errorf("unknown architecture: %s", fileHeader.Machine.String()) } // Finally write the stack size to the binary. |