[PATCH] common-user: Really fix i386 calls to safe_syscall_set_errno_tail

Richard Henderson posted 1 patch 2 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220105052339.677395-1-richard.henderson@linaro.org
Maintainers: Riku Voipio <riku.voipio@iki.fi>
common-user/host/i386/safe-syscall.inc.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] common-user: Really fix i386 calls to safe_syscall_set_errno_tail
Posted by Richard Henderson 2 years, 3 months ago
Brown bag time: offset 0 from esp is the return address,
offset 4 is the first argument.

Fixes: d7478d4229f0 ("common-user: Fix tail calls to safe_syscall_set_errno_tail")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

Ho hum.  I'm disappointed that our CI didn't catch this,
despite cross-i386-user.  And I'm disappointed that I
didn't run a full manual build on an i386 vm to catch
it myself.  I plan on committing this directly.

---
 common-user/host/i386/safe-syscall.inc.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common-user/host/i386/safe-syscall.inc.S b/common-user/host/i386/safe-syscall.inc.S
index 9c45e56e480..db2ed098394 100644
--- a/common-user/host/i386/safe-syscall.inc.S
+++ b/common-user/host/i386/safe-syscall.inc.S
@@ -120,7 +120,7 @@ safe_syscall_end:
         pop     %ebp
         .cfi_adjust_cfa_offset -4
         .cfi_restore ebp
-        mov     %eax, (%esp)
+        mov     %eax, 4(%esp)
         jmp     safe_syscall_set_errno_tail
 
         .cfi_endproc
-- 
2.25.1


Re: [PATCH] common-user: Really fix i386 calls to safe_syscall_set_errno_tail
Posted by Philippe Mathieu-Daudé 2 years, 3 months ago
On 1/5/22 06:23, Richard Henderson wrote:
> Brown bag time: offset 0 from esp is the return address,
> offset 4 is the first argument.
> 
> Fixes: d7478d4229f0 ("common-user: Fix tail calls to safe_syscall_set_errno_tail")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> Ho hum.  I'm disappointed that our CI didn't catch this,
> despite cross-i386-user.  And I'm disappointed that I
> didn't run a full manual build on an i386 vm to catch
> it myself.  I plan on committing this directly.

Sorry for missing this myself too while reviewing, x86 is my weakness.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  common-user/host/i386/safe-syscall.inc.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common-user/host/i386/safe-syscall.inc.S b/common-user/host/i386/safe-syscall.inc.S
> index 9c45e56e480..db2ed098394 100644
> --- a/common-user/host/i386/safe-syscall.inc.S
> +++ b/common-user/host/i386/safe-syscall.inc.S
> @@ -120,7 +120,7 @@ safe_syscall_end:
>          pop     %ebp
>          .cfi_adjust_cfa_offset -4
>          .cfi_restore ebp
> -        mov     %eax, (%esp)
> +        mov     %eax, 4(%esp)
>          jmp     safe_syscall_set_errno_tail
>  
>          .cfi_endproc