Revert "arch: Allocate the space from the beginning in up_stack_frame"

This reverts commit 2335b69. It seems that the commit is question broke sim/Linux and sim/macOS. Both of the following crashes are fixed by this revert. My app running with sim/Linux started crashing with the commit. ``` Program received signal SIGSEGV, Segmentation fault. 0x00000000004583ad in snprintf (buf=0x7f6260682b30 "\020", size=140060500962096, format=0xffffffffffffffff <error: Cannot access memory at address 0xffffffffffffffff>) at stdio/lib_snprintf.c:41 41 stdio/lib_snprintf.c: No such file or directory. rax 0x0 0 rbx 0x0 0 rcx 0x1 1 rdx 0x5515d0 5576144 rsi 0x10 16 rdi 0x7f6260682858 140060500961368 rbp 0x7f6260682808 0x7f6260682808 rsp 0x7f6260682628 0x7f6260682628 r8 0x7f62606825e0 140060500960736 r9 0x0 0 r10 0x8 8 r11 0x246 582 r12 0x0 0 r13 0x0 0 r14 0x0 0 r15 0x0 0 rip 0x4583ad 0x4583ad <snprintf+13> eflags 0x10246 [ PF ZF IF RF ] cs 0x33 51 ss 0x2b 43 ds 0x0 0 es 0x0 0 fs 0x0 0 gs 0x0 0 ``` sim:ostest on macOS crashes like the following. ``` spacetanuki% lldb ./nuttx (lldb) target create "./nuttx" Current executable set to './nuttx' (x86_64). (lldb) run Process 67434 launched: '/Users/yamamoto/git/nuttx/nuttx/nuttx' (x86_64) Process 67434 stopped * thread apache#1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT) frame #0: 0x00007fff6f1633a6 libdyld.dylib`stack_not_16_byte_aligned_error libdyld.dylib`stack_not_16_byte_aligned_error: -> 0x7fff6f1633a6 <+0>: movdqa %xmm0, (%rsp) 0x7fff6f1633ab <+5>: int3 libdyld.dylib`_dyld_fast_stub_entry: 0x7fff6f1633ac <+0>: pushq %rbp 0x7fff6f1633ad <+1>: movq %rsp, %rbp Target 0: (nuttx) stopped. (lldb) bt * thread apache#1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT) * frame #0: 0x00007fff6f1633a6 libdyld.dylib`stack_not_16_byte_aligned_error frame apache#1: 0x0000000101002048 frame apache#2: 0x000000010001682d nuttx`tty_send(dev=0x000000010002f370, ch=115) at up_uart.c:447:3 frame apache#3: 0x000000010000d7df nuttx`uart_xmitchars(dev=0x000000010002f370) at serial_io.c:68:7 frame apache#4: 0x0000000100016a95 nuttx`tty_txint(dev=0x000000010002f370, enable='\x01') at up_uart.c:462:7 frame apache#5: 0x000000010000ce48 nuttx`uart_write(filep=0x00000001010011e8, buffer="", buflen=0) at serial.c:1260:7 frame apache#6: 0x0000000100024ef3 nuttx`file_write(filep=0x00000001010011e8, buf=0x0000000100027a30, nbytes=23) at fs_write.c:89:10 frame apache#7: 0x0000000100024f6a nuttx`nx_write(fd=1, buf=0x0000000100027a30, nbytes=23) at fs_write.c:138:13 frame apache#8: 0x0000000100024fab nuttx`file_write(filep=0x0000000100027a30, buf=0x0000000000000017, nbytes=0) at fs_write.c:76:7 frame apache#9: 0x000000010002215e nuttx`stdio_test at ostest_main.c:574:3 frame apache#10: 0x0000000100021f1b nuttx`ostest_main(argc=1, argv=0x0000000101001300) at ostest_main.c:602:3 frame apache#11: 0x000000010000ff05 nuttx`nxtask_startup(entrypt=(nuttx`ostest_main at ostest_main.c:592), argc=1, argv=0x0000000101001300) at task_startup.c:150:8 frame apache#12: 0x000000010000a580 nuttx`nxtask_start at task_start.c:129:7 (lldb) ```
yamt · Apr 16, 2021 · be752f0 · be752f0
1 parent 2335b69
commit be752f0
Show file tree

