davids5 commented on a change in pull request #3517:
URL: https://github.com/apache/incubator-nuttx/pull/3517#discussion_r612489165



##########
File path: arch/arm/src/common/arm_usestack.c
##########
@@ -127,37 +117,17 @@ int up_use_stack(struct tcb_s *tcb, void *stack, size_t 
stack_size)
   /* Save the new stack allocation */
 
   tcb->stack_alloc_ptr = stack;
-
-  /* Align stack top */
-
-  tcb->adj_stack_ptr =
-      (FAR void *)STACK_ALIGN_DOWN((uintptr_t)stack + stack_size);
-
-  /* Offset by tls_size */
-
-  stack = (FAR void *)((uintptr_t)stack + tls_size);
-
-  /* Is there enough room for at least TLS ? */
-
-  if ((uintptr_t)stack > (uintptr_t)tcb->adj_stack_ptr)
-    {
-      return -ENOMEM;
-    }
-
-  tcb->adj_stack_size = (uintptr_t)tcb->adj_stack_ptr - (uintptr_t)stack;
-
-  /* Initialize the TLS data structure */
-
-  memset(tcb->stack_alloc_ptr, 0, tls_size);
+  tcb->adj_stack_ptr   = tcb->stack_alloc_ptr;
+  tcb->adj_stack_size  =
+      STACK_ALIGN_DOWN((uintptr_t)stack + stack_size) - (uintptr_t)stack;

Review comment:
       In the old code the task/thread SP was set to the adj_stack_ptr. That 
value by inspection was properly aligned.
   This change splits that alignment across the pointer and the size. Making 
all this very pron to errors.
   

##########
File path: arch/arm/src/armv6-m/arm_initialstate.c
##########
@@ -74,7 +74,8 @@ void up_initial_state(struct tcb_s *tcb)
 
   /* Save the initial stack pointer */
 
-  xcp->regs[REG_SP]      = (uint32_t)tcb->adj_stack_ptr;
+  xcp->regs[REG_SP]      = (uint32_t)tcb->adj_stack_ptr +

Review comment:
       @xiaoxiang781216 - the old code's naming make sense and matched the 
semantic.  `xcp->regs[REG_SP] = (uint32_t)tcb->adj_stack_ptr`
   
   In the new code you have changed the semantics and I feel very strongly that 
the naming has change.
   tcb->adj_stack_ptr is NOW the stack base. Please call it that. 
`stackbase_ptr`
   
   Then a the stack penetration test will start at the `stackbase_ptr` upward 
for the guard value.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to