linux-user: Move syscall error detection into safe_syscall_base

The current api from safe_syscall_base() is to return -errno, which is
the interface provided by *some* linux kernel abis.  The wrapper macro,
safe_syscall(), detects error, stores into errno, and returns -1, to
match the api of the system syscall().

For those kernel abis that do not return -errno natively, this leads
to double syscall error detection.  E.g. Linux ppc64, which sets the
SO flag for error.

Simplify the usage from C by moving the error detection into assembly,
and usage from assembly by providing a C helper with which to set errno.

Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

10 files changed, 144 insertions, 84 deletions

diff --git a/linux-user/host/aarch64/safe-syscall.inc.S b/linux-user/host/aarch64/safe-syscall.inc.S
index e2e726e..76a0a18 100644
--- a/linux-user/host/aarch64/safe-syscall.inc.S
+++ b/linux-user/host/aarch64/safe-syscall.inc.S
@@ -22,15 +22,12 @@
          * first argument an 'int *' to the signal_pending flag, the
          * second one the system call number (as a 'long'), and all further
          * arguments being syscall arguments (also 'long').
-         * We return a long which is the syscall's return value, which
-         * may be negative-errno on failure. Conversion to the
-         * -1-and-errno-set convention is done by the calling wrapper.
          */
 safe_syscall_base:
         .cfi_startproc
         /* The syscall calling convention isn't the same as the
          * C one:
-         * we enter with x0 == *signal_pending
+         * we enter with x0 == &signal_pending
          *               x1 == syscall number
          *               x2 ... x7, (stack) == syscall arguments
          *               and return the result in x0
@@ -60,16 +57,21 @@ safe_syscall_base:
 safe_syscall_start:
         /* if signal_pending is non-zero, don't do the call */
         ldr     w10, [x9]
-        cbnz    w10, 0f
+        cbnz    w10, 2f
         svc     0x0
 safe_syscall_end:
         /* code path for having successfully executed the syscall */
+        cmp     x0, #-4096
+        b.hi    0f
         ret
 
-0:
+        /* code path setting errno */
+0:      neg     w0, w0
+        b       safe_syscall_set_errno_tail
+
         /* code path when we didn't execute the syscall */
-        mov     x0, #-TARGET_ERESTARTSYS
-        ret
-        .cfi_endproc
+2:      mov     w0, #TARGET_ERESTARTSYS
+        b       safe_syscall_set_errno_tail
 
+        .cfi_endproc
         .size   safe_syscall_base, .-safe_syscall_base
diff --git a/linux-user/host/arm/safe-syscall.inc.S b/linux-user/host/arm/safe-syscall.inc.S
index 1f1ee83..618112c 100644
--- a/linux-user/host/arm/safe-syscall.inc.S
+++ b/linux-user/host/arm/safe-syscall.inc.S
@@ -27,9 +27,6 @@
          * first argument an 'int *' to the signal_pending flag, the
          * second one the system call number (as a 'long'), and all further
          * arguments being syscall arguments (also 'long').
-         * We return a long which is the syscall's return value, which
-         * may be negative-errno on failure. Conversion to the
-         * -1-and-errno-set convention is done by the calling wrapper.
          */
 safe_syscall_base:
         .fnstart
@@ -46,7 +43,7 @@ safe_syscall_base:
         .cfi_rel_offset lr, 20
 
         /* The syscall calling convention isn't the same as the C one:
-         * we enter with r0 == *signal_pending
+         * we enter with r0 == &signal_pending
          *               r1 == syscall number
          *               r2, r3, [sp+0] ... [sp+12] == syscall arguments
          *               and return the result in r0
@@ -74,17 +71,29 @@ safe_syscall_start:
         /* if signal_pending is non-zero, don't do the call */
         ldr     r12, [r8]               /* signal_pending */
         tst     r12, r12
-        bne     1f
+        bne     2f
         swi     0
 safe_syscall_end:
         /* code path for having successfully executed the syscall */
