davids5 commented on a change in pull request #3517:
URL: https://github.com/apache/incubator-nuttx/pull/3517#discussion_r613162096
##########
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:
@patacongo (wrong sig above) Here is the patch:
[d485c36](https://github.com/apache/incubator-nuttx/commit/d485c3687fc9e25380706f4af09de5157c9c0a79)
@xiaoxiang781216 - Thank you! That feels a lot safer. I think @patacongo
gave some important usage guidelines. Which raises the question: Do we have
documentation, all in one place, that defines stack usage, sizing and
guidelines for selecting the TLS config? If not I feel this could really hurt
adoption. Nothing worse than a bad first impression. If a new user see a 16K
stack, or has blind stack related crashes.
Since the the symptom of a shallow stack crash will now corrupt the TLS
should we add guards and checks in the future usage code?
--
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
