[PATCH 0/8] linux-user: Futex improvements

[PATCH 0/8] linux-user: Futex improvements

[Richard Henderson]

Fixes a bug in FUTEX_FD, and implements a bunch of other ops.
Also, some assorted strace fixes.

r~

Richard Henderson (8):
  linux-user: Combine do_futex and do_futex_time64
  linux-user: Sink call to do_safe_futex
  linux-user: Implement FUTEX_WAKE_BITSET
  linux-user: Convert signal number for FUTEX_FD
  linux-user: Implement PI futexes
  linux-user: Update print_futex_op
  linux-user: Lock log around strace
  linux-user: Log tid for strace

 linux-user/syscall_defs.h |   3 +
 linux-user/strace.c       | 130 ++++++++++++++++++++++----------------
 linux-user/syscall.c      | 125 ++++++++++++++----------------------
 3 files changed, 125 insertions(+), 133 deletions(-)

-- 
2.34.1

[PATCH 1/8] linux-user: Combine do_futex and do_futex_time64

[Richard Henderson]

Pass a boolean to select between time32 and time64.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 67 ++++++++------------------------------------
 1 file changed, 11 insertions(+), 56 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f409121202..a7e66d8d28 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7753,9 +7753,10 @@ static int do_safe_futex(int *uaddr, int op, int val,
    futexes locally would make futexes shared between multiple processes
    tricky.  However they're probably useless because guest atomic
    operations won't work either.  */
-#if defined(TARGET_NR_futex)
-static int do_futex(CPUState *cpu, target_ulong uaddr, int op, int val,
-                    target_ulong timeout, target_ulong uaddr2, int val3)
+#if defined(TARGET_NR_futex) || defined(TARGET_NR_futex_time64)
+static int do_futex(CPUState *cpu, bool time64, target_ulong uaddr,
+                    int op, int val, target_ulong timeout,
+                    target_ulong uaddr2, int val3)
 {
     struct timespec ts, *pts;
     int base_op;
@@ -7772,7 +7773,11 @@ static int do_futex(CPUState *cpu, target_ulong uaddr, int op, int val,
     case FUTEX_WAIT_BITSET:
         if (timeout) {
             pts = &ts;
-            target_to_host_timespec(pts, timeout);
+            if (time64
+                ? target_to_host_timespec64(pts, timeout)
+                : target_to_host_timespec(pts, timeout)) {
+                return -TARGET_EFAULT;
+            }
         } else {
             pts = NULL;
         }
@@ -7802,56 +7807,6 @@ static int do_futex(CPUState *cpu, target_ulong uaddr, int op, int val,
 }
 #endif
 
-#if defined(TARGET_NR_futex_time64)
-static int do_futex_time64(CPUState *cpu, target_ulong uaddr, int op,
-                           int val, target_ulong timeout,
-                           target_ulong uaddr2, int val3)
-{
-    struct timespec ts, *pts;
-    int base_op;
-
-    /* ??? We assume FUTEX_* constants are the same on both host
-       and target.  */
-#ifdef FUTEX_CMD_MASK
-    base_op = op & FUTEX_CMD_MASK;
-#else
-    base_op = op;
-#endif
-    switch (base_op) {
-    case FUTEX_WAIT:
-    case FUTEX_WAIT_BITSET:
-        if (timeout) {
-            pts = &ts;
-            if (target_to_host_timespec64(pts, timeout)) {
-                return -TARGET_EFAULT;
-            }
-        } else {
-            pts = NULL;
-        }
-        return do_safe_futex(g2h(cpu, uaddr), op,
-                             tswap32(val), pts, NULL, val3);
-    case FUTEX_WAKE:
-        return do_safe_futex(g2h(cpu, uaddr), op, val, NULL, NULL, 0);
-    case FUTEX_FD:
-        return do_safe_futex(g2h(cpu, uaddr), op, val, NULL, NULL, 0);
-    case FUTEX_REQUEUE:
-    case FUTEX_CMP_REQUEUE:
-    case FUTEX_WAKE_OP:
-        /* For FUTEX_REQUEUE, FUTEX_CMP_REQUEUE, and FUTEX_WAKE_OP, the
-           TIMEOUT parameter is interpreted as a uint32_t by the kernel.
-           But the prototype takes a `struct timespec *'; insert casts
-           to satisfy the compiler.  We do not need to tswap TIMEOUT
-           since it's not compared to guest memory.  */
-        pts = (struct timespec *)(uintptr_t) timeout;
-        return do_safe_futex(g2h(cpu, uaddr), op, val, pts, g2h(cpu, uaddr2),
-                             (base_op == FUTEX_CMP_REQUEUE
-                              ? tswap32(val3) : val3));
-    default:
-        return -TARGET_ENOSYS;
-    }
-}
-#endif
-
 #if defined(TARGET_NR_name_to_handle_at) && defined(CONFIG_OPEN_BY_HANDLE)
 static abi_long do_name_to_handle_at(abi_long dirfd, abi_long pathname,
                                      abi_long handle, abi_long mount_id,
@@ -12318,11 +12273,11 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_futex
     case TARGET_NR_futex:
-        return do_futex(cpu, arg1, arg2, arg3, arg4, arg5, arg6);
+        return do_futex(cpu, false, arg1, arg2, arg3, arg4, arg5, arg6);
 #endif
 #ifdef TARGET_NR_futex_time64
     case TARGET_NR_futex_time64:
-        return do_futex_time64(cpu, arg1, arg2, arg3, arg4, arg5, arg6);
+        return do_futex(cpu, true, arg1, arg2, arg3, arg4, arg5, arg6);
 #endif
 #ifdef CONFIG_INOTIFY
 #if defined(TARGET_NR_inotify_init)
-- 
2.34.1

Re: [PATCH 1/8] linux-user: Combine do_futex and do_futex_time64

[Laurent Vivier]

Le 29/08/2022 à 04:09, Richard Henderson a écrit :
> Pass a boolean to select between time32 and time64.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall.c | 67 ++++++++------------------------------------
>   1 file changed, 11 insertions(+), 56 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[PATCH 2/8] linux-user: Sink call to do_safe_futex

[Richard Henderson]

Leave only the argument adjustments within the shift,
and sink the actual syscall to the end.  Sink the
timespec conversion as well, as there will be more users.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 60 +++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a7e66d8d28..8fbd5a9556 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7758,11 +7758,11 @@ static int do_futex(CPUState *cpu, bool time64, target_ulong uaddr,
                     int op, int val, target_ulong timeout,
                     target_ulong uaddr2, int val3)
 {
-    struct timespec ts, *pts;
+    struct timespec ts, *pts = NULL;
+    void *haddr2 = NULL;
     int base_op;
 
-    /* ??? We assume FUTEX_* constants are the same on both host
-       and target.  */
+    /* We assume FUTEX_* constants are the same on both host and target. */
 #ifdef FUTEX_CMD_MASK
     base_op = op & FUTEX_CMD_MASK;
 #else
@@ -7771,39 +7771,41 @@ static int do_futex(CPUState *cpu, bool time64, target_ulong uaddr,
     switch (base_op) {
     case FUTEX_WAIT:
     case FUTEX_WAIT_BITSET:
-        if (timeout) {
-            pts = &ts;
-            if (time64
-                ? target_to_host_timespec64(pts, timeout)
-                : target_to_host_timespec(pts, timeout)) {
-                return -TARGET_EFAULT;
-            }
-        } else {
-            pts = NULL;
-        }
-        return do_safe_futex(g2h(cpu, uaddr),
-                             op, tswap32(val), pts, NULL, val3);
+        val = tswap32(val);
+        break;
     case FUTEX_WAKE:
-        return do_safe_futex(g2h(cpu, uaddr),
-                             op, val, NULL, NULL, 0);
+        timeout = 0;
+        break;
     case FUTEX_FD:
-        return do_safe_futex(g2h(cpu, uaddr),
-                             op, val, NULL, NULL, 0);
-    case FUTEX_REQUEUE:
+        timeout = 0;
+        break;
     case FUTEX_CMP_REQUEUE:
+        val3 = tswap32(val3);
+        /* fall through */
+    case FUTEX_REQUEUE:
     case FUTEX_WAKE_OP:
-        /* For FUTEX_REQUEUE, FUTEX_CMP_REQUEUE, and FUTEX_WAKE_OP, the
-           TIMEOUT parameter is interpreted as a uint32_t by the kernel.
-           But the prototype takes a `struct timespec *'; insert casts
-           to satisfy the compiler.  We do not need to tswap TIMEOUT
-           since it's not compared to guest memory.  */
-        pts = (struct timespec *)(uintptr_t) timeout;
-        return do_safe_futex(g2h(cpu, uaddr), op, val, pts, g2h(cpu, uaddr2),
-                             (base_op == FUTEX_CMP_REQUEUE
-                              ? tswap32(val3) : val3));
+        /*
+         * For these, the 4th argument is not TIMEOUT, but VAL2.
+         * But the prototype of do_safe_futex takes a pointer, so
+         * insert casts to satisfy the compiler.  We do not need
+         * to tswap VAL2 since it's not compared to guest memory.
+          */
+        pts = (struct timespec *)(uintptr_t)timeout;
+        timeout = 0;
+        haddr2 = g2h(cpu, uaddr2);
+        break;
     default:
         return -TARGET_ENOSYS;
     }
+    if (timeout) {
+        pts = &ts;
+        if (time64
+            ? target_to_host_timespec64(pts, timeout)
+            : target_to_host_timespec(pts, timeout)) {
+            return -TARGET_EFAULT;
+        }
+    }
+    return do_safe_futex(g2h(cpu, uaddr), op, val, pts, haddr2, val3);
 }
 #endif
 
-- 
2.34.1

Re: [PATCH 2/8] linux-user: Sink call to do_safe_futex

[Laurent Vivier]

Le 29/08/2022 à 04:10, Richard Henderson a écrit :
> Leave only the argument adjustments within the shift,
> and sink the actual syscall to the end.  Sink the
> timespec conversion as well, as there will be more users.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall.c | 60 +++++++++++++++++++++++---------------------
>   1 file changed, 31 insertions(+), 29 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[PATCH 3/8] linux-user: Implement FUTEX_WAKE_BITSET

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8fbd5a9556..8bf4b79a9e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7774,6 +7774,7 @@ static int do_futex(CPUState *cpu, bool time64, target_ulong uaddr,
         val = tswap32(val);
         break;
     case FUTEX_WAKE:
+    case FUTEX_WAKE_BITSET:
         timeout = 0;
         break;
     case FUTEX_FD:
-- 
2.34.1

Re: [PATCH 3/8] linux-user: Implement FUTEX_WAKE_BITSET

[Laurent Vivier]

Le 29/08/2022 à 04:10, Richard Henderson a écrit :
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 8fbd5a9556..8bf4b79a9e 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7774,6 +7774,7 @@ static int do_futex(CPUState *cpu, bool time64, target_ulong uaddr,
>           val = tswap32(val);
>           break;
>       case FUTEX_WAKE:
> +    case FUTEX_WAKE_BITSET:
>           timeout = 0;
>           break;
>       case FUTEX_FD:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[PATCH 4/8] linux-user: Convert signal number for FUTEX_FD

[Richard Henderson]

The val argument to FUTEX_FD is a signal number.  Convert to match
the host, as it will be converted back when the signal is delivered.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8bf4b79a9e..f50cc70f1a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7778,6 +7778,7 @@ static int do_futex(CPUState *cpu, bool time64, target_ulong uaddr,
         timeout = 0;
         break;
     case FUTEX_FD:
+        val = target_to_host_signal(val);
         timeout = 0;
         break;
     case FUTEX_CMP_REQUEUE:
-- 
2.34.1

Re: [PATCH 4/8] linux-user: Convert signal number for FUTEX_FD

[Laurent Vivier]

Le 29/08/2022 à 04:10, Richard Henderson a écrit :
> The val argument to FUTEX_FD is a signal number.  Convert to match
> the host, as it will be converted back when the signal is delivered.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 8bf4b79a9e..f50cc70f1a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7778,6 +7778,7 @@ static int do_futex(CPUState *cpu, bool time64, target_ulong uaddr,
>           timeout = 0;
>           break;
>       case FUTEX_FD:
> +        val = target_to_host_signal(val);
>           timeout = 0;
>           break;
>       case FUTEX_CMP_REQUEUE:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[PATCH 5/8] linux-user: Implement PI futexes

[Richard Henderson]

Define the missing FUTEX_* constants in syscall_defs.h

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall_defs.h |  3 +++
 linux-user/syscall.c      | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 85b0f33e91..9a6d7893d9 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2699,6 +2699,9 @@ struct target_drm_i915_getparam {
 #define FUTEX_TRYLOCK_PI        8
 #define FUTEX_WAIT_BITSET       9
 #define FUTEX_WAKE_BITSET       10
+#define FUTEX_WAIT_REQUEUE_PI   11
+#define FUTEX_CMP_REQUEUE_PI    12
+#define FUTEX_LOCK_PI2          13
 
 #define FUTEX_PRIVATE_FLAG      128
 #define FUTEX_CLOCK_REALTIME    256
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f50cc70f1a..d2d18d29a6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7773,8 +7773,17 @@ static int do_futex(CPUState *cpu, bool time64, target_ulong uaddr,
     case FUTEX_WAIT_BITSET:
         val = tswap32(val);
         break;
+    case FUTEX_WAIT_REQUEUE_PI:
+        val = tswap32(val);
+        haddr2 = g2h(cpu, uaddr2);
+        break;
+    case FUTEX_LOCK_PI:
+    case FUTEX_LOCK_PI2:
+        break;
     case FUTEX_WAKE:
     case FUTEX_WAKE_BITSET:
+    case FUTEX_TRYLOCK_PI:
+    case FUTEX_UNLOCK_PI:
         timeout = 0;
         break;
     case FUTEX_FD:
@@ -7782,6 +7791,7 @@ static int do_futex(CPUState *cpu, bool time64, target_ulong uaddr,
         timeout = 0;
         break;
     case FUTEX_CMP_REQUEUE:
+    case FUTEX_CMP_REQUEUE_PI:
         val3 = tswap32(val3);
         /* fall through */
     case FUTEX_REQUEUE:
-- 
2.34.1

Re: [PATCH 5/8] linux-user: Implement PI futexes

[Laurent Vivier]

Le 29/08/2022 à 04:10, Richard Henderson a écrit :
> Define the missing FUTEX_* constants in syscall_defs.h
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/syscall_defs.h |  3 +++
>   linux-user/syscall.c      | 10 ++++++++++
>   2 files changed, 13 insertions(+)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[PATCH 6/8] linux-user: Update print_futex_op

[Richard Henderson]

Use a table for the names; print unknown values in hex,
since the value contains flags.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/strace.c | 65 ++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 36 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 7d882526da..8eadbed39e 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -3576,44 +3576,37 @@ print_munmap(CPUArchState *cpu_env, const struct syscallname *name,
 #endif
 
 #ifdef TARGET_NR_futex
-static void print_futex_op(abi_long tflag, int last)
+static void print_futex_op(int cmd, int last)
 {
-#define print_op(val) \
-if( cmd == val ) { \
-    qemu_log(#val); \
-    return; \
-}
+    static const char * const futex_names[] = {
+#define NAME(X)  [X] = #X
+        NAME(FUTEX_WAIT),
+        NAME(FUTEX_WAKE),
+        NAME(FUTEX_FD),
+        NAME(FUTEX_REQUEUE),
+        NAME(FUTEX_CMP_REQUEUE),
+        NAME(FUTEX_WAKE_OP),
+        NAME(FUTEX_LOCK_PI),
+        NAME(FUTEX_UNLOCK_PI),
+        NAME(FUTEX_TRYLOCK_PI),
+        NAME(FUTEX_WAIT_BITSET),
+        NAME(FUTEX_WAKE_BITSET),
+        NAME(FUTEX_WAIT_REQUEUE_PI),
+        NAME(FUTEX_CMP_REQUEUE_PI),
+        NAME(FUTEX_LOCK_PI2),
+#undef NAME
+    };
 
-    int cmd = (int)tflag;
-#ifdef FUTEX_PRIVATE_FLAG
-    if (cmd & FUTEX_PRIVATE_FLAG) {
-        qemu_log("FUTEX_PRIVATE_FLAG|");
-        cmd &= ~FUTEX_PRIVATE_FLAG;
+    unsigned base_cmd = cmd & FUTEX_CMD_MASK;
+
+    if (base_cmd < ARRAY_SIZE(futex_names)) {
+        qemu_log("%s%s%s",
+                 (cmd & FUTEX_PRIVATE_FLAG ? "FUTEX_PRIVATE_FLAG|" : ""),
+                 (cmd & FUTEX_CLOCK_REALTIME ? "FUTEX_CLOCK_REALTIME|" : ""),
+                 futex_names[base_cmd]);
+    } else {
+        qemu_log("0x%x", cmd);
     }
-#endif
-#ifdef FUTEX_CLOCK_REALTIME
-    if (cmd & FUTEX_CLOCK_REALTIME) {
-        qemu_log("FUTEX_CLOCK_REALTIME|");
-        cmd &= ~FUTEX_CLOCK_REALTIME;
-    }
-#endif
-    print_op(FUTEX_WAIT)
-    print_op(FUTEX_WAKE)
-    print_op(FUTEX_FD)
-    print_op(FUTEX_REQUEUE)
-    print_op(FUTEX_CMP_REQUEUE)
-    print_op(FUTEX_WAKE_OP)
-    print_op(FUTEX_LOCK_PI)
-    print_op(FUTEX_UNLOCK_PI)
-    print_op(FUTEX_TRYLOCK_PI)
-#ifdef FUTEX_WAIT_BITSET
-    print_op(FUTEX_WAIT_BITSET)
-#endif
-#ifdef FUTEX_WAKE_BITSET
-    print_op(FUTEX_WAKE_BITSET)
-#endif
-    /* unknown values */
-    qemu_log("%d", cmd);
 }
 
 static void
@@ -3625,7 +3618,7 @@ print_futex(CPUArchState *cpu_env, const struct syscallname *name,
     print_pointer(arg0, 0);
     print_futex_op(arg1, 0);
     print_raw_param(",%d", arg2, 0);
-    print_pointer(arg3, 0); /* struct timespec */
+    print_pointer(arg3, 0); /* struct timespec or val2 */
     print_pointer(arg4, 0);
     print_raw_param("%d", arg4, 1);
     print_syscall_epilogue(name);
-- 
2.34.1

Re: [PATCH 6/8] linux-user: Update print_futex_op

[Laurent Vivier]

Le 29/08/2022 à 04:10, Richard Henderson a écrit :
> Use a table for the names; print unknown values in hex,
> since the value contains flags.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/strace.c | 65 ++++++++++++++++++++-------------------------
>   1 file changed, 29 insertions(+), 36 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[PATCH 7/8] linux-user: Lock log around strace

[Richard Henderson]

Do not allow syscall arguments to be interleaved between threads.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/strace.c | 65 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 19 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 8eadbed39e..c47d91bb3d 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -3773,26 +3773,37 @@ print_syscall(CPUArchState *cpu_env, int num,
               abi_long arg4, abi_long arg5, abi_long arg6)
 {
     int i;
-    const char *format="%s(" TARGET_ABI_FMT_ld "," TARGET_ABI_FMT_ld "," TARGET_ABI_FMT_ld "," TARGET_ABI_FMT_ld "," TARGET_ABI_FMT_ld "," TARGET_ABI_FMT_ld ")";
+    FILE *f;
+    const char *format = "%s(" TARGET_ABI_FMT_ld "," TARGET_ABI_FMT_ld ","
+                               TARGET_ABI_FMT_ld "," TARGET_ABI_FMT_ld ","
+                               TARGET_ABI_FMT_ld "," TARGET_ABI_FMT_ld ")";
 
-    qemu_log("%d ", getpid());
+    f = qemu_log_trylock();
+    if (!f) {
+        return;
+    }
+    fprintf(f, "%d ", getpid());
 
-    for(i=0;i<nsyscalls;i++)
-        if( scnames[i].nr == num ) {
-            if( scnames[i].call != NULL ) {
-                scnames[i].call(
-                    cpu_env, &scnames[i], arg1, arg2, arg3, arg4, arg5, arg6);
+    for (i = 0; i < nsyscalls; i++) {
+        if (scnames[i].nr == num) {
+            if (scnames[i].call != NULL) {
+                scnames[i].call(cpu_env, &scnames[i], arg1, arg2, arg3,
+                                arg4, arg5, arg6);
             } else {
                 /* XXX: this format system is broken because it uses
                    host types and host pointers for strings */
-                if( scnames[i].format != NULL )
+                if (scnames[i].format != NULL) {
                     format = scnames[i].format;
-                qemu_log(format,
-                         scnames[i].name, arg1, arg2, arg3, arg4, arg5, arg6);
+                }
+                fprintf(f, format, scnames[i].name, arg1, arg2,
+                        arg3, arg4, arg5, arg6);
             }
+            qemu_log_unlock(f);
             return;
         }
-    qemu_log("Unknown syscall %d\n", num);
+    }
+    fprintf(f, "Unknown syscall %d\n", num);
+    qemu_log_unlock(f);
 }
 
 
@@ -3802,21 +3813,29 @@ print_syscall_ret(CPUArchState *cpu_env, int num, abi_long ret,
                   abi_long arg4, abi_long arg5, abi_long arg6)
 {
     int i;
+    FILE *f;
 
-    for(i=0;i<nsyscalls;i++)
-        if( scnames[i].nr == num ) {
-            if( scnames[i].result != NULL ) {
+    f = qemu_log_trylock();
+    if (!f) {
+        return;
+    }
+
+    for (i = 0; i < nsyscalls; i++) {
+        if (scnames[i].nr == num) {
+            if (scnames[i].result != NULL) {
                 scnames[i].result(cpu_env, &scnames[i], ret,
                                   arg1, arg2, arg3,
                                   arg4, arg5, arg6);
             } else {
                 if (!print_syscall_err(ret)) {
-                    qemu_log(TARGET_ABI_FMT_ld, ret);
+                    fprintf(f, TARGET_ABI_FMT_ld, ret);
                 }
-                qemu_log("\n");
+                fprintf(f, "\n");
             }
             break;
         }
+    }
+    qemu_log_unlock(f);
 }
 
 void print_taken_signal(int target_signum, const target_siginfo_t *tinfo)
@@ -3824,9 +3843,17 @@ void print_taken_signal(int target_signum, const target_siginfo_t *tinfo)
     /* Print the strace output for a signal being taken:
      * --- SIGSEGV {si_signo=SIGSEGV, si_code=SI_KERNEL, si_addr=0} ---
      */
-    qemu_log("--- ");
+    FILE *f;
+
+    f = qemu_log_trylock();
+    if (!f) {
+        return;
+    }
+
+    fprintf(f, "--- ");
     print_signal(target_signum, 1);
-    qemu_log(" ");
+    fprintf(f, " ");
     print_siginfo(tinfo);
-    qemu_log(" ---\n");
+    fprintf(f, " ---\n");
+    qemu_log_unlock(f);
 }
-- 
2.34.1

Re: [PATCH 7/8] linux-user: Lock log around strace

[Laurent Vivier]

Le 29/08/2022 à 04:10, Richard Henderson a écrit :
> Do not allow syscall arguments to be interleaved between threads.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/strace.c | 65 ++++++++++++++++++++++++++++++++-------------
>   1 file changed, 46 insertions(+), 19 deletions(-)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[PATCH 8/8] linux-user: Log tid for strace

[Richard Henderson]

Printing the same pid for all threads isn't helpful.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/strace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index c47d91bb3d..ca9d44fa7c 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -3782,7 +3782,7 @@ print_syscall(CPUArchState *cpu_env, int num,
     if (!f) {
         return;
     }
-    fprintf(f, "%d ", getpid());
+    fprintf(f, "%d ", gettid());
 
     for (i = 0; i < nsyscalls; i++) {
         if (scnames[i].nr == num) {
-- 
2.34.1

Re: [PATCH 8/8] linux-user: Log tid for strace

[Laurent Vivier]

Le 29/08/2022 à 04:10, Richard Henderson a écrit :
> Printing the same pid for all threads isn't helpful.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/strace.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index c47d91bb3d..ca9d44fa7c 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -3782,7 +3782,7 @@ print_syscall(CPUArchState *cpu_env, int num,
>       if (!f) {
>           return;
>       }
> -    fprintf(f, "%d ", getpid());
> +    fprintf(f, "%d ", gettid());
>   
>       for (i = 0; i < nsyscalls; i++) {
>           if (scnames[i].nr == num) {

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 8/8] linux-user: Log tid for strace

[Laurent Vivier]

Le 29/08/2022 à 04:10, Richard Henderson a écrit :
> Printing the same pid for all threads isn't helpful.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/strace.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index c47d91bb3d..ca9d44fa7c 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -3782,7 +3782,7 @@ print_syscall(CPUArchState *cpu_env, int num,
>       if (!f) {
>           return;
>       }
> -    fprintf(f, "%d ", getpid());
> +    fprintf(f, "%d ", gettid());
>   
>       for (i = 0; i < nsyscalls; i++) {
>           if (scnames[i].nr == num) {

I've removed this patch from the PR because of a problem reported by Stefan:

cc -m64 -mcx16 -Ilibqemu-hexagon-linux-user.fa.p -I. -I..
-Itarget/hexagon -I../target/hexagon -I../common-user/host/x86_64
-I../linux-user/include/host/x86_64 -I../linux-user/include
-Ilinux-user -I../linux-user -I../linux-user/hexagon -Iqapi -Itrace
-Iui/shader -I/usr/include/capstone -I/usr/include/glib-2.0
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto
-Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem
/builds/qemu-project/qemu/linux-headers -isystem linux-headers -iquote
. -iquote /builds/qemu-project/qemu -iquote
/builds/qemu-project/qemu/include -iquote
/builds/qemu-project/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE
-D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef
-Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common
-fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined
-Wimplicit-fallthrough=2 -Wno-missing-include-dirs
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE
-isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H
'-DCONFIG_TARGET="hexagon-linux-user-config-target.h"'
'-DCONFIG_DEVICES="hexagon-linux-user-config-devices.h"' -MD -MQ
libqemu-hexagon-linux-user.fa.p/linux-user_strace.c.o -MF
libqemu-hexagon-linux-user.fa.p/linux-user_strace.c.o.d -o
libqemu-hexagon-linux-user.fa.p/linux-user_strace.c.o -c
../linux-user/strace.c
../linux-user/strace.c: In function 'print_syscall':
../linux-user/strace.c:3931:23: error: implicit declaration of
function 'gettid'; did you mean 'getgid'?
[-Werror=implicit-function-declaration]
fprintf(f, "%d ", gettid());
^~~~~~
getgid
../linux-user/strace.c:3931:23: error: nested extern declaration of
'gettid' [-Werror=nested-externs]

https://gitlab.com/qemu-project/qemu/-/jobs/3100018258

Thanks,
Laurent

Re: [PATCH 0/8] linux-user: Futex improvements

[Laurent Vivier]

Le 29/08/2022 à 04:09, Richard Henderson a écrit :
> Fixes a bug in FUTEX_FD, and implements a bunch of other ops.
> Also, some assorted strace fixes.
> 
> r~
> 
> Richard Henderson (8):
>    linux-user: Combine do_futex and do_futex_time64
>    linux-user: Sink call to do_safe_futex
>    linux-user: Implement FUTEX_WAKE_BITSET
>    linux-user: Convert signal number for FUTEX_FD
>    linux-user: Implement PI futexes
>    linux-user: Update print_futex_op
>    linux-user: Lock log around strace
>    linux-user: Log tid for strace
> 
>   linux-user/syscall_defs.h |   3 +
>   linux-user/strace.c       | 130 ++++++++++++++++++++++----------------
>   linux-user/syscall.c      | 125 ++++++++++++++----------------------
>   3 files changed, 125 insertions(+), 133 deletions(-)
> 

Series applied to my linux-user-for-7.2 branch.

Thanks,
Laurent