+        cmp     r0, #-4096
+        neghi   r0, r0
+        bhi     1f
         pop     { r4, r5, r6, r7, r8, pc }
 
-1:
         /* code path when we didn't execute the syscall */
-        ldr     r0, =-TARGET_ERESTARTSYS
-        pop     { r4, r5, r6, r7, r8, pc }
+2:      mov     r0, #TARGET_ERESTARTSYS
+
+        /* code path setting errno */
+1:      pop     { r4, r5, r6, r7, r8, lr }
+        .cfi_adjust_cfa_offset -24
+        .cfi_restore r4
+        .cfi_restore r5
+        .cfi_restore r6
+        .cfi_restore r7
+        .cfi_restore r8
+        .cfi_restore lr
+        b       safe_syscall_set_errno_tail
+
         .fnend
         .cfi_endproc
-
         .size   safe_syscall_base, .-safe_syscall_base
diff --git a/linux-user/host/i386/safe-syscall.inc.S b/linux-user/host/i386/safe-syscall.inc.S
index e425aa5..f588323 100644
--- a/linux-user/host/i386/safe-syscall.inc.S
+++ b/linux-user/host/i386/safe-syscall.inc.S
@@ -20,9 +20,6 @@
          * first argument an 'int *' to the signal_pending flag, the
          * second one the system call number (as a 'long'), and all further
          * arguments being syscall arguments (also 'long').
-         * We return a long which is the syscall's return value, which
-         * may be negative-errno on failure. Conversion to the
-         * -1-and-errno-set convention is done by the calling wrapper.
          */
 safe_syscall_base:
         .cfi_startproc
@@ -41,7 +38,7 @@ safe_syscall_base:
 
         /* The syscall calling convention isn't the same as the C one:
          * we enter with 0(%esp) == return address
-         *               4(%esp) == *signal_pending
+         *               4(%esp) == &signal_pending
          *               8(%esp) == syscall number
          *               12(%esp) ... 32(%esp) == syscall arguments
          *               and return the result in eax
@@ -70,11 +67,13 @@ safe_syscall_start:
         /* if signal_pending is non-zero, don't do the call */
         mov     4+16(%esp), %eax        /* signal_pending */
         cmpl    $0, (%eax)
-        jnz     1f
+        jnz     2f
         mov     8+16(%esp), %eax        /* syscall number */
         int     $0x80
 safe_syscall_end:
         /* code path for having successfully executed the syscall */
+        cmp     $-4095, %eax
+        jae     0f
         pop     %ebx
         .cfi_remember_state
         .cfi_adjust_cfa_offset -4
@@ -89,12 +88,28 @@ safe_syscall_end:
         .cfi_adjust_cfa_offset -4
         .cfi_restore ebp
         ret
+        .cfi_restore_state
+
+0:      neg     %eax
+        jmp     1f
 
-1:
         /* code path when we didn't execute the syscall */
-        .cfi_restore_state
-        mov     $-TARGET_ERESTARTSYS, %eax
-        jmp     safe_syscall_end
-        .cfi_endproc
+2:      mov     $TARGET_ERESTARTSYS, %eax
 
+        /* code path setting errno */
+1:      pop     %ebx
+        .cfi_adjust_cfa_offset -4
+        .cfi_restore ebx
+        pop     %edi
+        .cfi_adjust_cfa_offset -4
+        .cfi_restore edi
+        pop     %esi
+        .cfi_adjust_cfa_offset -4
+        .cfi_restore esi
+        pop     %ebp
+        .cfi_adjust_cfa_offset -4
+        .cfi_restore ebp
+        jmp     safe_syscall_set_errno_tail
+
+        .cfi_endproc
         .size   safe_syscall_base, .-safe_syscall_base