Hide file tree

Showing 155 changed files with 1,710 additions and 1,074 deletions.
diff --git a/Documentation/reference/os/arch.rst b/Documentation/reference/os/arch.rst
@@ -57,8 +57,8 @@ APIs Exported by Architecture-Specific Logic to NuttX
   -  ``adj_stack_size``: Stack size after adjustment for hardware,
      processor, etc. This value is retained only for debug purposes.
   -  ``stack_alloc_ptr``: Pointer to allocated stack
-  -  ``stack_base_ptr``: Adjusted stack base pointer after the TLS Data
-     and Arguments has been removed from the stack allocation.
+  -  ``adj_stack_ptr``: Adjusted ``stack_alloc_ptr`` for HW. The
+     initial value of the stack pointer.
 
   :param tcb: The TCB of new task.
   :param stack_size: The requested stack size. At least this much
@@ -93,8 +93,8 @@ APIs Exported by Architecture-Specific Logic to NuttX
   -  ``adj_stack_size``: Stack size after adjustment for hardware,
      processor, etc. This value is retained only for debug purposes.
   -  ``stack_alloc_ptr``: Pointer to allocated stack
-  -  ``stack_base_ptr``: Adjusted stack base pointer after the TLS Data
-     and Arguments has been removed from the stack allocation.
+  -  ``adj_stack_ptr``: Adjusted ``stack_alloc_ptr`` for HW. The
+     initial value of the stack pointer.
 
   :param tcb: The TCB of new task.
   :param stack_size: The allocated stack size.
@@ -120,24 +120,9 @@ APIs Exported by Architecture-Specific Logic to NuttX
 
   -  ``adj_stack_size``: Stack size after removal of the stack frame
      from the stack.
-  -  ``stack_base_ptr``: Adjusted stack base pointer after the TLS Data
-     and Arguments has been removed from the stack allocation.
-
-  Here is the diagram after some allocation(tls, arg):
-
-                   +-------------+ <-stack_alloc_ptr(lowest)
-                   |  TLS Data   |
-                   +-------------+
-                   |  Arguments  |
-  stack_base_ptr-> +-------------+\
-                   |  Available  | +
-                   |    Stack    | |
-                |  |             | |
-                |  |             | +->adj_stack_size
-                v  |             | |
-                   |             | |
-                   |             | +
-                   +-------------+/
+  -  ``adj_stack_ptr``: Adjusted initial stack pointer after the
+     frame has been removed from the stack. This will still be the
+     initial value of the stack pointer when the task is started.
 
   :param tcb: The TCB of new task.
   :param frame_size: The size of the stack frame to allocate.

diff --git a/arch/arm/src/arm/arm_assert.c b/arch/arm/src/arm/arm_assert.c
@@ -171,7 +171,7 @@ static void up_dumpstate(void)
 
   /* Get the limits on the user stack memory */
 
-  ustackbase = (uint32_t)rtcb->stack_base_ptr;
+  ustackbase = (uint32_t)rtcb->adj_stack_ptr;
   ustacksize = (uint32_t)rtcb->adj_stack_size;
 
   /* Get the limits on the interrupt stack memory */
@@ -240,14 +240,14 @@ static void up_dumpstate(void)
    * stack memory.
    */
 
-  if (sp >= ustackbase && sp < ustackbase + ustacksize)
+  if (sp >= ustackbase || sp < ustackbase - ustacksize)
     {
-      up_stackdump(sp, ustackbase + ustacksize);
+      _alert("ERROR: Stack pointer is not within allocated stack\n");
+      up_stackdump(ustackbase - ustacksize, ustackbase);
     }
   else
     {
-      _alert("ERROR: Stack pointer is not within allocated stack\n");
-      up_stackdump(ustackbase, ustackbase + ustacksize);
+      up_stackdump(sp, ustackbase);
     }
 
 #ifdef CONFIG_ARCH_USBDUMP

