You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "xiaoxiang781216 (via GitHub)" <gi...@apache.org> on 2023/01/26 18:17:03 UTC

[GitHub] [nuttx] xiaoxiang781216 opened a new pull request, #8252: tlsr82/backtrace: tc32 backtrace bug fix

xiaoxiang781216 opened a new pull request, #8252:
URL: https://github.com/apache/nuttx/pull/8252

   ## Summary
   
   - tc32/Make.defs: fix build break after merge unblock/block_task
   
   ## Impact
   
   tlsr82
   
   ## Testing
   
   Internal testing


-- 
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] [nuttx] CV-Bowen commented on a diff in pull request #8252: tlsr82/backtrace: tc32 backtrace bug fix

Posted by "CV-Bowen (via GitHub)" <gi...@apache.org>.
CV-Bowen commented on code in PR #8252:
URL: https://github.com/apache/nuttx/pull/8252#discussion_r1090307345


##########
arch/arm/src/tlsr82/tc32/tc32_backtrace.c:
##########
@@ -63,6 +63,22 @@
 
 static void **g_backtrace_code_regions;
 
+/****************************************************************************
+ * Inline functions
+ ****************************************************************************/
+
+static inline uint32_t tc32_getsp(void)

Review Comment:
   I will verify this later.



-- 
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] [nuttx] pkarashchenko commented on a diff in pull request #8252: tlsr82/backtrace: tc32 backtrace bug fix

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8252:
URL: https://github.com/apache/nuttx/pull/8252#discussion_r1088427666


##########
arch/arm/src/tlsr82/tc32/tc32_backtrace.c:
##########
@@ -185,25 +201,53 @@ static void *backtrace_push_internal(void **psp, void **ppc)
 
   found = false;
 
+  /* Stack frame increase order:
+   * <function>:
+   * 1  tpush {rx, lr}     (must    , IOP_T_PUSH     , case 2)
+   * 2  other instructions
+   * 3  tpush {rx}         (not must, IOP_T_PUSH_LO  , case 1)
+   * 4  tpush {rx}         (not must, IOP_T_PUSH_LO  , case 1)
+   * 5  ...
+   * 6  tpush {rx}         (not must, IOP_T_PUSH_LO  , case 1)
+   * 7  other instructions
+   * 8  tsub  sp, #n       (not must, IOP_T_SUB_SP_16, case 0)
+   */
+
   for (i = 0; i < INSTR_LIMIT; i += 2)
     {
       base  = pc - i;
       ins16 = *(uint16_t *)(base);
-      if (INSTR_IS(ins16, T_PUSH))
+      switch (state)
         {
-          /* Bit 1 number in low byte indicates the number of pushed
-           * low register, +1 for LR is alao pushed into the stack.
-           */
-
-          frame = __builtin_popcount(ins16 & 0xff) + 1;
-          ins16 = *(uint16_t *)(base - 2);
-          if (INSTR_IS(ins16, T_PUSH_LO))
-            {
-              offset += __builtin_popcount(ins16 & 0xff);
-              frame  += offset - 1;
-            }
-
-          found = true;
+          case 0:
+            if (INSTR_IS(ins16, T_SUB_SP_16))
+              {
+                /* Low 7 bit (imm7) is the number (4*imm7) sub to sp */
+
+                frame += (ins16 & 0x7f);
+                state  = 1;
+                break;
+              }

Review Comment:
   is this intentional fall-thru?



##########
arch/arm/src/tlsr82/tc32/tc32_backtrace.c:
##########
@@ -63,6 +63,22 @@
 
 static void **g_backtrace_code_regions;
 
+/****************************************************************************
+ * Inline functions
+ ****************************************************************************/
+
+static inline uint32_t tc32_getsp(void)

Review Comment:
   why not `__builtin_frame_address(0)`?



-- 
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] [nuttx] CV-Bowen commented on a diff in pull request #8252: tlsr82/backtrace: tc32 backtrace bug fix

Posted by "CV-Bowen (via GitHub)" <gi...@apache.org>.
CV-Bowen commented on code in PR #8252:
URL: https://github.com/apache/nuttx/pull/8252#discussion_r1090145192


##########
arch/arm/src/tlsr82/tc32/tc32_backtrace.c:
##########
@@ -185,25 +201,53 @@ static void *backtrace_push_internal(void **psp, void **ppc)
 
   found = false;
 
+  /* Stack frame increase order:
+   * <function>:
+   * 1  tpush {rx, lr}     (must    , IOP_T_PUSH     , case 2)
+   * 2  other instructions
+   * 3  tpush {rx}         (not must, IOP_T_PUSH_LO  , case 1)
+   * 4  tpush {rx}         (not must, IOP_T_PUSH_LO  , case 1)
+   * 5  ...
+   * 6  tpush {rx}         (not must, IOP_T_PUSH_LO  , case 1)
+   * 7  other instructions
+   * 8  tsub  sp, #n       (not must, IOP_T_SUB_SP_16, case 0)
+   */
+
   for (i = 0; i < INSTR_LIMIT; i += 2)
     {
       base  = pc - i;
       ins16 = *(uint16_t *)(base);
-      if (INSTR_IS(ins16, T_PUSH))
+      switch (state)
         {
-          /* Bit 1 number in low byte indicates the number of pushed
-           * low register, +1 for LR is alao pushed into the stack.
-           */
-
-          frame = __builtin_popcount(ins16 & 0xff) + 1;
-          ins16 = *(uint16_t *)(base - 2);
-          if (INSTR_IS(ins16, T_PUSH_LO))
-            {
-              offset += __builtin_popcount(ins16 & 0xff);
-              frame  += offset - 1;
-            }
-
-          found = true;
+          case 0:
+            if (INSTR_IS(ins16, T_SUB_SP_16))
+              {
+                /* Low 7 bit (imm7) is the number (4*imm7) sub to sp */
+
+                frame += (ins16 & 0x7f);
+                state  = 1;
+                break;
+              }

Review Comment:
   Yes, seems add a comment here is better.



-- 
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] [nuttx] acassis merged pull request #8252: tlsr82/backtrace: tc32 backtrace bug fix

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #8252:
URL: https://github.com/apache/nuttx/pull/8252


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