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



##########
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:
       > The stack overflow is a critical error regardless it overflow TLS 
region or something else. The same tool(ps, up_check_stack) work as before, we 
don't need invent the new one.
   
   Of course. If it always was a liner blow through and simply detected, I 
would agree with you, But in the real world it is not. I have seen a call at 
the bottom of a stack that do not corrupt the bottom but under the base by 
chunks of locals.  When this hits a a well defined struct (like TCB)  you can 
tell, when it hits something in else not clearly defined like TLS, it is harder 
to debug. 
   
   Some other OS (pSOS) protect label and protect their data.  It is just a 
suggestion to make debugging efficient. 




-- 
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