diff --git a/arch/arm/src/arm/arm_initialstate.c b/arch/arm/src/arm/arm_initialstate.c
@@ -62,7 +62,7 @@ void up_initial_state(struct tcb_s *tcb)
     {
       tcb->stack_alloc_ptr = (void *)(g_idle_topstack -
                                       CONFIG_IDLETHREAD_STACKSIZE);
-      tcb->stack_base_ptr   = tcb->stack_alloc_ptr;
+      tcb->adj_stack_ptr   = (void *)g_idle_topstack;
       tcb->adj_stack_size  = CONFIG_IDLETHREAD_STACKSIZE;
     }
 
@@ -72,8 +72,7 @@ void up_initial_state(struct tcb_s *tcb)
 
   /* Save the initial stack pointer */
 
-  xcp->regs[REG_SP]      = (uint32_t)tcb->stack_base_ptr +
-                                     tcb->adj_stack_size;
+  xcp->regs[REG_SP]      = (uint32_t)tcb->adj_stack_ptr;
 
   /* Save the task entry point */
 

diff --git a/arch/arm/src/arm/vfork.S b/arch/arm/src/arm/vfork.S
@@ -57,16 +57,16 @@
  *
  *   1) User code calls vfork().  vfork() collects context information and
  *      transfers control up up_vfork().
- *   2) up_vfork() and calls nxtask_setup_vfork().
+ *   2) up_vfork()and calls nxtask_setup_vfork().
  *   3) nxtask_setup_vfork() allocates and configures the child task's TCB.
  *      This consists of:
  *      - Allocation of the child task's TCB.
  *      - Initialization of file descriptors and streams
  *      - Configuration of environment variables
- *      - Allocate and initialize the stack
  *      - Setup the input parameters for the task.
- *      - Initialization of the TCB (including call to up_initial_state())
+ *      - Initialization of the TCB (including call to up_initial_state()
  *   4) up_vfork() provides any additional operating context. up_vfork must:
+ *      - Allocate and initialize the stack
  *      - Initialize special values in any CPU registers that were not
  *        already configured by up_initial_state()
  *   5) up_vfork() then calls nxtask_start_vfork()

diff --git a/arch/arm/src/armv6-m/arm_assert.c b/arch/arm/src/armv6-m/arm_assert.c
@@ -209,7 +209,7 @@ static void up_dumpstate(void)
 
   /* Get the limits on the user stack memory */
 
-  ustackbase = (uint32_t)rtcb->stack_base_ptr;
+  ustackbase = (uint32_t)rtcb->adj_stack_ptr;
   ustacksize = (uint32_t)rtcb->adj_stack_size;
 
   /* Get the limits on the interrupt stack memory */
@@ -266,14 +266,14 @@ static void up_dumpstate(void)
    * stack memory.
    */
 
-  if (sp >= ustackbase && sp < ustackbase + ustacksize)
+  if (sp < ustackbase && sp >= ustackbase - ustacksize)
     {
       up_stackdump(sp, ustackbase);
     }
   else
     {
       _alert("ERROR: Stack pointer is not within the allocated stack\n");
-      up_stackdump(ustackbase, ustackbase + ustacksize);
+      up_stackdump(ustackbase - ustacksize, ustackbase);
     }
 
 #else
@@ -288,14 +288,14 @@ static void up_dumpstate(void)
    * stack memory.
    */
 
-  if (sp >= ustackbase && sp < ustackbase + ustacksize)
+  if (sp >= ustackbase || sp < ustackbase - ustacksize)
     {
-      up_stackdump(sp, ustackbase + ustacksize);
+      _alert("ERROR: Stack pointer is not within allocated stack\n");
+      up_stackdump(ustackbase - ustacksize, ustackbase);
     }
   else
     {
-      _alert("ERROR: Stack pointer is not within allocated stack\n");
-      up_stackdump(ustackbase, ustackbase + ustacksize);
+      up_stackdump(sp, ustackbase);
     }
 #endif
 

