From 0565b7c0e050f0cccbccdcbc1a9cee43462603d9 Mon Sep 17 00:00:00 2001
From: Ayke van Laethem <aykevanlaethem@gmail.com>
Date: Mon, 5 Jul 2021 01:50:52 +0200
Subject: [PATCH] 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.
---
 builder/build.go | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/builder/build.go b/builder/build.go
index 8a433ca2..f17d26c7 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.