diff --git a/linux-user/host/ppc64/safe-syscall.inc.S b/linux-user/host/ppc64/safe-syscall.inc.S
index 4b57440..3a640cf 100644
--- a/linux-user/host/ppc64/safe-syscall.inc.S
+++ b/linux-user/host/ppc64/safe-syscall.inc.S
@@ -22,9 +22,6 @@
          * first argument an 'int *' to the signal_pending flag, the
          * second one the system call number (as a 'long'), and all further
          * arguments being syscall arguments (also 'long').
-         * We return a long which is the syscall's return value, which
-         * may be negative-errno on failure. Conversion to the
-         * -1-and-errno-set convention is done by the calling wrapper.
          */
 #if _CALL_ELF == 2
 safe_syscall_base:
@@ -39,7 +36,7 @@ safe_syscall_base:
 .L.safe_syscall_base:
         .cfi_startproc
 #endif
-        /* We enter with r3 == *signal_pending
+        /* We enter with r3 == &signal_pending
          *               r4 == syscall number
          *               r5 ... r10 == syscall arguments
          *               and return the result in r3
@@ -71,21 +68,22 @@ safe_syscall_start:
         /* if signal_pending is non-zero, don't do the call */
         lwz     12, 0(14)
         cmpwi   0, 12, 0
-        bne-    0f
+        bne-    2f
         sc
 safe_syscall_end:
         /* code path when we did execute the syscall */
-        ld 14, 16(1) /* restore r14 to its original value */
-        bnslr+
-
-        /* syscall failed; return negative errno */
-        neg     3, 3
+        ld      14, 16(1) /* restore r14 */
+        bso-    1f
         blr
 
         /* code path when we didn't execute the syscall */
-0:      addi    3, 0, -TARGET_ERESTARTSYS
-        ld 14, 16(1) /* restore r14 to its original value */
-        blr
+2:      ld      14, 16(1) /* restore r14 */
+        addi    3, 0, TARGET_ERESTARTSYS
+
+        /* code path setting errno */
+1:      b       safe_syscall_set_errno_tail
+        nop     /* per abi, for the linker to modify */
+
         .cfi_endproc
 
 #if _CALL_ELF == 2
diff --git a/linux-user/host/riscv/safe-syscall.inc.S b/linux-user/host/riscv/safe-syscall.inc.S
index 95c4832..54c2e23 100644
--- a/linux-user/host/riscv/safe-syscall.inc.S
+++ b/linux-user/host/riscv/safe-syscall.inc.S
@@ -23,15 +23,12 @@
          * first argument an 'int *' to the signal_pending flag, the
          * second one the system call number (as a 'long'), and all further
          * arguments being syscall arguments (also 'long').
-         * We return a long which is the syscall's return value, which
-         * may be negative-errno on failure. Conversion to the
-         * -1-and-errno-set convention is done by the calling wrapper.
          */
 safe_syscall_base:
         .cfi_startproc
         /*
          * The syscall calling convention is nearly the same as C:
-         * we enter with a0 == *signal_pending
+         * we enter with a0 == &signal_pending
          *               a1 == syscall number
          *               a2 ... a7 == syscall arguments
          *               and return the result in a0
@@ -62,16 +59,21 @@ safe_syscall_base:
 safe_syscall_start:
         /* If signal_pending is non-zero, don't do the call */
         lw      t1, 0(t0)
-        bnez    t1, 0f
+        bnez    t1, 2f
         scall
 safe_syscall_end:
         /* code path for having successfully executed the syscall */
+        li      t2, -4096
+        bgtu    a0, t2, 0f
         ret
 
-0:
+        /* code path setting errno */
+0:      neg     a0, a0
+        j       safe_syscall_set_errno_tail
+
         /* code path when we didn't execute the syscall */
-        li      a0, -TARGET_ERESTARTSYS
-        ret
-        .cfi_endproc
+2:      li      a0, TARGET_ERESTARTSYS
+        j       safe_syscall_set_errno_tail
 