diff --git a/arch/arm/src/armv6-m/arm_initialstate.c b/arch/arm/src/armv6-m/arm_initialstate.c
@@ -64,7 +64,7 @@ void up_initial_state(struct tcb_s *tcb)
     {
       tcb->stack_alloc_ptr = (void *)(g_idle_topstack -
                                       CONFIG_IDLETHREAD_STACKSIZE);
-      tcb->stack_base_ptr   = tcb->stack_alloc_ptr;
+      tcb->adj_stack_ptr   = (void *)g_idle_topstack;
       tcb->adj_stack_size  = CONFIG_IDLETHREAD_STACKSIZE;
     }
 
@@ -74,8 +74,7 @@ void up_initial_state(struct tcb_s *tcb)
 
   /* Save the initial stack pointer */
 
-  xcp->regs[REG_SP]      = (uint32_t)tcb->stack_base_ptr +
-                                     tcb->adj_stack_size;
+  xcp->regs[REG_SP]      = (uint32_t)tcb->adj_stack_ptr;
 
   /* Save the task entry point (stripping off the thumb bit) */
 

diff --git a/arch/arm/src/armv6-m/vfork.S b/arch/arm/src/armv6-m/vfork.S
@@ -57,16 +57,16 @@
  *
  *   1) User code calls vfork().  vfork() collects context information and
  *      transfers control up up_vfork().
- *   2) up_vfork() and calls nxtask_setup_vfork().
+ *   2) up_vfork()and calls nxtask_setup_vfork().
  *   3) nxtask_setup_vfork() allocates and configures the child task's TCB.
  *      This consists of:
  *      - Allocation of the child task's TCB.
  *      - Initialization of file descriptors and streams
  *      - Configuration of environment variables
- *      - Allocate and initialize the stack
  *      - Setup the input parameters for the task.
- *      - Initialization of the TCB (including call to up_initial_state())
+ *      - Initialization of the TCB (including call to up_initial_state()
  *   4) up_vfork() provides any additional operating context. up_vfork must:
+ *      - Allocate and initialize the stack
  *      - Initialize special values in any CPU registers that were not
  *        already configured by up_initial_state()
  *   5) up_vfork() then calls nxtask_start_vfork()

diff --git a/arch/arm/src/armv7-a/arm_assert.c b/arch/arm/src/armv7-a/arm_assert.c
@@ -212,7 +212,7 @@ static void up_dumpstate(void)
 
   /* Get the limits on the user stack memory */
 
-  ustackbase = (uint32_t)rtcb->stack_base_ptr;
+  ustackbase = (uint32_t)rtcb->adj_stack_ptr;
   ustacksize = (uint32_t)rtcb->adj_stack_size;
 
   _alert("Current sp: %08x\n", sp);
@@ -291,10 +291,10 @@ static void up_dumpstate(void)
    * stack memory.
    */
 
-  if (sp >= ustackbase && sp < ustackbase + ustacksize)
+  if (sp >= ustackbase - ustacksize && sp < ustackbase)
     {
       _alert("User Stack\n", sp);
-      up_stackdump(sp, ustackbase + ustacksize);
+      up_stackdump(sp, ustackbase);
     }
 
 #ifdef CONFIG_ARCH_KERNEL_STACK
