[HELP] <title> arch/arm/src/armv6-m/arm_svcall.c There seems to be a problem

[blacksun-V]

Description

There may be a bug in this code.

---

#ifdef CONFIG_LIB_SYSCALL
static void dispatch_syscall(void) naked_function;
static void dispatch_syscall(void)
{
asm volatile
(
" push {r4, r5}\n" /* Save R4 and R5 /
" sub sp, sp, #12\n" / Create a stack frame to hold 3 parms 这条指令调整堆栈指针（sp），为传递的三个参数腾出空间。每个参数占 4 字节，总共是 12 字节，创建了一个新的栈帧。/
" str r4, [sp, #0]\n" / Move parameter 4 (if any) into position /
" str r5, [sp, #4]\n" / Move parameter 5 (if any) into position /
" str r6, [sp, #8]\n" / Move parameter 6 (if any) into position /
" mov r5, lr\n" / Save lr in R5 这条指令将链接寄存器（lr，Link Register）的值复制到 r5 中。lr 存储的是当前函数返回时的地址。保存它是为了在系统调用返回后恢复。/
" ldr r4, =g_stublookup\n" / R4=The base of the stub lookup table /
" lsl r0, r0, #2\n" / R0=Offset of the stub for this syscall 这条指令将寄存器 r0 的值左移 2 位，相当于乘以 4。由于每个函数指针占 4 字节，这样的操作可以计算出当前系统调用的偏移量。/
" ldr r4, [r4, r0]\n" / R4=Address of the stub for this syscall 从 g_stublookup 数组中加载对应系统调用的处理函数的地址到 r4 中*/
" blx r5\n" /* Call the stub (modifies lr) 这条指令使用 blx 指令跳转到寄存器 r5 中保存的地址，并调用系统调用处理函数。blx 是带有链接的跳转指令，它会将返回地址保存在 lr 中，跳转到 r5 保存的地址。/
" mov lr, r5\n" / Restore lr 这条指令将 r5 中保存的返回地址恢复到 lr 中。这是为了确保在系统调用完成后能够正确返回到调用点。/
" add sp, sp, #12\n" / Destroy the stack frame 这条指令恢复栈指针（sp），释放之前为保存参数所分配的 12 字节空间。/
" pop {r4, r5}\n" / Recover R4 and R5 这条指令将之前保存的 r4 和 r5 的值从栈中恢复到寄存器中，恢复调用前的状态。/
" mov r2, r0\n" / R2=Save return value in R2 将返回值（存储在 r0 中）保存到 r2 中。这是为了在之后的系统调用中使用。/
" mov r0, #" STRINGIFY(SYS_syscall_return) "\n" / R0=SYS_syscall_return 这条指令将 SYS_syscall_return 的值加载到 r0 中。SYS_syscall_return 是一个表示系统调用返回的常量。/
" svc #" STRINGIFY(SYS_syscall) "\n" / 相当于 int 0x80; Return from the SYSCALL 这条指令触发一个软件中断（svc），调用系统调用返回机制，并将 r0 中的值传递给内核。这表示系统调用完成后要返回到用户空间。*/
);
}
#endif

---

blx r5\n" /* Call the stub (modifies lr) */

---

This should be blx r4 ,but not r5

Verification

• I have verified before submitting the report.

[acassis]

@guoshichao @xiaoxiang781216 could you please take a look?

[extinguish]

@guoshichao @xiaoxiang781216 could you please take a look?

@patacongo Please help to take a look, in the commit of bd1488b, the process of calling the sub-routine in the implementation of dispatch_syscall is as follows:

+  __asm__ __volatile__
+  (
+    " push {r4-r6}\n"             /* Save R4, R5 and R6 */
+    " mov r6, r14\n"              /* Save LR in R6 */
+    " ldr r4, =g_stublookup\n"    /* Get the base of the stub lookup table */
+    " lsl r3, r0, #2\n"           /* Get the offset of the stub for this syscall */
+    " ldr r3, [r4, r3]\n"         /* Load the entry of the stub for this syscall */
+    " blx r3\n"                   /* Call the stub */
+    " mov r14, r6\n"              /* Restore R14 */
+    " pop {r4-r6}\n"              /* Restore R4, R5, and R6 */
+    " mov r2, r0\n"               /* Save the return value in R0 in R2 for now */
+    " mov r0, #3\n"               /* R0=SYS_syscall_return */
+    " svc 0"                      /* Return from the syscall */
+    :::
+  );

However, in a subsequent commit 686f21d, the implementation of calling the sub-routine within dispatch_syscall was modified to:

-    " push {r4-r6}\n"             /* Save R4, R5 and R6 */
-    " mov r6, r14\n"              /* Save LR in R6 */
-    " ldr r4, =g_stublookup\n"    /* Get the base of the stub lookup table */
-    " lsl r3, r0, #2\n"           /* Get the offset of the stub for this syscall */
-    " ldr r3, [r4, r3]\n"         /* Load the entry of the stub for this syscall */
-    " blx r3\n"                   /* Call the stub */
-    " mov r14, r6\n"              /* Restore R14 */
-    " pop {r4-r6}\n"              /* Restore R4, R5, and R6 */
-    " mov r2, r0\n"               /* Save the return value in R0 in R2 for now */
+    " push {r4}\n"                /* Save R4 */
+    " ldr r4, =g_stublookup\n"    /* R4=The base of the stub lookup table */
+    " lsl r0, r0, #2\n"           /* R0=Offset of the stub for this syscall */
+    " ldr r4, [r4, r0]\n"         /* R4=Address of the stub for this syscall */
+    " blx r5\n"                   /* Call the stub (modifies R14) */
+    " pop {r4}\n"                 /* Restore R4 */
+    " mov r2, r0\n"               /* R2=Saves return value in R0 */
     " mov r0, #3\n"               /* R0=SYS_syscall_return */
     " svc 0"                      /* Return from the syscall */

The process of calling the sub-routine has changed from
blx r3 # Here, the value stored in r3 is still the entry point of the sub-routine
to
blx r5 # Here, the value stored in r5 is the value of lr.

Why has it been changed like this?

[patacongo]

@patacongo Please help to take a look, in the commit of bd1488b, the process of calling the sub-routine in the implementation of dispatch_syscall is as follows

That change was from a very long time ago. I don't remember much in detail.

The original was in C. That was converted to assembly in bd1488b.

A subsequent commit just fixed some assembly language errors.