Skip to content

Commit

Permalink
arch/stack_color: correct the end address of stack color
Browse files Browse the repository at this point in the history
The different optimization of compilers will cause ambiguity in
obtaining sp through up_getsp() in arm_stack_color(), if compile
with clang and enable the optimization flag (-Ofast), up_getsp()
call will be earlier than push {r0-r9,lr}, the end address of color
stack will overlap with saved registers.

Compile line:
clang --target=arm-none-eabi -c "-Ofast" -fno-builtin -march=armv8.1-m.main+mve.fp+fp.dp \
-mtune=cortex-m55 -mthumb -mfpu=fpv5-d16 -mfloat-abi=hard -D__NuttX__ -common/arm_checkstack.c -o  arm_checkstack.o

Assembler code:
llvm-objdump -aS arm_checkstack.o
------------------------------------
|00000000 <arm_stack_color>:
|;   start = INT32_ALIGN_UP((uintptr_t)stackbase);
|       0: c2 1c         adds  r2, r0, #3
|       2: 22 f0 03 02   bic r2, r2, #3
|;   end   = nbytes ? INT32_ALIGN_DOWN((uintptr_t)stackbase + nbytes) :
|       6: 19 b1         cbz r1, 0x10 <arm_stack_color+0x10> @ imm = #6
|       8: 08 44         add r0, r1
|       a: 20 f0 03 00   bic r0, r0, #3
|       e: 00 e0         b 0x12 <arm_stack_color+0x12> @ imm = #0
|;   __asm__
|      10: 68 46         mov r0, sp                               <--- fetch the sp before push {r7 lr}
|      12: 80 b5         push  {r7, lr}                           <--- sp changed
|;   nwords = (end - start) >> 2;
|      14: 80 1a         subs  r0, r0, r2
|      16: 80 08         lsrs  r0, r0, #2
|; }
|      18: 08 bf         it  eq
|      1a: 80 bd         popeq {r7, pc}
|      1c: 4b f6 ef 63   movw  r3, #48879
|      20: cd f6 ad 63   movt  r3, #57005
|      24: a0 ee 10 3b   vdup.32 q0, r3
|;   while (nwords-- > 0)
|      28: 20 f0 01 e0   dlstp.32  lr, r0
|;       *ptr++ = STACK_COLOR;                                    <--- overwrite
|      2c: a2 ec 04 1f   vstrw.32  q0, [r2], #16
|      30: 1f f0 05 c0   letp  lr, 0x2c <arm_stack_color+0x2c> @ imm = #-8
|; }
|      34: 80 bd         pop {r7, pc}
------------------------------------

Signed-off-by: chao.an <[email protected]>
  • Loading branch information
anchao authored and xiaoxiang781216 committed Apr 13, 2022
1 parent df5a8a5 commit ff210e1
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 16 deletions.
3 changes: 2 additions & 1 deletion arch/arm/src/common/arm_checkstack.c
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,13 @@ void arm_stack_color(FAR void *stackbase, size_t nbytes)
uintptr_t end;
size_t nwords;
FAR uint32_t *ptr;
uintptr_t sp;

/* Take extra care that we do not write outside the stack boundaries */

start = INT32_ALIGN_UP((uintptr_t)stackbase);
end = nbytes ? INT32_ALIGN_DOWN((uintptr_t)stackbase + nbytes) :
up_getsp(); /* 0: colorize the running stack */
(uintptr_t)&sp; /* 0: colorize the running stack */

/* Get the adjusted size based on the top and bottom of the stack */