@@ -312,7 +312,7 @@ static void up_dumpstate(void)
   else
     {
       _alert("ERROR: Stack pointer is not within the allocated stack\n");
-      up_stackdump(ustackbase, ustackbase + ustacksize);
+      up_stackdump(ustackbase - ustacksize, ustackbase);
 #ifdef CONFIG_ARCH_KERNEL_STACK
       up_stackdump(kstackbase, kstackbase + CONFIG_ARCH_KERNEL_STACKSIZE);
 #endif

diff --git a/arch/arm/src/armv7-a/arm_cpuidlestack.c b/arch/arm/src/armv7-a/arm_cpuidlestack.c
@@ -94,8 +94,8 @@ static FAR const uint32_t *g_cpu_stackalloc[CONFIG_SMP_NCPUS] =
  *   - adj_stack_size: Stack size after adjustment for hardware, processor,
  *     etc.  This value is retained only for debug purposes.
  *   - stack_alloc_ptr: Pointer to allocated stack
- *   - stack_base_ptr: Adjusted stack base pointer after the TLS Data and
- *     Arguments has been removed from the stack allocation.
+ *   - adj_stack_ptr: Adjusted stack_alloc_ptr for HW.  The initial value of
+ *     the stack pointer.
  *
  * Input Parameters:
  *   - cpu:         CPU index that indicates which CPU the IDLE task is
@@ -111,6 +111,7 @@ int up_cpu_idlestack(int cpu, FAR struct tcb_s *tcb, size_t stack_size)
 {
 #if CONFIG_SMP_NCPUS > 1
   uintptr_t stack_alloc;
+  uintptr_t top_of_stack;
 
   DEBUGASSERT(cpu > 0 && cpu < CONFIG_SMP_NCPUS && tcb != NULL &&
               stack_size <= SMP_STACK_SIZE);
@@ -119,10 +120,11 @@ int up_cpu_idlestack(int cpu, FAR struct tcb_s *tcb, size_t stack_size)
 
   stack_alloc          = (uintptr_t)g_cpu_stackalloc[cpu];
   DEBUGASSERT(stack_alloc != 0 && STACK_ISALIGNED(stack_alloc));
+  top_of_stack         = stack_alloc + SMP_STACK_SIZE;
 
   tcb->adj_stack_size  = SMP_STACK_SIZE;
-  tcb->stack_alloc_ptr = (FAR void *)stack_alloc;
-  tcb->stack_base_ptr   = tcb->stack_alloc_ptr;
+  tcb->stack_alloc_ptr = (FAR uint32_t *)stack_alloc;
+  tcb->adj_stack_ptr   = (FAR uint32_t *)top_of_stack;
 #endif
 
   return OK;

diff --git a/arch/arm/src/armv7-a/arm_initialstate.c b/arch/arm/src/armv7-a/arm_initialstate.c
@@ -62,7 +62,7 @@ void up_initial_state(struct tcb_s *tcb)
     {
       tcb->stack_alloc_ptr = (void *)(g_idle_topstack -
                                       CONFIG_IDLETHREAD_STACKSIZE);
-      tcb->stack_base_ptr   = tcb->stack_alloc_ptr;
+      tcb->adj_stack_ptr   = (void *)g_idle_topstack;
       tcb->adj_stack_size  = CONFIG_IDLETHREAD_STACKSIZE;
     }
 
@@ -72,8 +72,7 @@ void up_initial_state(struct tcb_s *tcb)
 
   /* Save the initial stack pointer */
 
-  xcp->regs[REG_SP] = (uint32_t)tcb->stack_base_ptr +
-                                tcb->adj_stack_size;
+  xcp->regs[REG_SP] = (uint32_t)tcb->adj_stack_ptr;
 
   /* Save the task entry point */
 

diff --git a/arch/arm/src/armv7-a/vfork.S b/arch/arm/src/armv7-a/vfork.S
@@ -57,16 +57,16 @@
  *
  *   1) User code calls vfork().  vfork() collects context information and
  *      transfers control up up_vfork().
- *   2) up_vfork() and calls nxtask_setup_vfork().
+ *   2) up_vfork()and calls nxtask_setup_vfork().
  *   3) nxtask_setup_vfork() allocates and configures the child task's TCB.
  *      This consists of:
  *      - Allocation of the child task's TCB.
  *      - Initialization of file descriptors and streams
  *      - Configuration of environment variables
- *      - Allocate and initialize the stack
  *      - Setup the input parameters for the task.
- *      - Initialization of the TCB (including call to up_initial_state())
+ *      - Initialization of the TCB (including call to up_initial_state()
  *   4) up_vfork() provides any additional operating context. up_vfork must:
+ *      - Allocate and initialize the stack
  *      - Initialize special values in any CPU registers that were not
  *        already configured by up_initial_state()
  *   5) up_vfork() then calls nxtask_start_vfork()