+        .cfi_endproc
         .size   safe_syscall_base, .-safe_syscall_base
diff --git a/linux-user/host/s390x/safe-syscall.inc.S b/linux-user/host/s390x/safe-syscall.inc.S
index d97d274..899dab3 100644
--- a/linux-user/host/s390x/safe-syscall.inc.S
+++ b/linux-user/host/s390x/safe-syscall.inc.S
@@ -20,9 +20,6 @@
          * first argument an 'int *' to the signal_pending flag, the
          * second one the system call number (as a 'long'), and all further
          * arguments being syscall arguments (also 'long').
-         * We return a long which is the syscall's return value, which
-         * may be negative-errno on failure. Conversion to the
-         * -1-and-errno-set convention is done by the calling wrapper.
          */
 safe_syscall_base:
         .cfi_startproc
@@ -44,9 +41,9 @@ safe_syscall_base:
         stg     %r1,0(%r15)             /* store back chain */
         stg     %r0,8(%r15)             /* store eos */
 
-        /* The syscall calling convention isn't the same as the
-         * C one:
-         * we enter with r2 == *signal_pending
+        /*
+         * The syscall calling convention isn't the same as the C one:
+         * we enter with r2 == &signal_pending
          *               r3 == syscall number
          *               r4, r5, r6, (stack) == syscall arguments
          *               and return the result in r2
@@ -77,14 +74,25 @@ safe_syscall_start:
         svc     0
 safe_syscall_end:
 
-1:      lg      %r15,0(%r15)            /* load back chain */
+        /* code path for having successfully executed the syscall */
+        lg      %r15,0(%r15)            /* load back chain */
         .cfi_remember_state
         .cfi_adjust_cfa_offset -160
         lmg     %r6,%r15,48(%r15)       /* load saved registers */
-        br      %r14
+
+        lghi    %r0, -4095              /* check for syscall error */
+        clgr    %r2, %r0
+        blr     %r14                    /* return on success */
+        lcr     %r2, %r2                /* create positive errno */
+        jg      safe_syscall_set_errno_tail
         .cfi_restore_state
-2:      lghi    %r2, -TARGET_ERESTARTSYS
-        j       1b
-        .cfi_endproc
 
+        /* code path when we didn't execute the syscall */
+2:      lg      %r15,0(%r15)            /* load back chain */
+        .cfi_adjust_cfa_offset -160
+        lmg     %r6,%r15,48(%r15)       /* load saved registers */
+        lghi    %r2, TARGET_ERESTARTSYS
+        jg      safe_syscall_set_errno_tail
+
+        .cfi_endproc
         .size   safe_syscall_base, .-safe_syscall_base
diff --git a/linux-user/host/x86_64/safe-syscall.inc.S b/linux-user/host/x86_64/safe-syscall.inc.S
index 1582255..39b6425 100644
--- a/linux-user/host/x86_64/safe-syscall.inc.S
+++ b/linux-user/host/x86_64/safe-syscall.inc.S
@@ -19,9 +19,6 @@
          * first argument an 'int *' to the signal_pending flag, the
          * second one the system call number (as a 'long'), and all further
          * arguments being syscall arguments (also 'long').
-         * We return a long which is the syscall's return value, which
-         * may be negative-errno on failure. Conversion to the
-         * -1-and-errno-set convention is done by the calling wrapper.
          */
 safe_syscall_base:
         .cfi_startproc
@@ -35,9 +32,9 @@ safe_syscall_base:
         .cfi_adjust_cfa_offset 8
         .cfi_rel_offset rbp, 0
 
-        /* The syscall calling convention isn't the same as the
-         * C one:
-         * we enter with rdi == *signal_pending
+        /*
+         * The syscall calling convention isn't the same as the C one:
+         * we enter with rdi == &signal_pending
          *               rsi == syscall number
          *               rdx, rcx, r8, r9, (stack), (stack) == syscall arguments
          *               and return the result in rax
@@ -68,24 +65,30 @@ safe_syscall_base:
 safe_syscall_start:
         /* if signal_pending is non-zero, don't do the call */
         cmpl    $0, (%rbp)
