[PATCH] linux-user/strace: fix printing of file offsets

Jean-Christian CÎRSTEA posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251225183644.1919184-1-jean.christian.cirstea@gmail.com
Maintainers: Laurent Vivier <laurent@vivier.eu>
There is a newer version of this series
linux-user/strace.c | 38 ++++++++++++++++++++++++++++----------
1 file changed, 28 insertions(+), 10 deletions(-)
[PATCH] linux-user/strace: fix printing of file offsets
Posted by Jean-Christian CÎRSTEA 1 month, 2 weeks ago
Previously, 64-bit file offsets (loff_t) were printed using `print_raw_param()`
function, which led to silent truncation of the upper part. This commit fixes
this issue by adding two helper functions:

1. print_file_offset32(): prints 32-bit file offsets (off_t)
2. print_file_offset64(): prints 64-bit file offsets (loff_t)

*NOTE*: checkpatch.pl gives the following errors:

```
ERROR: externs should be avoided in .c files
#30: FILE: linux-user/strace.c:88:
+UNUSED void print_file_offset32(abi_long offset, int);

ERROR: storage class should be at the beginning of the declaration
#31: FILE: linux-user/strace.c:89:
+UNUSED static void print_file_offset64(abi_long low, abi_long high, int);
```

The errors are may be removed if `UNUSED` and `static` are switched. Should this
patch fix this and swap all `UNUSED`s and `static`s?

Signed-off-by: Jean-Christian CÎRSTEA <jean.christian.cirstea@gmail.com>
---
 linux-user/strace.c | 38 ++++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 758c5d32b6..f790cab4da 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -85,6 +85,8 @@ UNUSED static void print_enums(const struct enums *, abi_long, int);
 UNUSED static void print_at_dirfd(abi_long, int);
 UNUSED static void print_file_mode(abi_long, int);
 UNUSED static void print_open_flags(abi_long, int);
+UNUSED void print_file_offset32(abi_long offset, int);
+UNUSED static void print_file_offset64(abi_long low, abi_long high, int);
 UNUSED static void print_syscall_prologue(const struct syscallname *);
 UNUSED static void print_syscall_epilogue(const struct syscallname *);
 UNUSED static void print_string(abi_long, int);
@@ -1664,6 +1666,20 @@ print_open_flags(abi_long flags, int last)
     print_flags(open_flags, flags, last);
 }
 
