You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/21 14:11:06 UTC
[GitHub] [incubator-nuttx] Donny9 opened a new pull request, #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Donny9 opened a new pull request, #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665
## Summary
1. using running_task to get current tcb because this task maybe wrong in interrupt context.
2. Limit dump size when sp is not within stack. we need to limit this dump size because if the stack size of task is over syslog buffer, it will overwrite other dump message.
## Impact
N/A
## Testing
Vela Ci
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r928639906
##########
arch/arm/src/common/arm_assert.c:
##########
@@ -335,7 +337,7 @@ static int assert_tracecallback(struct usbtrace_s *trace, void *arg)
* Name: arm_dump_stack
****************************************************************************/
-static void arm_dump_stack(const char *tag, uint32_t sp,
+static void arm_dump_stack(const char *tag, uint32_t sp, ssize_t remain,
Review Comment:
Done!
##########
arch/arm/src/common/arm_assert.c:
##########
@@ -411,6 +425,11 @@ static void arm_dumpstate(void)
/* Dump the user stack */
arm_dump_stack("User", sp,
+#ifdef CONFIG_STACK_COLORATION
+ up_check_stack_remain(),
Review Comment:
Done! patch 2
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r928640248
##########
arch/arm/src/common/arm_checkstack.c:
##########
@@ -209,12 +209,12 @@ ssize_t up_check_tcbstack_remain(struct tcb_s *tcb)
size_t up_check_stack(void)
{
- return up_check_tcbstack(this_task());
+ return up_check_tcbstack(running_task());
Review Comment:
Done! patch 1
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r930570574
##########
arch/arm/src/common/arm_assert.c:
##########
@@ -352,10 +352,17 @@ static void arm_dump_stack(const char *tag, uint32_t sp,
else
{
_alert("ERROR: %s Stack pointer is not within the stack\n", tag);
+#ifdef CONFIG_STACK_COLORATION
Review Comment:
Done!
##########
arch/risc-v/src/common/riscv_assert.c:
##########
@@ -307,6 +307,35 @@ static inline void riscv_showtasks(void)
#endif
}
+/****************************************************************************
+ * Name: riscv_dump_stack
+ ****************************************************************************/
+
+static void riscv_dump_stack(const char *tag, uintptr_t sp,
+ uintptr_t base, uint32_t size, bool force)
+{
+ uintptr_t top = base + size;
+
+ _alert("%s Stack:\n", tag);
+ _alert("sp: %08" PRIx32 "\n", sp);
+ _alert(" base: %08" PRIx32 "\n", base);
Review Comment:
Done!
##########
arch/risc-v/src/common/riscv_assert.c:
##########
@@ -307,6 +307,35 @@ static inline void riscv_showtasks(void)
#endif
}
+/****************************************************************************
+ * Name: riscv_dump_stack
+ ****************************************************************************/
+
+static void riscv_dump_stack(const char *tag, uintptr_t sp,
+ uintptr_t base, uint32_t size, bool force)
+{
+ uintptr_t top = base + size;
+
+ _alert("%s Stack:\n", tag);
+ _alert("sp: %08" PRIx32 "\n", sp);
Review Comment:
Done!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r929513354
##########
arch/arm/src/common/arm_assert.c:
##########
@@ -353,9 +354,12 @@ static void arm_dump_stack(const char *tag, uint32_t sp,
{
_alert("ERROR: %s Stack pointer is not within the stack\n", tag);
+ remain = size - arm_stackcheck((FAR void *)(uintptr_t)base, size);
Review Comment:
Done!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
Donny9 commented on PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#issuecomment-1192150974
>
>
> @Donny9
>
> > Limit dump size when sp is not within stack. we need to limit this dump size because if the stack size of task is over syslog buffer, it will overwrite other dump message.
>
> This is a use case limitation. Please make it conditional or deal with it another way.
Hello, this is a common issue for dump super large stack? If sp isn't within stack, this case usually not caused by stack overflow, if dump all stack, you will see that there are a lot of deadbeef(if color support) content in the lower stack, they are useless, so we need to rely on remain to ignore them(deadbeed).
This PR will set limit size to 8K, so when this case happened, dump_stack function will dump stack from base+remain --- base+remain + 8K(if stack grow from high addr to low addr), this can allows user to see the effective information they need.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
davids5 commented on PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#issuecomment-1191665435
@Donny9
>Limit dump size when sp is not within stack. we need to limit this dump size because if the stack size of task is over syslog buffer, it will overwrite other dump message.
This is a use case limitation. Please make it conditional or deal with it another way.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r928505053
##########
arch/arm/src/common/arm_assert.c:
##########
@@ -411,6 +425,11 @@ static void arm_dumpstate(void)
/* Dump the user stack */
arm_dump_stack("User", sp,
+#ifdef CONFIG_STACK_COLORATION
+ up_check_stack_remain(),
Review Comment:
let's rename do_stackcheck to arm_check_stack and call this function inside arm_dump_stack?
##########
arch/arm/src/common/arm_assert.c:
##########
@@ -335,7 +337,7 @@ static int assert_tracecallback(struct usbtrace_s *trace, void *arg)
* Name: arm_dump_stack
****************************************************************************/
-static void arm_dump_stack(const char *tag, uint32_t sp,
+static void arm_dump_stack(const char *tag, uint32_t sp, ssize_t remain,
Review Comment:
ssize_t to uint32_t
##########
arch/arm/src/common/arm_checkstack.c:
##########
@@ -209,12 +209,12 @@ ssize_t up_check_tcbstack_remain(struct tcb_s *tcb)
size_t up_check_stack(void)
{
- return up_check_tcbstack(this_task());
+ return up_check_tcbstack(running_task());
Review Comment:
let's update arm64 too
##########
arch/xtensa/src/common/xtensa_dumpstate.c:
##########
@@ -44,6 +44,12 @@
#ifdef CONFIG_DEBUG_ALERT
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define STACKDUMP_SIZE 8192
Review Comment:
let's update the risc-v too
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r928941254
##########
arch/arm64/src/common/arm64_internal.h:
##########
@@ -352,6 +352,7 @@ void arm64_usbuninitialize(void);
/* Debug */
#ifdef CONFIG_STACK_COLORATION
+size_t arm64_stackcheck(void *stackbase, size_t nbytes);
Review Comment:
ditto
##########
arch/arm/src/common/arm_internal.h:
##########
@@ -440,6 +440,7 @@ void arm_usbuninitialize(void);
/* Debug ********************************************************************/
#ifdef CONFIG_STACK_COLORATION
+size_t arm_stackcheck(void *stackbase, size_t nbytes);
Review Comment:
arm_stack_check
##########
arch/Kconfig:
##########
@@ -837,6 +837,14 @@ config ARCH_STACKDUMP
---help---
Enable to do stack dumps after assertions
+if ARCH_STACKDUMP
+
+config ARCH_STACKDUMP_LIMIT_LENGTH
Review Comment:
LIMIT_LENGTH->MAX_LENGTH
##########
arch/arm/src/common/arm_assert.c:
##########
@@ -353,9 +354,12 @@ static void arm_dump_stack(const char *tag, uint32_t sp,
{
_alert("ERROR: %s Stack pointer is not within the stack\n", tag);
+ remain = size - arm_stackcheck((FAR void *)(uintptr_t)base, size);
Review Comment:
need:
```
#ifdef CONFIG_STACK_COLORATION
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r928953103
##########
arch/xtensa/src/common/xtensa_dumpstate.c:
##########
@@ -395,6 +395,21 @@ void xtensa_dumpstate(void)
}
else
{
+#ifdef CONFIG_STACK_COLORATION
+ uint32_t remain;
+
+ remain = ustacksize - xtensa_stackcheck((uintptr_t)ustackbase,
Review Comment:
where is risc-v?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r929513255
##########
arch/arm/src/common/arm_internal.h:
##########
@@ -440,6 +440,7 @@ void arm_usbuninitialize(void);
/* Debug ********************************************************************/
#ifdef CONFIG_STACK_COLORATION
+size_t arm_stackcheck(void *stackbase, size_t nbytes);
Review Comment:
Done!
##########
arch/arm64/src/common/arm64_internal.h:
##########
@@ -352,6 +352,7 @@ void arm64_usbuninitialize(void);
/* Debug */
#ifdef CONFIG_STACK_COLORATION
+size_t arm64_stackcheck(void *stackbase, size_t nbytes);
Review Comment:
Done!
##########
arch/Kconfig:
##########
@@ -837,6 +837,14 @@ config ARCH_STACKDUMP
---help---
Enable to do stack dumps after assertions
+if ARCH_STACKDUMP
+
+config ARCH_STACKDUMP_LIMIT_LENGTH
Review Comment:
Done!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r929513475
##########
arch/xtensa/src/common/xtensa_dumpstate.c:
##########
@@ -395,6 +395,21 @@ void xtensa_dumpstate(void)
}
else
{
+#ifdef CONFIG_STACK_COLORATION
+ uint32_t remain;
+
+ remain = ustacksize - xtensa_stackcheck((uintptr_t)ustackbase,
Review Comment:
Done!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#issuecomment-1196665388
@davids5 could you take a look?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
Donny9 commented on PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#issuecomment-1196419708
@xiaoxiang781216 need to retrigger CI?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
Donny9 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r930570504
##########
arch/Kconfig:
##########
@@ -837,6 +837,14 @@ config ARCH_STACKDUMP
---help---
Enable to do stack dumps after assertions
+if ARCH_STACKDUMP
+
+config ARCH_STACKDUMP_MAX_LENGTH
+ int "The maximum length for dump stack on assertions"
Review Comment:
Done.
##########
arch/xtensa/src/common/xtensa_dumpstate.c:
##########
@@ -395,6 +395,15 @@ void xtensa_dumpstate(void)
}
else
{
+#ifdef CONFIG_STACK_COLORATION
+ uint32_t remain;
+
+ remain = ustacksize - xtensa_stack_check((uintptr_t)ustackbase,
+ ustacksize);
+ ustackbase += remain;
+ ustacksize -= remain;
+#endif
+
_alert("ERROR: Stack pointer is not within allocated stack\n");
xtensa_stackdump(ustackbase, ustackbase + ustacksize);
Review Comment:
Done
##########
arch/risc-v/src/common/riscv_assert.c:
##########
@@ -327,11 +327,19 @@ static void riscv_dump_stack(const char *tag, uintptr_t sp,
}
else
{
+#ifdef CONFIG_STACK_COLORATION
Review Comment:
Done!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665#discussion_r930330304
##########
arch/risc-v/src/common/riscv_assert.c:
##########
@@ -307,6 +307,35 @@ static inline void riscv_showtasks(void)
#endif
}
+/****************************************************************************
+ * Name: riscv_dump_stack
+ ****************************************************************************/
+
+static void riscv_dump_stack(const char *tag, uintptr_t sp,
+ uintptr_t base, uint32_t size, bool force)
+{
+ uintptr_t top = base + size;
+
+ _alert("%s Stack:\n", tag);
+ _alert("sp: %08" PRIx32 "\n", sp);
Review Comment:
PRIxPTR
##########
arch/risc-v/src/common/riscv_assert.c:
##########
@@ -307,6 +307,35 @@ static inline void riscv_showtasks(void)
#endif
}
+/****************************************************************************
+ * Name: riscv_dump_stack
+ ****************************************************************************/
+
+static void riscv_dump_stack(const char *tag, uintptr_t sp,
+ uintptr_t base, uint32_t size, bool force)
+{
+ uintptr_t top = base + size;
+
+ _alert("%s Stack:\n", tag);
+ _alert("sp: %08" PRIx32 "\n", sp);
+ _alert(" base: %08" PRIx32 "\n", base);
Review Comment:
PRIxPTR
##########
arch/risc-v/src/common/riscv_assert.c:
##########
@@ -327,11 +327,19 @@ static void riscv_dump_stack(const char *tag, uintptr_t sp,
}
else
{
+#ifdef CONFIG_STACK_COLORATION
Review Comment:
move after line 342
##########
arch/arm/src/common/arm_assert.c:
##########
@@ -352,10 +352,17 @@ static void arm_dump_stack(const char *tag, uint32_t sp,
else
{
_alert("ERROR: %s Stack pointer is not within the stack\n", tag);
+#ifdef CONFIG_STACK_COLORATION
Review Comment:
move after line 358
##########
arch/xtensa/src/common/xtensa_dumpstate.c:
##########
@@ -395,6 +395,15 @@ void xtensa_dumpstate(void)
}
else
{
+#ifdef CONFIG_STACK_COLORATION
+ uint32_t remain;
+
+ remain = ustacksize - xtensa_stack_check((uintptr_t)ustackbase,
+ ustacksize);
+ ustackbase += remain;
+ ustacksize -= remain;
+#endif
+
_alert("ERROR: Stack pointer is not within allocated stack\n");
xtensa_stackdump(ustackbase, ustackbase + ustacksize);
Review Comment:
should we add force to xtensa_stackdump and remove the code dup?
##########
arch/Kconfig:
##########
@@ -837,6 +837,14 @@ config ARCH_STACKDUMP
---help---
Enable to do stack dumps after assertions
+if ARCH_STACKDUMP
+
+config ARCH_STACKDUMP_MAX_LENGTH
+ int "The maximum length for dump stack on assertions"
Review Comment:
depends on ARCH_STACKDUMP and remove if/endif
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6665: arch/stack: get correct stack remain and limit dump size when sp is not within stack
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6665:
URL: https://github.com/apache/incubator-nuttx/pull/6665
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org