Hi all,
I have detected the armv{6,7,8}-m/arm_vectors.c assigns the initial
stack pointer value (using the first entry at the exception vectors
table) just to _ebss + CONFIG_IDLETHREAD_STACKSIZE, without further
checking. This can yield to an unusable binary if .data or .bss
sections grow too much.
To test, use the attached patch 0001 and ./tools/configure.sh
lm3s6965-ek:qemu-flat
(Note with stacks just below SRAM limit the NSH prompt will still not
be visible because booting will fail during heap allocation, but still
the "ABCDF" booting markers will be shown; when the stack starting
point is past the SRAM end, booting fails immediately and those
markers are not shown).
There are multiple ways to fix (= detect this at build time) this
problem. The best IMHO is to make the stack space appear at the memory
map and be allocated. This will cause a region overflow error during
linking when the initial stack overflows SRAM (as intended). For a
demo of this fix (for lm3s6965-ek:qemu-flat only) see attached patch
0002.
Unfortunately, this requires linker script changes for most platforms.
Without the changes, the initial stack would be correctly allocated
(ld would just map ".stack" section to any writable memory region) but
several other parts of the code assume the heap to be just after the
idle thread stack, which cannot be guaranteed without the
aforementioned linker script changes.
So I want to ask here for opinion before progressing further, as a
change this deep would affect a lot of platforms which I do not have
the means to test.
WDYT?
Carlos
From 0dc292a1fa17c9776784e1d4c648bf225ee79710 Mon Sep 17 00:00:00 2001
From: Carlos Sanchez <carlossanchez@geotab.com>
Date: Wed, 2 Nov 2022 11:59:49 +0100
Subject: [PATCH 1/2] Changes to force fill bss section causing sp past the
SRAM end.
---
arch/arm/src/armv7-m/arm_vectors.c | 12 ++++++++++++
boards/arm/tiva/lm3s6965-ek/scripts/ld.script | 1 +
2 files changed, 13 insertions(+)
diff --git a/arch/arm/src/armv7-m/arm_vectors.c b/arch/arm/src/armv7-m/arm_vectors.c
index 36bdfb8075..ca86f2a58a 100644
--- a/arch/arm/src/armv7-m/arm_vectors.c
+++ b/arch/arm/src/armv7-m/arm_vectors.c
@@ -90,3 +90,15 @@ unsigned _vectors[] locate_data(".vectors") =
[2 ... (15 + ARMV7M_PERIPHERAL_INTERRUPTS)] = (unsigned)&exception_common
};
+
+/* Stack initialized to 0x20010000 */
+/* static volatile char fill[49160]; */
+
+/* Stack initialized to 0x20010008, still works because
+ Dump of assembler code for function __start:
+ => 0x0000011c <+0>: push {r3, lr}
+ which are never pop'ed. */
+/* static volatile char fill[49168]; */
+
+/* Stack initialized to 0x20010010 */
+static volatile char fill[49176];
diff --git a/boards/arm/tiva/lm3s6965-ek/scripts/ld.script b/boards/arm/tiva/lm3s6965-ek/scripts/ld.script
index 15cc25511f..f16f4588d7 100644
--- a/boards/arm/tiva/lm3s6965-ek/scripts/ld.script
+++ b/boards/arm/tiva/lm3s6965-ek/scripts/ld.script
@@ -83,6 +83,7 @@ SECTIONS
*(.bss .bss.*)
*(.gnu.linkonce.b.*)
*(COMMON)
+ KEEP(*(.bss.fill))
. = ALIGN(4);
_ebss = ABSOLUTE(.);
} > sram
--
2.25.1
From 958993fb56fb22cd1d4af94645cb570adb3579e5 Mon Sep 17 00:00:00 2001
From: Carlos Sanchez <carlossanchez@geotab.com>
Date: Wed, 2 Nov 2022 13:27:59 +0100
Subject: [PATCH 2/2] Allocate stack in the memory map.
---
arch/arm/src/armv7-m/arm_vectors.c | 8 +++++++-
boards/arm/tiva/lm3s6965-ek/scripts/ld.script | 4 ++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm/src/armv7-m/arm_vectors.c b/arch/arm/src/armv7-m/arm_vectors.c
index ca86f2a58a..86d7fd6496 100644
--- a/arch/arm/src/armv7-m/arm_vectors.c
+++ b/arch/arm/src/armv7-m/arm_vectors.c
@@ -45,12 +45,18 @@
* Pre-processor Definitions
****************************************************************************/
-#define IDLE_STACK ((unsigned)&_ebss+CONFIG_IDLETHREAD_STACKSIZE)
+#define IDLE_STACK ((unsigned) idle_stack + sizeof(idle_stack))
#ifndef ARMV7M_PERIPHERAL_INTERRUPTS
# error ARMV7M_PERIPHERAL_INTERRUPTS must be defined to the number of I/O interrupts to be supported
#endif
+/****************************************************************************
+ * Private data
+ ****************************************************************************/
+
+static char idle_stack[CONFIG_IDLETHREAD_STACKSIZE] locate_data(".stack");
+
/****************************************************************************
* Public Functions
****************************************************************************/
diff --git a/boards/arm/tiva/lm3s6965-ek/scripts/ld.script b/boards/arm/tiva/lm3s6965-ek/scripts/ld.script
index f16f4588d7..1f13773591 100644
--- a/boards/arm/tiva/lm3s6965-ek/scripts/ld.script
+++ b/boards/arm/tiva/lm3s6965-ek/scripts/ld.script
@@ -88,6 +88,10 @@ SECTIONS
_ebss = ABSOLUTE(.);
} > sram
+ .stack (NOLOAD) : ALIGN(4) {
+ *(.stack)
+ } > sram
+
/* Stabs debugging sections. */
.stab 0 : { *(.stab) }
--
2.25.1
