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 2021/12/16 01:33:47 UTC
[GitHub] [incubator-nuttx] pkarashchenko opened a new pull request #5008: assert: unify stack and register dump across platforms
pkarashchenko opened a new pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008
## Summary
unify stack and register dump across platforms
## Impact
All platforms
## Testing
Pass 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] pkarashchenko commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r770652901
##########
File path: arch/arm/src/armv6-m/arm_assert.c
##########
@@ -288,27 +286,13 @@ static void up_dumpstate(void)
#ifdef CONFIG_STACK_COLORATION
_alert(" used: %08x\n", up_check_tcbstack(rtcb));
#endif
-
- /* Dump the user stack if the stack pointer lies within the allocated user
- * stack memory.
- */
-
- if (sp >= ustackbase && sp < ustackbase + ustacksize)
- {
- up_stackdump(sp, ustackbase);
- }
- else
- {
- _alert("ERROR: Stack pointer is not within the allocated stack\n");
Review comment:
few lines below at line 309
--
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] pkarashchenko commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r771999926
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -168,7 +168,7 @@ static inline void xtensa_registerdump(void)
void xtensa_dumpstate(void)
{
- struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = running_task();
Review comment:
But this seems to be not a valid statement as I see in `arch/risc-v/src`
1. `void up_tls_initialize(FAR struct tls_info_s *info)`
2. `void weak_function mpfs_spi0_select(FAR struct spi_dev_s *dev, uint32_t devid, bool selected)`
Probably need to clean-up this as well
--
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] pkarashchenko commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r770652901
##########
File path: arch/arm/src/armv6-m/arm_assert.c
##########
@@ -288,27 +286,13 @@ static void up_dumpstate(void)
#ifdef CONFIG_STACK_COLORATION
_alert(" used: %08x\n", up_check_tcbstack(rtcb));
#endif
-
- /* Dump the user stack if the stack pointer lies within the allocated user
- * stack memory.
- */
-
- if (sp >= ustackbase && sp < ustackbase + ustacksize)
- {
- up_stackdump(sp, ustackbase);
- }
- else
- {
- _alert("ERROR: Stack pointer is not within the allocated stack\n");
Review comment:
few lines below at line 302
--
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] pkarashchenko commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r770433012
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -96,10 +96,10 @@ static inline void up_showtasks(void)
#endif
/****************************************************************************
- * Name: xtensa_stackdump
+ * Name: up_stackdump
****************************************************************************/
-static void xtensa_stackdump(uint32_t sp, uint32_t stack_top)
+static void up_stackdump(uint32_t sp, uint32_t stack_top)
Review comment:
Ok. I will revert these parts of the changes
--
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] gustavonihei commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r771567712
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -168,7 +168,7 @@ static inline void xtensa_registerdump(void)
void xtensa_dumpstate(void)
{
- struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = running_task();
Review comment:
```suggestion
struct tcb_s *rtcb = running_task();
```
The `FAR` and `CODE` qualifiers had been wiped out from the `xtensa` and `risc-v` archs some time ago since it is mandatory only for common 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.
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] pkarashchenko commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r771637342
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -168,7 +168,7 @@ static inline void xtensa_registerdump(void)
void xtensa_dumpstate(void)
{
- struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = running_task();
Review comment:
I will redo the PR to address this
--
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] Ouss4 commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r770415082
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -96,10 +96,10 @@ static inline void up_showtasks(void)
#endif
/****************************************************************************
- * Name: xtensa_stackdump
+ * Name: up_stackdump
****************************************************************************/
-static void xtensa_stackdump(uint32_t sp, uint32_t stack_top)
+static void up_stackdump(uint32_t sp, uint32_t stack_top)
Review comment:
This is an internal function, the naming convention for these functions is `arch_function`. The `up_` prefix is reserved for functions exported outside the arch directory.
--
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] gustavonihei commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r772000964
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -168,7 +168,7 @@ static inline void xtensa_registerdump(void)
void xtensa_dumpstate(void)
{
- struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = running_task();
Review comment:
> But this seems to be not a valid statement as I see in `arch/risc-v/src`
The fact that new code introduced those qualifiers back does not invalidate my statement. They were completely removed from RISC-V and Xtensa archs in a past PR.
> Done
Thanks.
--
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] pkarashchenko commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r770433012
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -96,10 +96,10 @@ static inline void up_showtasks(void)
#endif
/****************************************************************************
- * Name: xtensa_stackdump
+ * Name: up_stackdump
****************************************************************************/
-static void xtensa_stackdump(uint32_t sp, uint32_t stack_top)
+static void up_stackdump(uint32_t sp, uint32_t stack_top)
Review comment:
Ok. I will revert this part of the change
--
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] Ouss4 commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r770415082
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -96,10 +96,10 @@ static inline void up_showtasks(void)
#endif
/****************************************************************************
- * Name: xtensa_stackdump
+ * Name: up_stackdump
****************************************************************************/
-static void xtensa_stackdump(uint32_t sp, uint32_t stack_top)
+static void up_stackdump(uint32_t sp, uint32_t stack_top)
Review comment:
This is an internal function, the naming convention for these functions is `arch_function`.
--
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] pkarashchenko commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r772000306
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -168,7 +168,7 @@ static inline void xtensa_registerdump(void)
void xtensa_dumpstate(void)
{
- struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = running_task();
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] gustavonihei merged pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
gustavonihei merged pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008
--
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] gustavonihei commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r771567712
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -168,7 +168,7 @@ static inline void xtensa_registerdump(void)
void xtensa_dumpstate(void)
{
- struct tcb_s *rtcb = running_task();
+ FAR struct tcb_s *rtcb = running_task();
Review comment:
```suggestion
struct tcb_s *rtcb = running_task();
```
The `FAR` qualifier had been wiped out from the `xtensa` and `risc-v` archs some time ago since it is mandatory only for common 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.
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] pkarashchenko commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r770652901
##########
File path: arch/arm/src/armv6-m/arm_assert.c
##########
@@ -288,27 +286,13 @@ static void up_dumpstate(void)
#ifdef CONFIG_STACK_COLORATION
_alert(" used: %08x\n", up_check_tcbstack(rtcb));
#endif
-
- /* Dump the user stack if the stack pointer lies within the allocated user
- * stack memory.
- */
-
- if (sp >= ustackbase && sp < ustackbase + ustacksize)
- {
- up_stackdump(sp, ustackbase);
- }
- else
- {
- _alert("ERROR: Stack pointer is not within the allocated stack\n");
Review comment:
few lines below at line 309 of the updated 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.
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] pkarashchenko commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r770650033
##########
File path: arch/xtensa/src/common/xtensa_dumpstate.c
##########
@@ -96,10 +96,10 @@ static inline void up_showtasks(void)
#endif
/****************************************************************************
- * Name: xtensa_stackdump
+ * Name: up_stackdump
****************************************************************************/
-static void xtensa_stackdump(uint32_t sp, uint32_t stack_top)
+static void up_stackdump(uint32_t sp, uint32_t stack_top)
Review comment:
@Ouss4 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] davids5 commented on a change in pull request #5008: assert: unify stack and register dump across platforms
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #5008:
URL: https://github.com/apache/incubator-nuttx/pull/5008#discussion_r770490242
##########
File path: arch/arm/src/armv6-m/arm_assert.c
##########
@@ -288,27 +286,13 @@ static void up_dumpstate(void)
#ifdef CONFIG_STACK_COLORATION
_alert(" used: %08x\n", up_check_tcbstack(rtcb));
#endif
-
- /* Dump the user stack if the stack pointer lies within the allocated user
- * stack memory.
- */
-
- if (sp >= ustackbase && sp < ustackbase + ustacksize)
- {
- up_stackdump(sp, ustackbase);
- }
- else
- {
- _alert("ERROR: Stack pointer is not within the allocated stack\n");
Review comment:
Where is the replacement for this?
--
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