+/* Prints 32-bit file offset (off_t) */
+static void
+print_file_offset32(abi_long offset, int last)
+{
+    print_raw_param(TARGET_ABI_FMT_ld, offset, 0);
+}
+
+/* Prints 64-bit file offset (loff_t) */
+static void
+print_file_offset64(abi_long low, abi_long high, int last)
+{
+    print_raw_param64("%" PRIu64, target_offset64(low, high), last);
+}
+
 static void
 print_syscall_prologue(const struct syscallname *sc)
 {
@@ -2187,11 +2203,13 @@ print_fallocate(CPUArchState *cpu_env, const struct syscallname *name,
     print_raw_param("%d", arg0, 0);
     print_flags(falloc_flags, arg1, 0);
 #if TARGET_ABI_BITS == 32
-    print_raw_param("%" PRIu64, target_offset64(arg2, arg3), 0);
-    print_raw_param("%" PRIu64, target_offset64(arg4, arg5), 1);
+    /* On 32-bit targets, two registers are used for `loff_t` */
+    print_file_offset64(arg2, arg3, 0);
+    print_file_offset64(arg4, arg5, 1);
 #else
-    print_raw_param(TARGET_ABI_FMT_ld, arg2, 0);
-    print_raw_param(TARGET_ABI_FMT_ld, arg3, 1);
+    /* On 64-bit targets, one register is used for `loff_t` */
+    print_file_offset64(arg2, 0, 0);
+    print_file_offset64(arg3, 0, 1);
 #endif
     print_syscall_epilogue(name);
 }
@@ -2619,7 +2637,7 @@ print_lseek(CPUArchState *cpu_env, const struct syscallname *name,
 {
     print_syscall_prologue(name);
     print_raw_param("%d", arg0, 0);
-    print_raw_param(TARGET_ABI_FMT_ld, arg1, 0);
+    print_file_offset32(arg1, 0);
     switch (arg2) {
     case SEEK_SET:
         qemu_log("SEEK_SET"); break;
@@ -2650,7 +2668,7 @@ print_truncate(CPUArchState *cpu_env, const struct syscallname *name,
 {
     print_syscall_prologue(name);
     print_string(arg0, 0);
-    print_raw_param(TARGET_ABI_FMT_ld, arg1, 1);
+    print_file_offset32(arg1, 1);
     print_syscall_epilogue(name);
 }
 #endif
@@ -2667,7 +2685,7 @@ print_truncate64(CPUArchState *cpu_env, const struct syscallname *name,
         arg1 = arg2;
         arg2 = arg3;
     }
-    print_raw_param("%" PRIu64, target_offset64(arg1, arg2), 1);
+    print_file_offset64(arg1, arg2, 1);
     print_syscall_epilogue(name);
 }
 #endif
@@ -2684,7 +2702,7 @@ print_ftruncate64(CPUArchState *cpu_env, const struct syscallname *name,
         arg1 = arg2;
         arg2 = arg3;
     }
-    print_raw_param("%" PRIu64, target_offset64(arg1, arg2), 1);
+    print_file_offset64(arg1, arg2, 1);
     print_syscall_epilogue(name);
 }
 #endif
@@ -3239,7 +3257,7 @@ print_stat(CPUArchState *cpu_env, const struct syscallname *name,
     print_syscall_epilogue(name);
 }
 #define print_lstat     print_stat
-#define print_stat64	print_stat
+#define print_stat64    print_stat
 #define print_lstat64   print_stat
 #endif
 
@@ -4228,7 +4246,7 @@ print_pread64(CPUArchState *cpu_env, const struct syscallname *name,
     print_raw_param("%d", arg0, 0);
     print_pointer(arg1, 0);
     print_raw_param("%d", arg2, 0);
-    print_raw_param("%" PRIu64, target_offset64(arg3, arg4), 1);
+    print_file_offset64(arg3, arg4, 1);
     print_syscall_epilogue(name);
 }
 #endif
-- 
2.51.0


Re: [PATCH] linux-user/strace: fix printing of file offsets
Posted by Helge Deller 1 month ago
On 12/25/25 19:36, Jean-Christian CÎRSTEA wrote:
> Previously, 64-bit file offsets (loff_t) were printed using `print_raw_param()`
> function, which led to silent truncation of the upper part. This commit fixes
> this issue by adding two helper functions:
> 
> 1. print_file_offset32(): prints 32-bit file offsets (off_t)
> 2. print_file_offset64(): prints 64-bit file offsets (loff_t)
> 
> *NOTE*: checkpatch.pl gives the following errors:
> 
> ```
> ERROR: externs should be avoided in .c files
> #30: FILE: linux-user/strace.c:88:
> +UNUSED void print_file_offset32(abi_long offset, int);
> 
> ERROR: storage class should be at the beginning of the declaration
> #31: FILE: linux-user/strace.c:89:
> +UNUSED static void print_file_offset64(abi_long low, abi_long high, int);
> ```
> 
> The errors are may be removed if `UNUSED` and `static` are switched. Should this
> patch fix this and swap all `UNUSED`s and `static`s?

I'd ignore the checkpatch warning for your patch.
If a switch is done (and wanted), please send it later in a seperate patch.
  
> Signed-off-by: Jean-Christian CÎRSTEA <jean.christian.cirstea@gmail.com>
> ---
>   linux-user/strace.c | 38 ++++++++++++++++++++++++++++----------
>   1 file changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 758c5d32b6..f790cab4da 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -85,6 +85,8 @@ UNUSED static void print_enums(const struct enums *, abi_long, int);
>   UNUSED static void print_at_dirfd(abi_long, int);
>   UNUSED static void print_file_mode(abi_long, int);
>   UNUSED static void print_open_flags(abi_long, int);
> +UNUSED void print_file_offset32(abi_long offset, int);

^ shouldn't it be static too?

> +UNUSED static void print_file_offset64(abi_long low, abi_long high, int);

I think it's wrong to have "low" and "high" here.
This very much depends if this is on big-endian or little-endian machines.
See definition of target_offset64() in linux-user/user-internals.h.
There it's called word0 and word1, which is probably better.

Overall your patch seems ok.
Did you test it on little- and big-endian machines?

Helge
Re: [PATCH] linux-user/strace: fix printing of file offsets
Posted by Jean-Christian CÎRSTEA 1 month ago
Hello,

It looks like you have reviewed the old version of the patch. The new version is
available at this
[address](https://lists.gnu.org/archive/html/qemu-devel/2025-12/msg03197.html).

> Did you test it on little- and big-endian machines?

Just on little-endian. I don't have any physical big-endian machine.
Re: [PATCH] linux-user/strace: fix printing of file offsets
Posted by Laurent Vivier 1 month, 2 weeks ago
Le 25/12/2025 à 19:36, Jean-Christian CÎRSTEA a écrit :
> Previously, 64-bit file offsets (loff_t) were printed using `print_raw_param()`
> function, which led to silent truncation of the upper part. This commit fixes
> this issue by adding two helper functions:
> 
> 1. print_file_offset32(): prints 32-bit file offsets (off_t)
> 2. print_file_offset64(): prints 64-bit file offsets (loff_t)
> 
> *NOTE*: checkpatch.pl gives the following errors:
> 
> ```
> ERROR: externs should be avoided in .c files
> #30: FILE: linux-user/strace.c:88:
> +UNUSED void print_file_offset32(abi_long offset, int);

I think there is an actual error here: the static is missing.

> 
> ERROR: storage class should be at the beginning of the declaration
> #31: FILE: linux-user/strace.c:89:
> +UNUSED static void print_file_offset64(abi_long low, abi_long high, int);
> ```
> 
> The errors are may be removed if `UNUSED` and `static` are switched. Should this
> patch fix this and swap all `UNUSED`s and `static`s?

The missing `static` on print_file_offset32 is a real error . The other checkpatch warning about 
`UNUSED static` matches existing style here, so I’d leave that alone.

> 
> Signed-off-by: Jean-Christian CÎRSTEA <jean.christian.cirstea@gmail.com>
> ---
>   linux-user/strace.c | 38 ++++++++++++++++++++++++++++----------
>   1 file changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 758c5d32b6..f790cab4da 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -85,6 +85,8 @@ UNUSED static void print_enums(const struct enums *, abi_long, int);
>   UNUSED static void print_at_dirfd(abi_long, int);
>   UNUSED static void print_file_mode(abi_long, int);
>   UNUSED static void print_open_flags(abi_long, int);
> +UNUSED void print_file_offset32(abi_long offset, int);

Missing static

> +UNUSED static void print_file_offset64(abi_long low, abi_long high, int);
>   UNUSED static void print_syscall_prologue(const struct syscallname *);
>   UNUSED static void print_syscall_epilogue(const struct syscallname *);
>   UNUSED static void print_string(abi_long, int);
> @@ -1664,6 +1666,20 @@ print_open_flags(abi_long flags, int last)
>       print_flags(open_flags, flags, last);
>   }
>   
> +/* Prints 32-bit file offset (off_t) */
> +static void
> +print_file_offset32(abi_long offset, int last)
> +{
> +    print_raw_param(TARGET_ABI_FMT_ld, offset, 0);

You should pass "last" instead of 0.

> +}
> +
> +/* Prints 64-bit file offset (loff_t) */
> +static void
> +print_file_offset64(abi_long low, abi_long high, int last)

You shouldn't call them "low", "high", because it depends on the endianness of the architecture.
Use word0, word1 as it is in target_offset64() definition.

> +{
> +    print_raw_param64("%" PRIu64, target_offset64(low, high), last);

You should use PRId64 as loff_t is supposed to be signed (PRIu64 is an existing bug)

> +}
> +
>   static void
>   print_syscall_prologue(const struct syscallname *sc)
>   {
> @@ -2187,11 +2203,13 @@ print_fallocate(CPUArchState *cpu_env, const struct syscallname *name,
>       print_raw_param("%d", arg0, 0);
>       print_flags(falloc_flags, arg1, 0);
>   #if TARGET_ABI_BITS == 32
> -    print_raw_param("%" PRIu64, target_offset64(arg2, arg3), 0);
> -    print_raw_param("%" PRIu64, target_offset64(arg4, arg5), 1);
> +    /* On 32-bit targets, two registers are used for `loff_t` */
> +    print_file_offset64(arg2, arg3, 0);
> +    print_file_offset64(arg4, arg5, 1);
>   #else
> -    print_raw_param(TARGET_ABI_FMT_ld, arg2, 0);
> -    print_raw_param(TARGET_ABI_FMT_ld, arg3, 1);
> +    /* On 64-bit targets, one register is used for `loff_t` */
> +    print_file_offset64(arg2, 0, 0);
> +    print_file_offset64(arg3, 0, 1);
>   #endif
>       print_syscall_epilogue(name);
>   }
> @@ -2619,7 +2637,7 @@ print_lseek(CPUArchState *cpu_env, const struct syscallname *name,
>   {
>       print_syscall_prologue(name);
>       print_raw_param("%d", arg0, 0);
> -    print_raw_param(TARGET_ABI_FMT_ld, arg1, 0);
> +    print_file_offset32(arg1, 0);
>       switch (arg2) {
>       case SEEK_SET:
>           qemu_log("SEEK_SET"); break;


I think print__llseek() should be updated/fixed too.

> @@ -2650,7 +2668,7 @@ print_truncate(CPUArchState *cpu_env, const struct syscallname *name,
>   {
>       print_syscall_prologue(name);
>       print_string(arg0, 0);
> -    print_raw_param(TARGET_ABI_FMT_ld, arg1, 1);
> +    print_file_offset32(arg1, 1);
>       print_syscall_epilogue(name);
>   }
>   #endif
> @@ -2667,7 +2685,7 @@ print_truncate64(CPUArchState *cpu_env, const struct syscallname *name,
>           arg1 = arg2;
>           arg2 = arg3;
>       }
> -    print_raw_param("%" PRIu64, target_offset64(arg1, arg2), 1);
> +    print_file_offset64(arg1, arg2, 1);
>       print_syscall_epilogue(name);
>   }
>   #endif
> @@ -2684,7 +2702,7 @@ print_ftruncate64(CPUArchState *cpu_env, const struct syscallname *name,
>           arg1 = arg2;
>           arg2 = arg3;
>       }
> -    print_raw_param("%" PRIu64, target_offset64(arg1, arg2), 1);
> +    print_file_offset64(arg1, arg2, 1);
>       print_syscall_epilogue(name);
>   }
>   #endif
> @@ -3239,7 +3257,7 @@ print_stat(CPUArchState *cpu_env, const struct syscallname *name,
>       print_syscall_epilogue(name);
>   }
>   #define print_lstat     print_stat
> -#define print_stat64	print_stat
> +#define print_stat64    print_stat
>   #define print_lstat64   print_stat
>   #endif
>   
> @@ -4228,7 +4246,7 @@ print_pread64(CPUArchState *cpu_env, const struct syscallname *name,
>       print_raw_param("%d", arg0, 0);
>       print_pointer(arg1, 0);
>       print_raw_param("%d", arg2, 0);
> -    print_raw_param("%" PRIu64, target_offset64(arg3, arg4), 1);
> +    print_file_offset64(arg3, arg4, 1);
>       print_syscall_epilogue(name);
>   }
>   #endif

Thanks,
Laurent