Expand Down
13 changes: 9 additions & 4 deletions arch/ceva/src/common/up_createstack.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,15 @@ void up_stack_color(FAR void *stackbase, size_t nbytes)
{
/* Take extra care that we do not write outsize the stack boundaries */

uint32_t *stkptr = (uint32_t *)(((uintptr_t)stackbase + 3) & ~3);
uintptr_t stkend = nbytes ? (((uintptr_t)stackbase + nbytes) & ~3) :
up_getsp(); /* 0: colorize the running stack */
size_t nwords = (stkend - (uintptr_t)stackbase) >> 2;
uint32_t *stkptr;
uintptr_t stkend;
size_t nwords;
uintptr_t sp;

stkptr = (uint32_t *)(((uintptr_t)stackbase + 3) & ~3);
stkend = nbytes ? (((uintptr_t)stackbase + nbytes) & ~3) :
(uintptr_t)&sp; /* 0: colorize the running stack */
nwords = (stkend - (uintptr_t)stackbase) >> 2;

/* Set the entire stack to the coloration value */

Expand Down
13 changes: 9 additions & 4 deletions arch/or1k/src/common/up_createstack.c
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,15 @@ void up_stack_color(FAR void *stackbase, size_t nbytes)
{
/* Take extra care that we do not write outsize the stack boundaries */

uint32_t *stkptr = (uint32_t *)(((uintptr_t)stackbase + 3) & ~3);
uintptr_t stkend = nbytes ? (((uintptr_t)stackbase + nbytes) & ~3) :
up_getsp(); /* 0: colorize the running stack */
size_t nwords = (stkend - (uintptr_t)stackbase) >> 2;
uint32_t *stkptr;
uintptr_t stkend;
size_t nwords;
uintptr_t sp;

stkptr = (uint32_t *)(((uintptr_t)stackbase + 3) & ~3);
stkend = nbytes ? (((uintptr_t)stackbase + nbytes) & ~3) :
(uintptr_t)&sp; /* 0: colorize the running stack */
nwords = (stkend - (uintptr_t)stackbase) >> 2;

/* Set the entire stack to the coloration value */

Expand Down
3 changes: 2 additions & 1 deletion arch/risc-v/src/common/riscv_createstack.c
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,13 @@ void riscv_stack_color(void *stackbase, size_t nbytes)
uintptr_t end;
size_t nwords;
uint32_t *ptr;
uintptr_t sp;

/* Take extra care that we do not write outside the stack boundaries */

start = STACK_ALIGN_UP((uintptr_t)stackbase);
end = nbytes ? STACK_ALIGN_DOWN((uintptr_t)stackbase + nbytes) :
up_getsp(); /* 0: colorize the running stack */
(uintptr_t)&sp; /* 0: colorize the running stack */

/* Get the adjusted size based on the top and bottom of the stack */

Expand Down
13 changes: 9 additions & 4 deletions arch/sim/src/sim/up_createstack.c
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,15 @@ void nostackprotect_function up_stack_color(FAR void *stackbase,
{
/* Take extra care that we do not write outsize the stack boundaries */

uint32_t *stkptr = (uint32_t *)(((uintptr_t)stackbase + 3) & ~3);
uintptr_t stkend = nbytes ? (((uintptr_t)stackbase + nbytes) & ~3) :
up_getsp(); /* 0: colorize the running stack */
size_t nwords = (stkend - (uintptr_t)stackbase) >> 2;
uint32_t *stkptr;
uintptr_t stkend;
size_t nwords;
uintptr_t sp;

stkptr = (uint32_t *)(((uintptr_t)stackbase + 3) & ~3);
stkend = nbytes ? (((uintptr_t)stackbase + nbytes) & ~3) :
(uintptr_t)&sp; /* 0: colorize the running stack */
nwords = (stkend - (uintptr_t)stackbase) >> 2;

/* Set the entire stack to the coloration value */

Expand Down
3 changes: 2 additions & 1 deletion arch/sparc/src/common/up_checkstack.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,13 @@ void up_stack_color(FAR void *stackbase, size_t nbytes)
uintptr_t end;
size_t nwords;
FAR uint32_t *ptr;
uintptr_t sp;

/* Take extra care that we do not write outside the stack boundaries */

start = INT32_ALIGN_UP((uintptr_t)stackbase);
end = nbytes ? INT32_ALIGN_DOWN((uintptr_t)stackbase + nbytes) :
up_getsp(); /* 0: colorize the running stack */
(uintptr_t)&sp; /* 0: colorize the running stack */

/* Get the adjusted size based on the top and bottom of the stack */

Expand Down
3 changes: 2 additions & 1 deletion arch/xtensa/src/common/xtensa_createstack.c
Original file line number Diff line number Diff line change
Expand Up @@ -223,12 +223,13 @@ void xtensa_stack_color(void *stackbase, size_t nbytes)
uintptr_t end;
size_t nwords;
uint32_t *ptr;
uintptr_t sp;

/* Take extra care that we do not write outside the stack boundaries */

start = STACK_ALIGN_UP((uintptr_t)stackbase);
end = nbytes ? STACK_ALIGN_DOWN((uintptr_t)stackbase + nbytes) :
up_getsp(); /* 0: colorize the running stack */
(uintptr_t)&sp; /* 0: colorize the running stack */

/* Get the adjusted size based on the top and bottom of the stack */

Expand Down

0 comments on commit ff210e1

Please sign in to comment.