diff --git a/arch/arm/src/armv7-m/arm_assert.c b/arch/arm/src/armv7-m/arm_assert.c
@@ -217,7 +217,7 @@ static void up_dumpstate(void)
 
   /* Get the limits on the user stack memory */
 
-  ustackbase = (uint32_t)rtcb->stack_base_ptr;
+  ustackbase = (uint32_t)rtcb->adj_stack_ptr;
   ustacksize = (uint32_t)rtcb->adj_stack_size;
 
 #if CONFIG_ARCH_INTERRUPTSTACK > 7
@@ -278,14 +278,14 @@ static void up_dumpstate(void)
    * stack memory.
    */
 
-  if (sp >= ustackbase && sp < ustackbase + ustacksize)
+  if (sp < ustackbase && sp >= ustackbase - ustacksize)
     {
-      up_stackdump(sp, ustackbase + ustacksize);
+      up_stackdump(sp, ustackbase);
     }
   else
     {
       _alert("ERROR: Stack pointer is not within the allocated stack\n");
-      up_stackdump(ustackbase, ustackbase + ustacksize);
+      up_stackdump(ustackbase - ustacksize, ustackbase);
     }
 
 #else
@@ -303,14 +303,14 @@ static void up_dumpstate(void)
    * stack memory.
    */
 
-  if (sp >= ustackbase && sp < ustackbase + ustacksize)
+  if (sp >= ustackbase || sp < ustackbase - ustacksize)
     {
-      up_stackdump(sp, ustackbase + ustacksize);
+      _alert("ERROR: Stack pointer is not within the allocated stack\n");
+      up_stackdump(ustackbase - ustacksize, ustackbase);
     }
   else
     {
-      _alert("ERROR: Stack pointer is not within the allocated stack\n");
-      up_stackdump(ustackbase, ustackbase + ustacksize);
+      up_stackdump(sp, ustackbase);
     }
 
 #endif

diff --git a/arch/arm/src/armv7-m/arm_initialstate.c b/arch/arm/src/armv7-m/arm_initialstate.c
@@ -65,7 +65,7 @@ void up_initial_state(struct tcb_s *tcb)
     {
       tcb->stack_alloc_ptr = (void *)(g_idle_topstack -
                                       CONFIG_IDLETHREAD_STACKSIZE);
-      tcb->stack_base_ptr   = tcb->stack_alloc_ptr;
+      tcb->adj_stack_ptr   = (void *)g_idle_topstack;
       tcb->adj_stack_size  = CONFIG_IDLETHREAD_STACKSIZE;
     }
 
@@ -75,8 +75,7 @@ void up_initial_state(struct tcb_s *tcb)
 
   /* Save the initial stack pointer */
 
-  xcp->regs[REG_SP]      = (uint32_t)tcb->stack_base_ptr +
-                                     tcb->adj_stack_size;
+  xcp->regs[REG_SP]      = (uint32_t)tcb->adj_stack_ptr;
 
 #ifdef CONFIG_ARMV7M_STACKCHECK
   /* Set the stack limit value */

diff --git a/arch/arm/src/armv7-m/gnu/vfork.S b/arch/arm/src/armv7-m/gnu/vfork.S
@@ -59,16 +59,16 @@
  *
  *   1) User code calls vfork().  vfork() collects context information and
  *      transfers control up up_vfork().
- *   2) up_vfork() and calls nxtask_setup_vfork().
+ *   2) up_vfork()and calls nxtask_setup_vfork().
  *   3) nxtask_setup_vfork() allocates and configures the child task's TCB.
  *      This consists of:
  *      - Allocation of the child task's TCB.
  *      - Initialization of file descriptors and streams
  *      - Configuration of environment variables
- *      - Allocate and initialize the stack
  *      - Setup the input parameters for the task.
- *      - Initialization of the TCB (including call to up_initial_state())
+ *      - Initialization of the TCB (including call to up_initial_state()
  *   4) up_vfork() provides any additional operating context. up_vfork must:
+ *      - Allocate and initialize the stack
  *      - Initialize special values in any CPU registers that were not
  *        already configured by up_initial_state()
  *   5) up_vfork() then calls nxtask_start_vfork()