-        jnz     1f
+        jnz     2f
         syscall
 safe_syscall_end:
         /* code path for having successfully executed the syscall */
+        cmp     $-4095, %rax
+        jae     0f
         pop     %rbp
         .cfi_remember_state
         .cfi_def_cfa_offset 8
         .cfi_restore rbp
         ret
+        .cfi_restore_state
+
+0:      neg     %eax
+        jmp     1f
 
-1:
         /* code path when we didn't execute the syscall */
-        .cfi_restore_state
-        mov     $-TARGET_ERESTARTSYS, %rax
-        pop     %rbp
+2:      mov     $TARGET_ERESTARTSYS, %eax
+
+        /* code path setting errno */
+1:      pop     %rbp
         .cfi_def_cfa_offset 8
         .cfi_restore rbp
-        ret
+        jmp     safe_syscall_set_errno_tail
         .cfi_endproc
 
         .size   safe_syscall_base, .-safe_syscall_base
diff --git a/linux-user/meson.build b/linux-user/meson.build
index bf62c13..94ac3c5 100644
--- a/linux-user/meson.build
+++ b/linux-user/meson.build
@@ -10,6 +10,7 @@ linux_user_ss.add(files(
   'main.c',
   'mmap.c',
   'safe-syscall.S',
+  'safe-syscall-error.c',
   'signal.c',
   'strace.c',
   'syscall.c',
diff --git a/linux-user/safe-syscall-error.c b/linux-user/safe-syscall-error.c
new file mode 100644
index 0000000..d7e2700
--- /dev/null
+++ b/linux-user/safe-syscall-error.c
@@ -0,0 +1,28 @@
+/*
+ * safe-syscall-error.c: errno setting fragment
+ * This is intended to be invoked by safe-syscall.S
+ *
+ * Written by Richard Henderson <rth@twiddle.net>
+ * Copyright (C) 2021 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hostdep.h"
+#include "safe-syscall.h"
+
+#ifdef HAVE_SAFE_SYSCALL
+/*
+ * This is intended to be invoked via tail-call on the error path
+ * from the assembly in host/arch/safe-syscall.inc.S.  This takes
+ * care of the host specific addressing of errno.
+ * Return -1 to finalize the return value for safe_syscall_base.
+ */
+long safe_syscall_set_errno_tail(int value)
+{
+    errno = value;
+    return -1;
+}
+#endif
diff --git a/linux-user/safe-syscall.h b/linux-user/safe-syscall.h
index aaa9ffc..97837fa 100644
--- a/linux-user/safe-syscall.h
+++ b/linux-user/safe-syscall.h
@@ -127,21 +127,15 @@
 #ifdef HAVE_SAFE_SYSCALL
 /* The core part of this function is implemented in assembly */
 extern long safe_syscall_base(int *pending, long number, ...);
+extern long safe_syscall_set_errno_tail(int value);
+
 /* These are defined by the safe-syscall.inc.S file */
 extern char safe_syscall_start[];
 extern char safe_syscall_end[];
 
-#define safe_syscall(...)                                               \
-    ({                                                                  \
-        long ret_;                                                      \
-        int *psp_ = &((TaskState *)thread_cpu->opaque)->signal_pending; \
-        ret_ = safe_syscall_base(psp_, __VA_ARGS__);                    \
-        if (is_error(ret_)) {                                           \
-            errno = -ret_;                                              \
-            ret_ = -1;                                                  \
-        }                                                               \
-        ret_;                                                           \
-    })
+#define safe_syscall(...)                                                 \
+    safe_syscall_base(&((TaskState *)thread_cpu->opaque)->signal_pending, \
+                      __VA_ARGS__)
 
 #else