All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls
@ 2020-06-08 16:43 Filip Bozuta
  2020-06-08 16:43 ` [PATCH v2 1/6] linux-user: Extend strace support to enable argument printing after syscall execution Filip Bozuta
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Filip Bozuta @ 2020-06-08 16:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

This series covers strace support for printing arguments of following syscalls:

    *acct()           *lgetxattr()       *removexattr()       *lchown()
    *fsync()          *fgetxattr()       *lremovexattr()      *fallocate()
    *fdatasync()      *listxattr()       *fremovexattr()
    *listen()         *llistxattr()      *lseek()
    *getxattr()       *flistxattr()      *chown()

The implementation details for strace support is described in this series patch
commit messages.

Testing method:

    Mini test programs were written that run these syscalls for different arguments.
    Those programs were compiled (sometimes using cross-compilers) for the following
    architectures:

        * Intel 64-bit (little endian) (gcc)
        * Power pc 32-bit (big endian) (powerpc-linux-gnu-gcc)
        * Power pc 64-bit (big endian) (powerpc64-linux-gnu-gcc)
        * Mips 32-bit (little endian) (mipsel-linux-gnu-gcc)
        * Mips 64-bit (little endian) (mips64el-linux-gnuabi64-gcc)

    The corresponding native programs were executed with strace, without using
    QEMU, on Intel Core i7-4790K (x86_64) host.

    All applicable compiled programs were in turn executed with "-strace"
    through QEMU and the strace printing results obtained were the same 
    ones gotten for native execution.

v2:

    * Added patch that extends strace support by enabling argument printing
      after syscall execution
    * Added strace support for argument printing for syscalls:
      removexattr(), lremovexattr(), fremovexattr()
    * Added "print_syscall_ret_listxattr()" that prints list of extended
      attributes after execution of syscalls: listxattr(), llistxattr(),
      flistxattr()
    * Corrected formats in some printing functions
    * Moved target_offset64() function definition from "syscall.c" to
      "qemu.h"

Filip Bozuta (6):
  linux-user: Extend strace support to enable argument printing after
    syscall execution
  linux-user: Add strace support for a group of syscalls
  linux-user: Add strace support for printing argument of syscalls used
    for extended attributes
  linux-user: Add strace support for printing arguments of lseek()
  linux-user: Add strace support for printing arguments of
    chown()/lchown()
  linux-user: Add strace support for printing arguments of fallocate()

 linux-user/qemu.h      |  20 +++-
 linux-user/strace.c    | 247 ++++++++++++++++++++++++++++++++++++++++-
 linux-user/strace.list |  37 +++---
 linux-user/syscall.c   |  18 +--
 4 files changed, 281 insertions(+), 41 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/6] linux-user: Extend strace support to enable argument printing after syscall execution
  2020-06-08 16:43 [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls Filip Bozuta
@ 2020-06-08 16:43 ` Filip Bozuta
  2020-06-10 17:33   ` Laurent Vivier
  2020-06-08 16:43 ` [PATCH v2 2/6] linux-user: Add strace support for a group of syscalls Filip Bozuta
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Filip Bozuta @ 2020-06-08 16:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

    Structure "struct syscallname" in file "strace.c" is used for "-strace"
    to print arguments and return values of syscalls. The last field of
    this structure "result" represents the calling function that prints the
    return values. This field was extended in this patch so that this functions
    takes all syscalls arguments beside the return value. In this way, it enables
    "-strace" to print arguments of syscalls that have changed after the syscall
    execution. This extension will be useful as there are many syscalls that
    return values inside their arguments (i.e. listxattr() that returns the list
    of extended attributes inside the "list" argument).

Implementation notes:

    Since there are already three existing "print_syscall_ret*" functions inside
    "strace.c" ("print_syscall_ret_addr()", "print_syscall_ret_adjtimex()",
    "print_syscall_ret_newselect()"), they were changed to have all syscall arguments
    beside the return value. This was done so that these functions don't cause build
    errors (even though syscall arguments are not used in these functions).

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
---
 linux-user/qemu.h    |  4 +++-
 linux-user/strace.c  | 24 ++++++++++++++++++------
 linux-user/syscall.c |  2 +-
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index ce902f5132..8f938b8105 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -383,7 +383,9 @@ int host_to_target_waitstatus(int status);
 void print_syscall(int num,
                    abi_long arg1, abi_long arg2, abi_long arg3,
                    abi_long arg4, abi_long arg5, abi_long arg6);
-void print_syscall_ret(int num, abi_long arg1);
+void print_syscall_ret(int num, abi_long ret,
+                       abi_long arg1, abi_long arg2, abi_long arg3,
+                       abi_long arg4, abi_long arg5, abi_long arg6);
 /**
  * print_taken_signal:
  * @target_signum: target signal being taken
diff --git a/linux-user/strace.c b/linux-user/strace.c
index 0d9095c674..23f7c386b5 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -19,7 +19,9 @@ struct syscallname {
     void (*call)(const struct syscallname *,
                  abi_long, abi_long, abi_long,
                  abi_long, abi_long, abi_long);
-    void (*result)(const struct syscallname *, abi_long);
+    void (*result)(const struct syscallname *, abi_long,
+                   abi_long, abi_long, abi_long,
+                   abi_long, abi_long, abi_long);
 };
 
 #ifdef __GNUC__
@@ -736,7 +738,9 @@ print_ipc(const struct syscallname *name,
  */
 
 static void
-print_syscall_ret_addr(const struct syscallname *name, abi_long ret)
+print_syscall_ret_addr(const struct syscallname *name, abi_long ret,
+                       abi_long arg0, abi_long arg1, abi_long arg2,
+                       abi_long arg3, abi_long arg4, abi_long arg5)
 {
     const char *errstr = NULL;
 
@@ -760,7 +764,9 @@ print_syscall_ret_raw(struct syscallname *name, abi_long ret)
 
 #ifdef TARGET_NR__newselect
 static void
-print_syscall_ret_newselect(const struct syscallname *name, abi_long ret)
+print_syscall_ret_newselect(const struct syscallname *name, abi_long ret,
+                            abi_long arg0, abi_long arg1, abi_long arg2,
+                            abi_long arg3, abi_long arg4, abi_long arg5)
 {
     qemu_log(" = 0x" TARGET_ABI_FMT_lx " (", ret);
     print_fdset(newselect_arg1,newselect_arg2);
@@ -783,7 +789,9 @@ print_syscall_ret_newselect(const struct syscallname *name, abi_long ret)
 #define TARGET_TIME_ERROR    5   /* clock not synchronized */
 #ifdef TARGET_NR_adjtimex
 static void
-print_syscall_ret_adjtimex(const struct syscallname *name, abi_long ret)
+print_syscall_ret_adjtimex(const struct syscallname *name, abi_long ret,
+                           abi_long arg0, abi_long arg1, abi_long arg2,
+                           abi_long arg3, abi_long arg4, abi_long arg5)
 {
     const char *errstr = NULL;
 
@@ -2847,7 +2855,9 @@ print_syscall(int num,
 
 
 void
-print_syscall_ret(int num, abi_long ret)
+print_syscall_ret(int num, abi_long ret,
+                  abi_long arg1, abi_long arg2, abi_long arg3,
+                  abi_long arg4, abi_long arg5, abi_long arg6)
 {
     int i;
     const char *errstr = NULL;
@@ -2855,7 +2865,9 @@ print_syscall_ret(int num, abi_long ret)
     for(i=0;i<nsyscalls;i++)
         if( scnames[i].nr == num ) {
             if( scnames[i].result != NULL ) {
-                scnames[i].result(&scnames[i], ret);
+                scnames[i].result(&scnames[i], ret,
+                                  arg1, arg2, arg3,
+                                  arg4, arg5, arg6);
             } else {
                 if (ret < 0) {
                     errstr = target_strerror(-ret);
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 05f03919ff..009bb67422 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -12441,7 +12441,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                       arg5, arg6, arg7, arg8);
 
     if (unlikely(qemu_loglevel_mask(LOG_STRACE))) {
-        print_syscall_ret(num, ret);
+        print_syscall_ret(num, ret, arg1, arg2, arg3, arg4, arg5, arg6);
     }
 
     record_syscall_return(cpu, num, ret);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/6] linux-user: Add strace support for a group of syscalls
  2020-06-08 16:43 [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls Filip Bozuta
  2020-06-08 16:43 ` [PATCH v2 1/6] linux-user: Extend strace support to enable argument printing after syscall execution Filip Bozuta
@ 2020-06-08 16:43 ` Filip Bozuta
  2020-06-08 16:43 ` [PATCH v2 3/6] linux-user: Add strace support for printing argument of syscalls used for extended attributes Filip Bozuta
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Filip Bozuta @ 2020-06-08 16:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

This patch implements strace argument printing functionality for following syscalls:

    *acct - switch process accounting on or off

        int acct(const char *filename)
        man page: https://www.man7.org/linux/man-pages/man2/acct.2.html

    *fsync, fdatasync - synchronize a file's in-core state with storage device

        int fsync(int fd)
        int fdatasync(int fd)
        man page: https://www.man7.org/linux/man-pages/man2/fsync.2.html

    *listen - listen for connections on a socket

        int listen(int sockfd, int backlog)
        man page: https://www.man7.org/linux/man-pages/man2/listen.2.html

Implementation notes:

    Syscall acct() takes string as its only argument and thus a separate
    print function "print_acct" is stated in file "strace.list". This
    function is defined and implemented in "strace.c" by using an
    existing function used to print string arguments: "print_string()".
    All the other syscalls have only primitive argument types, so the
    rest of the implementation was handled by stating an appropriate
    printing format in file "strace.list".

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/strace.c    | 13 ++++++++++++-
 linux-user/strace.list |  8 ++++----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 23f7c386b5..f980451e3f 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1361,6 +1361,18 @@ print_access(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_acct
+static void
+print_acct(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_string(arg0, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #ifdef TARGET_NR_brk
 static void
 print_brk(const struct syscallname *name,
@@ -1625,7 +1637,6 @@ print_fcntl(const struct syscallname *name,
 #define print_fcntl64   print_fcntl
 #endif
 
-
 #ifdef TARGET_NR_futimesat
 static void
 print_futimesat(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index d49a1e92a8..fb9799e7e6 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -13,7 +13,7 @@
 { TARGET_NR_access, "access" , NULL, print_access, NULL },
 #endif
 #ifdef TARGET_NR_acct
-{ TARGET_NR_acct, "acct" , NULL, NULL, NULL },
+{ TARGET_NR_acct, "acct" , NULL, print_acct, NULL },
 #endif
 #ifdef TARGET_NR_add_key
 { TARGET_NR_add_key, "add_key" , NULL, NULL, NULL },
@@ -215,7 +215,7 @@
 { TARGET_NR_fcntl64, "fcntl64" , NULL, print_fcntl64, NULL },
 #endif
 #ifdef TARGET_NR_fdatasync
-{ TARGET_NR_fdatasync, "fdatasync" , NULL, NULL, NULL },
+{ TARGET_NR_fdatasync, "fdatasync" , "%s(%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_fgetxattr
 { TARGET_NR_fgetxattr, "fgetxattr" , NULL, NULL, NULL },
@@ -251,7 +251,7 @@
 { TARGET_NR_fstatfs64, "fstatfs64" , "%s(%d,%p)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_fsync
-{ TARGET_NR_fsync, "fsync" , NULL, NULL, NULL },
+{ TARGET_NR_fsync, "fsync" , "%s(%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_ftime
 { TARGET_NR_ftime, "ftime" , NULL, NULL, NULL },
@@ -492,7 +492,7 @@
 { TARGET_NR_Linux, "Linux" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_listen
-{ TARGET_NR_listen, "listen" , NULL, NULL, NULL },
+{ TARGET_NR_listen, "listen" , "%s(%d,%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_listxattr
 { TARGET_NR_listxattr, "listxattr" , NULL, NULL, NULL },
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/6] linux-user: Add strace support for printing argument of syscalls used for extended attributes
  2020-06-08 16:43 [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls Filip Bozuta
  2020-06-08 16:43 ` [PATCH v2 1/6] linux-user: Extend strace support to enable argument printing after syscall execution Filip Bozuta
  2020-06-08 16:43 ` [PATCH v2 2/6] linux-user: Add strace support for a group of syscalls Filip Bozuta
@ 2020-06-08 16:43 ` Filip Bozuta
  2020-06-10 19:06   ` Laurent Vivier
  2020-06-08 16:43 ` [PATCH v2 4/6] linux-user: Add strace support for printing arguments of lseek() Filip Bozuta
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Filip Bozuta @ 2020-06-08 16:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

This patch implements strace argument printing functionality for following syscalls:

    *getxattr, lgetxattr, fgetxattr - retrieve an extended attribute value

        ssize_t getxattr(const char *path, const char *name, void *value, size_t size)
        ssize_t lgetxattr(const char *path, const char *name, void *value, size_t size)
        ssize_t fgetxattr(int fd, const char *name, void *value, size_t size)
        man page: https://www.man7.org/linux/man-pages/man2/getxattr.2.html

    *listxattr, llistxattr, flistxattr - list extended attribute names

        ssize_t listxattr(const char *path, char *list, size_t size)
        ssize_t llistxattr(const char *path, char *list, size_t size)
        ssize_t flistxattr(int fd, char *list, size_t size)
        man page: https://www.man7.org/linux/man-pages/man2/listxattr.2.html

    *removexattr, lremovexattr, fremovexattr - remove an extended attribute

         int removexattr(const char *path, const char *name)
         int lremovexattr(const char *path, const char *name)
         int fremovexattr(int fd, const char *name)
         man page: https://www.man7.org/linux/man-pages/man2/removexattr.2.html

Implementation notes:

    All of the syscalls have strings as argument types and thus a separate
    printing function was stated in file "strace.list" for every one of them.
    All of these printing functions were defined in "strace.c" using existing
    printing functions for appropriate argument types:
       "print_string()" - for (const char*) type
       "print_pointer()" - for (char*) and (void *) type
       "print_raw_param()" for (int) and (size_t) type
    Syscalls "getxattr()" and "lgetxattr()" have the same number and type of
    arguments and thus their print functions ("print_getxattr", "print_lgetxattr")
    share a same definition. The same statement applies to syscalls "listxattr()"
    and "llistxattr()".
    Function "print_syscall_ret_listxattr()" was added to print the returned list
    of extended attributes for syscalls and was listed as a "result" function in file
    "strace.list" for syscalls: "listxattr(), llistxattr(), flistxattr()".

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
---
 linux-user/strace.c    | 126 +++++++++++++++++++++++++++++++++++++++++
 linux-user/strace.list |  21 ++++---
 2 files changed, 138 insertions(+), 9 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index f980451e3f..59fdb0a05f 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -830,6 +830,45 @@ print_syscall_ret_adjtimex(const struct syscallname *name, abi_long ret,
 }
 #endif
 
+#if defined(TARGET_NR_listxattr) || defined(TARGET_NR_llistxattr) \
+ || defined(TARGGET_NR_flistxattr)
+static void
+print_syscall_ret_listxattr(const struct syscallname *name, abi_long ret,
+                            abi_long arg0, abi_long arg1, abi_long arg2,
+                            abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    const char *errstr = NULL;
+
+    qemu_log(" = ");
+    if (ret < 0) {
+        qemu_log("-1 errno=%d", errno);
+        errstr = target_strerror(-ret);
+        if (errstr) {
+            qemu_log(" (%s)", errstr);
+        }
+    } else {
+        qemu_log(TARGET_ABI_FMT_ld, ret);
+        qemu_log(" (list = ");
+        if (arg1 != 0) {
+            abi_long attr = arg1;
+            for (;;) {
+                print_string(attr, 1);
+                attr += target_strlen(attr) + 1;
+                if (target_strlen(attr) == 0) {
+                    break;
+                }
+                qemu_log(",");
+            }
+        } else {
+            qemu_log("NULL");
+        }
+        qemu_log(")");
+    }
+
+    qemu_log("\n");
+}
+#endif
+
 UNUSED static struct flags access_flags[] = {
     FLAG_GENERIC(F_OK),
     FLAG_GENERIC(R_OK),
@@ -1637,6 +1676,93 @@ print_fcntl(const struct syscallname *name,
 #define print_fcntl64   print_fcntl
 #endif
 
+#ifdef TARGET_NR_fgetxattr
+static void
+print_fgetxattr(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_raw_param("%d", arg0, 0);
+    print_string(arg1, 0);
+    print_pointer(arg2, 0);
+    print_raw_param(TARGET_FMT_lu, arg3, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
+#ifdef TARGET_NR_flistxattr
+static void
+print_flistxattr(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_raw_param("%d", arg0, 0);
+    print_pointer(arg1, 0);
+    print_raw_param(TARGET_FMT_lu, arg2, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
+#if defined(TARGET_NR_getxattr) || defined(TARGET_NR_lgetxattr)
+static void
+print_getxattr(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_string(arg0, 0);
+    print_string(arg1, 0);
+    print_pointer(arg2, 0);
+    print_raw_param(TARGET_FMT_lu, arg3, 1);
+    print_syscall_epilogue(name);
+}
+#define print_lgetxattr     print_getxattr
+#endif
+
+#if defined(TARGET_NR_listxattr) || defined(TARGET_NR_llistxattr)
+static void
+print_listxattr(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_string(arg0, 0);
+    print_pointer(arg1, 0);
+    print_raw_param(TARGET_FMT_lu, arg2, 1);
+    print_syscall_epilogue(name);
+}
+#define print_llistxattr     print_listxattr
+#endif
+
+#if defined(TARGET_NR_fremovexattr)
+static void
+print_fremovexattr(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_raw_param("%d", arg0, 0);
+    print_string(arg1, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
+#if defined(TARGET_NR_removexattr) || defined(TARGET_NR_lremovexattr)
+static void
+print_removexattr(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_string(arg0, 0);
+    print_string(arg1, 1);
+    print_syscall_epilogue(name);
+}
+#define print_lremovexattr     print_removexattr
+#endif
+
 #ifdef TARGET_NR_futimesat
 static void
 print_futimesat(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index fb9799e7e6..05a72370c1 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -218,13 +218,14 @@
 { TARGET_NR_fdatasync, "fdatasync" , "%s(%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_fgetxattr
-{ TARGET_NR_fgetxattr, "fgetxattr" , NULL, NULL, NULL },
+{ TARGET_NR_fgetxattr, "fgetxattr" , NULL, print_fgetxattr, NULL },
 #endif
 #ifdef TARGET_NR_finit_module
 { TARGET_NR_finit_module, "finit_module" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_flistxattr
-{ TARGET_NR_flistxattr, "flistxattr" , NULL, NULL, NULL },
+{ TARGET_NR_flistxattr, "flistxattr" , NULL, print_flistxattr,
+                        print_syscall_ret_listxattr},
 #endif
 #ifdef TARGET_NR_flock
 { TARGET_NR_flock, "flock" , NULL, NULL, NULL },
@@ -233,7 +234,7 @@
 { TARGET_NR_fork, "fork" , "%s()", NULL, NULL },
 #endif
 #ifdef TARGET_NR_fremovexattr
-{ TARGET_NR_fremovexattr, "fremovexattr" , NULL, NULL, NULL },
+{ TARGET_NR_fremovexattr, "fremovexattr" , NULL, print_fremovexattr, NULL },
 #endif
 #ifdef TARGET_NR_fsetxattr
 { TARGET_NR_fsetxattr, "fsetxattr" , NULL, NULL, NULL },
@@ -396,7 +397,7 @@
 { TARGET_NR_getuid32, "getuid32" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_getxattr
-{ TARGET_NR_getxattr, "getxattr" , NULL, NULL, NULL },
+{ TARGET_NR_getxattr, "getxattr" , NULL, print_getxattr, NULL },
 #endif
 #ifdef TARGET_NR_getxgid
 { TARGET_NR_getxgid, "getxgid" , NULL, NULL, NULL },
@@ -480,7 +481,7 @@
 { TARGET_NR_lchown32, "lchown32" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_lgetxattr
-{ TARGET_NR_lgetxattr, "lgetxattr" , NULL, NULL, NULL },
+{ TARGET_NR_lgetxattr, "lgetxattr" , NULL, print_lgetxattr, NULL },
 #endif
 #ifdef TARGET_NR_link
 { TARGET_NR_link, "link" , NULL, print_link, NULL },
@@ -495,10 +496,12 @@
 { TARGET_NR_listen, "listen" , "%s(%d,%d)", NULL, NULL },
 #endif
 #ifdef TARGET_NR_listxattr
-{ TARGET_NR_listxattr, "listxattr" , NULL, NULL, NULL },
+{ TARGET_NR_listxattr, "listxattr" , NULL, print_listxattr,
+                       print_syscall_ret_listxattr},
 #endif
 #ifdef TARGET_NR_llistxattr
-{ TARGET_NR_llistxattr, "llistxattr" , NULL, NULL, NULL },
+{ TARGET_NR_llistxattr, "llistxattr" , NULL, print_llistxattr,
+                        print_syscall_ret_listxattr},
 #endif
 #ifdef TARGET_NR__llseek
 { TARGET_NR__llseek, "_llseek" , NULL, print__llseek, NULL },
@@ -510,7 +513,7 @@
 { TARGET_NR_lookup_dcookie, "lookup_dcookie" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_lremovexattr
-{ TARGET_NR_lremovexattr, "lremovexattr" , NULL, NULL, NULL },
+{ TARGET_NR_lremovexattr, "lremovexattr" , NULL, print_lremovexattr, NULL },
 #endif
 #ifdef TARGET_NR_lseek
 { TARGET_NR_lseek, "lseek" , NULL, NULL, NULL },
@@ -1116,7 +1119,7 @@
 { TARGET_NR_remap_file_pages, "remap_file_pages" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_removexattr
-{ TARGET_NR_removexattr, "removexattr" , NULL, NULL, NULL },
+{ TARGET_NR_removexattr, "removexattr" , NULL, print_removexattr, NULL },
 #endif
 #ifdef TARGET_NR_rename
 { TARGET_NR_rename, "rename" , NULL, print_rename, NULL },
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/6] linux-user: Add strace support for printing arguments of lseek()
  2020-06-08 16:43 [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls Filip Bozuta
                   ` (2 preceding siblings ...)
  2020-06-08 16:43 ` [PATCH v2 3/6] linux-user: Add strace support for printing argument of syscalls used for extended attributes Filip Bozuta
@ 2020-06-08 16:43 ` Filip Bozuta
  2020-06-10 17:58   ` Laurent Vivier
  2020-06-08 16:43 ` [PATCH v2 5/6] linux-user: Add strace support for printing arguments of chown()/lchown() Filip Bozuta
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Filip Bozuta @ 2020-06-08 16:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

This patch implements strace argument printing functionality for syscall:

    *lseek - reposition read/write file offset

         off_t lseek(int fd, off_t offset, int whence)
         man page: https://www.man7.org/linux/man-pages/man2/lseek.2.html

Implementation notes:

    The syscall's third argument "whence" has predefined values:
    "SEEK_SET","SEEK_CUR","SEEK_END","SEEK_DATA","SEEK_HOLE"
    and thus a separate printing function "print_lseek" was stated
    in file "strace.list". This function is defined in "strace.c"
    by using an existing function "print_raw_param()" to print
    the first and second argument and a switch(case) statement
    for the predefined values of the third argument.
    Values "SEEK_DATA" and "SEEK_HOLE" are defined in kernel version 3.1.
    That is the reason why case statements for these values are
    enwrapped in #ifdef directive.

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
---
 linux-user/strace.c    | 31 +++++++++++++++++++++++++++++++
 linux-user/strace.list |  2 +-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 59fdb0a05f..adf9a47465 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1842,6 +1842,37 @@ print__llseek(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_lseek
+static void
+print_lseek(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_raw_param("%d", arg0, 0);
+    print_raw_param(TARGET_ABI_FMT_ld, arg1, 0);
+    switch (arg2) {
+    case SEEK_SET:
+        qemu_log("SEEK_SET"); break;
+    case SEEK_CUR:
+        qemu_log("SEEK_CUR"); break;
+    case SEEK_END:
+        qemu_log("SEEK_END"); break;
+#ifdef SEEK_DATA
+    case SEEK_DATA:
+        qemu_log("SEEK_DATA"); break;
+#endif
+#ifdef SEEK_HOLE
+    case SEEK_HOLE:
+        qemu_log("SEEK_HOLE"); break;
+#endif
+    default:
+        print_raw_param("%#x", arg2, 1);
+    }
+    print_syscall_epilogue(name);
+}
+#endif
+
 #if defined(TARGET_NR_socket)
 static void
 print_socket(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 05a72370c1..bd04596c50 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -516,7 +516,7 @@
 { TARGET_NR_lremovexattr, "lremovexattr" , NULL, print_lremovexattr, NULL },
 #endif
 #ifdef TARGET_NR_lseek
-{ TARGET_NR_lseek, "lseek" , NULL, NULL, NULL },
+{ TARGET_NR_lseek, "lseek" , NULL, print_lseek, NULL },
 #endif
 #ifdef TARGET_NR_lsetxattr
 { TARGET_NR_lsetxattr, "lsetxattr" , NULL, NULL, NULL },
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/6] linux-user: Add strace support for printing arguments of chown()/lchown()
  2020-06-08 16:43 [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls Filip Bozuta
                   ` (3 preceding siblings ...)
  2020-06-08 16:43 ` [PATCH v2 4/6] linux-user: Add strace support for printing arguments of lseek() Filip Bozuta
@ 2020-06-08 16:43 ` Filip Bozuta
  2020-06-08 16:43 ` [PATCH v2 6/6] linux-user: Add strace support for printing arguments of fallocate() Filip Bozuta
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Filip Bozuta @ 2020-06-08 16:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

This patch implements strace argument printing functionality for syscalls:

    *chown, lchown - change ownership of a file

        int chown(const char *pathname, uid_t owner, gid_t group)
        int lchown(const char *pathname, uid_t owner, gid_t group)
        man page: https://www.man7.org/linux/man-pages/man2/lchown.2.html

Implementation notes:

    Both syscalls use strings as arguments and thus a separate
    printing function was stated in "strace.list" for them.
    Both syscalls share the same number and types of arguments
    and thus share a same definition in file "syscall.c".
    This defintion uses existing functions "print_string()" to
    print the string argument and "print_raw_param()" to print
    other two arguments that are of basic types.

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/strace.c    | 15 +++++++++++++++
 linux-user/strace.list |  4 ++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index adf9a47465..cd6a2b8e3f 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1461,6 +1461,21 @@ print_chmod(const struct syscallname *name,
 }
 #endif
 
+#if defined(TARGET_NR_chown) || defined(TARGET_NR_lchown)
+static void
+print_chown(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(name);
+    print_string(arg0, 0);
+    print_raw_param("%d", arg1, 0);
+    print_raw_param("%d", arg2, 1);
+    print_syscall_epilogue(name);
+}
+#define print_lchown     print_chown
+#endif
+
 #ifdef TARGET_NR_clock_adjtime
 static void
 print_clock_adjtime(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index bd04596c50..44eb515ca4 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -71,7 +71,7 @@
 { TARGET_NR_chmod, "chmod" , NULL, print_chmod, NULL },
 #endif
 #ifdef TARGET_NR_chown
-{ TARGET_NR_chown, "chown" , NULL, NULL, NULL },
+{ TARGET_NR_chown, "chown" , NULL, print_chown, NULL },
 #endif
 #ifdef TARGET_NR_chown32
 { TARGET_NR_chown32, "chown32" , NULL, NULL, NULL },
@@ -475,7 +475,7 @@
 { TARGET_NR_kill, "kill", NULL, print_kill, NULL },
 #endif
 #ifdef TARGET_NR_lchown
-{ TARGET_NR_lchown, "lchown" , NULL, NULL, NULL },
+{ TARGET_NR_lchown, "lchown" , NULL, print_lchown, NULL },
 #endif
 #ifdef TARGET_NR_lchown32
 { TARGET_NR_lchown32, "lchown32" , NULL, NULL, NULL },
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 6/6] linux-user: Add strace support for printing arguments of fallocate()
  2020-06-08 16:43 [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls Filip Bozuta
                   ` (4 preceding siblings ...)
  2020-06-08 16:43 ` [PATCH v2 5/6] linux-user: Add strace support for printing arguments of chown()/lchown() Filip Bozuta
@ 2020-06-08 16:43 ` Filip Bozuta
  2020-06-10 17:49   ` Laurent Vivier
  2020-06-08 18:59 ` [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls no-reply
  2020-06-08 21:50 ` no-reply
  7 siblings, 1 reply; 13+ messages in thread
From: Filip Bozuta @ 2020-06-08 16:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

From: Filip Bozuta <Filip.Bozuta@syrmia.com>

This patch implements strace argument printing functionality for following syscall:

    *fallocate - manipulate file space

        int fallocate(int fd, int mode, off_t offset, off_t len)
        man page: https://www.man7.org/linux/man-pages/man2/fallocate.2.html

Implementation notes:

    This syscall's second argument "mode" is composed of predefined values
    which represent flags that determine the type of operation that is
    to be performed on the file space. For that reason, a printing
    function "print_fallocate" was stated in file "strace.list". This printing
    function uses an already existing function "print_flags()" to print flags of
    the "mode" argument. These flags are stated inside an array "falloc_flags"
    that contains values of type "struct flags". These values are instantiated
    using an existing macro "FLAG_GENERIC()". Most of these flags are defined
    after kernel version 3.0 which is why they are enwrapped in an #ifdef
    directive.
    The syscall's third ant fourth argument are of type "off_t" which can
    cause variations between 32/64-bit architectures. To handle this variation,
    function "target_offset64()" was copied from file "strace.c" and used in
    "print_fallocate" to print "off_t" arguments for 32-bit architectures.

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
---
 linux-user/qemu.h      | 16 ++++++++++++++++
 linux-user/strace.c    | 40 ++++++++++++++++++++++++++++++++++++++++
 linux-user/strace.list |  2 +-
 linux-user/syscall.c   | 16 ----------------
 4 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 8f938b8105..be67391ba4 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -670,6 +670,22 @@ static inline int is_error(abi_long ret)
     return (abi_ulong)ret >= (abi_ulong)(-4096);
 }
 
+#if TARGET_ABI_BITS == 32
+static inline uint64_t target_offset64(uint32_t word0, uint32_t word1)
+{
+#ifdef TARGET_WORDS_BIGENDIAN
+    return ((uint64_t)word0 << 32) | word1;
+#else
+    return ((uint64_t)word1 << 32) | word0;
+#endif
+}
+#else /* TARGET_ABI_BITS == 32 */
+static inline uint64_t target_offset64(uint64_t word0, uint64_t word1)
+{
+    return word0;
+}
+#endif /* TARGET_ABI_BITS != 32 */
+
 /**
  * preexit_cleanup: housekeeping before the guest exits
  *
diff --git a/linux-user/strace.c b/linux-user/strace.c
index cd6a2b8e3f..3ef7f80cd7 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1144,6 +1144,26 @@ UNUSED static struct flags statx_mask[] = {
     FLAG_END,
 };
 
+UNUSED static struct flags falloc_flags[] = {
+    FLAG_GENERIC(FALLOC_FL_KEEP_SIZE),
+    FLAG_GENERIC(FALLOC_FL_PUNCH_HOLE),
+#ifdef FALLOC_FL_NO_HIDE_STALE
+    FLAG_GENERIC(FALLOC_FL_NO_HIDE_STALE),
+#endif
+#ifdef FALLOC_FL_COLLAPSE_RANGE
+    FLAG_GENERIC(FALLOC_FL_COLLAPSE_RANGE),
+#endif
+#ifdef FALLOC_FL_ZERO_RANGE
+    FLAG_GENERIC(FALLOC_FL_ZERO_RANGE),
+#endif
+#ifdef FALLOC_FL_INSERT_RANGE
+    FLAG_GENERIC(FALLOC_FL_INSERT_RANGE),
+#endif
+#ifdef FALLOC_FL_UNSHARE_RANGE
+    FLAG_GENERIC(FALLOC_FL_UNSHARE_RANGE),
+#endif
+};
+
 /*
  * print_xxx utility functions.  These are used to print syscall
  * parameters in certain format.  All of these have parameter
@@ -1561,6 +1581,26 @@ print_faccessat(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_fallocate
+static void
+print_fallocate(const struct syscallname *name,
+    abi_long arg0, abi_long arg1, abi_long arg2,
+    abi_long arg3, abi_long arg4, abi_long arg5)
+{
+    print_syscall_prologue(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);
+#else
+    print_raw_param(TARGET_ABI_FMT_ld, arg2, 0);
+    print_raw_param(TARGET_ABI_FMT_ld, arg3, 1);
+#endif
+    print_syscall_epilogue(name);
+}
+#endif
+
 #ifdef TARGET_NR_fchmodat
 static void
 print_fchmodat(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 44eb515ca4..a8d7cbe7a4 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -182,7 +182,7 @@
 { TARGET_NR_fadvise64_64, "fadvise64_64" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_fallocate
-{ TARGET_NR_fallocate, "fallocate" , NULL, NULL, NULL },
+{ TARGET_NR_fallocate, "fallocate" , NULL, print_fallocate, NULL },
 #endif
 #ifdef TARGET_NR_fanotify_init
 { TARGET_NR_fanotify_init, "fanotify_init" , NULL, NULL, NULL },
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 009bb67422..7cc5a65b4f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6608,22 +6608,6 @@ void syscall_init(void)
     }
 }
 
-#if TARGET_ABI_BITS == 32
-static inline uint64_t target_offset64(uint32_t word0, uint32_t word1)
-{
-#ifdef TARGET_WORDS_BIGENDIAN
-    return ((uint64_t)word0 << 32) | word1;
-#else
-    return ((uint64_t)word1 << 32) | word0;
-#endif
-}
-#else /* TARGET_ABI_BITS == 32 */
-static inline uint64_t target_offset64(uint64_t word0, uint64_t word1)
-{
-    return word0;
-}
-#endif /* TARGET_ABI_BITS != 32 */
-
 #ifdef TARGET_NR_truncate64
 static inline abi_long target_truncate64(void *cpu_env, const char *arg1,
                                          abi_long arg2,
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls
  2020-06-08 16:43 [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls Filip Bozuta
                   ` (5 preceding siblings ...)
  2020-06-08 16:43 ` [PATCH v2 6/6] linux-user: Add strace support for printing arguments of fallocate() Filip Bozuta
@ 2020-06-08 18:59 ` no-reply
  2020-06-08 21:50 ` no-reply
  7 siblings, 0 replies; 13+ messages in thread
From: no-reply @ 2020-06-08 18:59 UTC (permalink / raw)
  To: filip.bozuta; +Cc: qemu-devel, laurent

Patchew URL: https://patchew.org/QEMU/20200608164357.25065-1-filip.bozuta@syrmia.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200608164357.25065-1-filip.bozuta@syrmia.com
Subject: [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20200608164357.25065-1-filip.bozuta@syrmia.com -> patchew/20200608164357.25065-1-filip.bozuta@syrmia.com
Switched to a new branch 'test'
320f874 linux-user: Add strace support for printing arguments of fallocate()
a4df4b8 linux-user: Add strace support for printing arguments of chown()/lchown()
2a5a74d linux-user: Add strace support for printing arguments of lseek()
ebc81cb linux-user: Add strace support for printing argument of syscalls used for extended attributes
595dc96 linux-user: Add strace support for a group of syscalls
b53f9b9 linux-user: Extend strace support to enable argument printing after syscall execution

=== OUTPUT BEGIN ===
1/6 Checking commit b53f9b913213 (linux-user: Extend strace support to enable argument printing after syscall execution)
2/6 Checking commit 595dc9662da6 (linux-user: Add strace support for a group of syscalls)
3/6 Checking commit ebc81cb1d684 (linux-user: Add strace support for printing argument of syscalls used for extended attributes)
4/6 Checking commit 2a5a74d1c629 (linux-user: Add strace support for printing arguments of lseek())
5/6 Checking commit a4df4b85ae87 (linux-user: Add strace support for printing arguments of chown()/lchown())
6/6 Checking commit 320f8741ece4 (linux-user: Add strace support for printing arguments of fallocate())
ERROR: storage class should be at the beginning of the declaration
#69: FILE: linux-user/strace.c:1147:
+UNUSED static struct flags falloc_flags[] = {

total: 1 errors, 0 warnings, 104 lines checked

Patch 6/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200608164357.25065-1-filip.bozuta@syrmia.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls
  2020-06-08 16:43 [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls Filip Bozuta
                   ` (6 preceding siblings ...)
  2020-06-08 18:59 ` [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls no-reply
@ 2020-06-08 21:50 ` no-reply
  7 siblings, 0 replies; 13+ messages in thread
From: no-reply @ 2020-06-08 21:50 UTC (permalink / raw)
  To: filip.bozuta; +Cc: qemu-devel, laurent

Patchew URL: https://patchew.org/QEMU/20200608164357.25065-1-filip.bozuta@syrmia.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200608164357.25065-1-filip.bozuta@syrmia.com
Subject: [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
dc47702 linux-user: Add strace support for printing arguments of fallocate()
8478ea8 linux-user: Add strace support for printing arguments of chown()/lchown()
f287b7f linux-user: Add strace support for printing arguments of lseek()
615a5d8 linux-user: Add strace support for printing argument of syscalls used for extended attributes
b0acbe9 linux-user: Add strace support for a group of syscalls
3370ce2 linux-user: Extend strace support to enable argument printing after syscall execution

=== OUTPUT BEGIN ===
1/6 Checking commit 3370ce28f836 (linux-user: Extend strace support to enable argument printing after syscall execution)
2/6 Checking commit b0acbe9ef27d (linux-user: Add strace support for a group of syscalls)
3/6 Checking commit 615a5d8e9d21 (linux-user: Add strace support for printing argument of syscalls used for extended attributes)
4/6 Checking commit f287b7f80630 (linux-user: Add strace support for printing arguments of lseek())
5/6 Checking commit 8478ea817695 (linux-user: Add strace support for printing arguments of chown()/lchown())
6/6 Checking commit dc477027df64 (linux-user: Add strace support for printing arguments of fallocate())
ERROR: storage class should be at the beginning of the declaration
#69: FILE: linux-user/strace.c:1147:
+UNUSED static struct flags falloc_flags[] = {

total: 1 errors, 0 warnings, 104 lines checked

Patch 6/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200608164357.25065-1-filip.bozuta@syrmia.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/6] linux-user: Extend strace support to enable argument printing after syscall execution
  2020-06-08 16:43 ` [PATCH v2 1/6] linux-user: Extend strace support to enable argument printing after syscall execution Filip Bozuta
@ 2020-06-10 17:33   ` Laurent Vivier
  0 siblings, 0 replies; 13+ messages in thread
From: Laurent Vivier @ 2020-06-10 17:33 UTC (permalink / raw)
  To: Filip Bozuta, qemu-devel

Le 08/06/2020 à 18:43, Filip Bozuta a écrit :
> From: Filip Bozuta <Filip.Bozuta@syrmia.com>
> 
>     Structure "struct syscallname" in file "strace.c" is used for "-strace"
>     to print arguments and return values of syscalls. The last field of
>     this structure "result" represents the calling function that prints the
>     return values. This field was extended in this patch so that this functions
>     takes all syscalls arguments beside the return value. In this way, it enables
>     "-strace" to print arguments of syscalls that have changed after the syscall
>     execution. This extension will be useful as there are many syscalls that
>     return values inside their arguments (i.e. listxattr() that returns the list
>     of extended attributes inside the "list" argument).
> 
> Implementation notes:
> 
>     Since there are already three existing "print_syscall_ret*" functions inside
>     "strace.c" ("print_syscall_ret_addr()", "print_syscall_ret_adjtimex()",
>     "print_syscall_ret_newselect()"), they were changed to have all syscall arguments
>     beside the return value. This was done so that these functions don't cause build
>     errors (even though syscall arguments are not used in these functions).
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/qemu.h    |  4 +++-
>  linux-user/strace.c  | 24 ++++++++++++++++++------
>  linux-user/syscall.c |  2 +-
>  3 files changed, 22 insertions(+), 8 deletions(-)
> 

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 6/6] linux-user: Add strace support for printing arguments of fallocate()
  2020-06-08 16:43 ` [PATCH v2 6/6] linux-user: Add strace support for printing arguments of fallocate() Filip Bozuta
@ 2020-06-10 17:49   ` Laurent Vivier
  0 siblings, 0 replies; 13+ messages in thread
From: Laurent Vivier @ 2020-06-10 17:49 UTC (permalink / raw)
  To: Filip Bozuta, qemu-devel

Le 08/06/2020 à 18:43, Filip Bozuta a écrit :
> From: Filip Bozuta <Filip.Bozuta@syrmia.com>
> 
> This patch implements strace argument printing functionality for following syscall:
> 
>     *fallocate - manipulate file space
> 
>         int fallocate(int fd, int mode, off_t offset, off_t len)
>         man page: https://www.man7.org/linux/man-pages/man2/fallocate.2.html
> 
> Implementation notes:
> 
>     This syscall's second argument "mode" is composed of predefined values
>     which represent flags that determine the type of operation that is
>     to be performed on the file space. For that reason, a printing
>     function "print_fallocate" was stated in file "strace.list". This printing
>     function uses an already existing function "print_flags()" to print flags of
>     the "mode" argument. These flags are stated inside an array "falloc_flags"
>     that contains values of type "struct flags". These values are instantiated
>     using an existing macro "FLAG_GENERIC()". Most of these flags are defined
>     after kernel version 3.0 which is why they are enwrapped in an #ifdef
>     directive.
>     The syscall's third ant fourth argument are of type "off_t" which can
>     cause variations between 32/64-bit architectures. To handle this variation,
>     function "target_offset64()" was copied from file "strace.c" and used in
>     "print_fallocate" to print "off_t" arguments for 32-bit architectures.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/qemu.h      | 16 ++++++++++++++++
>  linux-user/strace.c    | 40 ++++++++++++++++++++++++++++++++++++++++
>  linux-user/strace.list |  2 +-
>  linux-user/syscall.c   | 16 ----------------
>  4 files changed, 57 insertions(+), 17 deletions(-)
> 

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 4/6] linux-user: Add strace support for printing arguments of lseek()
  2020-06-08 16:43 ` [PATCH v2 4/6] linux-user: Add strace support for printing arguments of lseek() Filip Bozuta
@ 2020-06-10 17:58   ` Laurent Vivier
  0 siblings, 0 replies; 13+ messages in thread
From: Laurent Vivier @ 2020-06-10 17:58 UTC (permalink / raw)
  To: Filip Bozuta, qemu-devel

Le 08/06/2020 à 18:43, Filip Bozuta a écrit :
> From: Filip Bozuta <Filip.Bozuta@syrmia.com>
> 
> This patch implements strace argument printing functionality for syscall:
> 
>     *lseek - reposition read/write file offset
> 
>          off_t lseek(int fd, off_t offset, int whence)
>          man page: https://www.man7.org/linux/man-pages/man2/lseek.2.html
> 
> Implementation notes:
> 
>     The syscall's third argument "whence" has predefined values:
>     "SEEK_SET","SEEK_CUR","SEEK_END","SEEK_DATA","SEEK_HOLE"
>     and thus a separate printing function "print_lseek" was stated
>     in file "strace.list". This function is defined in "strace.c"
>     by using an existing function "print_raw_param()" to print
>     the first and second argument and a switch(case) statement
>     for the predefined values of the third argument.
>     Values "SEEK_DATA" and "SEEK_HOLE" are defined in kernel version 3.1.
>     That is the reason why case statements for these values are
>     enwrapped in #ifdef directive.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/strace.c    | 31 +++++++++++++++++++++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 32 insertions(+), 1 deletion(-)

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/6] linux-user: Add strace support for printing argument of syscalls used for extended attributes
  2020-06-08 16:43 ` [PATCH v2 3/6] linux-user: Add strace support for printing argument of syscalls used for extended attributes Filip Bozuta
@ 2020-06-10 19:06   ` Laurent Vivier
  0 siblings, 0 replies; 13+ messages in thread
From: Laurent Vivier @ 2020-06-10 19:06 UTC (permalink / raw)
  To: Filip Bozuta, qemu-devel

Le 08/06/2020 à 18:43, Filip Bozuta a écrit :
> From: Filip Bozuta <Filip.Bozuta@syrmia.com>
> 
> This patch implements strace argument printing functionality for following syscalls:
> 
>     *getxattr, lgetxattr, fgetxattr - retrieve an extended attribute value
> 
>         ssize_t getxattr(const char *path, const char *name, void *value, size_t size)
>         ssize_t lgetxattr(const char *path, const char *name, void *value, size_t size)
>         ssize_t fgetxattr(int fd, const char *name, void *value, size_t size)
>         man page: https://www.man7.org/linux/man-pages/man2/getxattr.2.html
> 
>     *listxattr, llistxattr, flistxattr - list extended attribute names
> 
>         ssize_t listxattr(const char *path, char *list, size_t size)
>         ssize_t llistxattr(const char *path, char *list, size_t size)
>         ssize_t flistxattr(int fd, char *list, size_t size)
>         man page: https://www.man7.org/linux/man-pages/man2/listxattr.2.html
> 
>     *removexattr, lremovexattr, fremovexattr - remove an extended attribute
> 
>          int removexattr(const char *path, const char *name)
>          int lremovexattr(const char *path, const char *name)
>          int fremovexattr(int fd, const char *name)
>          man page: https://www.man7.org/linux/man-pages/man2/removexattr.2.html
> 
> Implementation notes:
> 
>     All of the syscalls have strings as argument types and thus a separate
>     printing function was stated in file "strace.list" for every one of them.
>     All of these printing functions were defined in "strace.c" using existing
>     printing functions for appropriate argument types:
>        "print_string()" - for (const char*) type
>        "print_pointer()" - for (char*) and (void *) type
>        "print_raw_param()" for (int) and (size_t) type
>     Syscalls "getxattr()" and "lgetxattr()" have the same number and type of
>     arguments and thus their print functions ("print_getxattr", "print_lgetxattr")
>     share a same definition. The same statement applies to syscalls "listxattr()"
>     and "llistxattr()".
>     Function "print_syscall_ret_listxattr()" was added to print the returned list
>     of extended attributes for syscalls and was listed as a "result" function in file
>     "strace.list" for syscalls: "listxattr(), llistxattr(), flistxattr()".
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/strace.c    | 126 +++++++++++++++++++++++++++++++++++++++++
>  linux-user/strace.list |  21 ++++---
>  2 files changed, 138 insertions(+), 9 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index f980451e3f..59fdb0a05f 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -830,6 +830,45 @@ print_syscall_ret_adjtimex(const struct syscallname *name, abi_long ret,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_listxattr) || defined(TARGET_NR_llistxattr) \
> + || defined(TARGGET_NR_flistxattr)
> +static void
> +print_syscall_ret_listxattr(const struct syscallname *name, abi_long ret,
> +                            abi_long arg0, abi_long arg1, abi_long arg2,
> +                            abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    const char *errstr = NULL;
> +
> +    qemu_log(" = ");
> +    if (ret < 0) {
> +        qemu_log("-1 errno=%d", errno);
> +        errstr = target_strerror(-ret);
> +        if (errstr) {
> +            qemu_log(" (%s)", errstr);
> +        }

We have several time this kind of code in strace.c
(print_syscall_ret_addr, print_syscall_ret_adjtimex, print_syscall_ret)
perhaps it could be moved  to generic function (in a previous patch)?

> +    } else {
> +        qemu_log(TARGET_ABI_FMT_ld, ret);
> +        qemu_log(" (list = ");
> +        if (arg1 != 0) {
> +            abi_long attr = arg1;
> +            for (;;) {

We should avoid an infinite loop, and it's easy as you now the size of
the buffer (ret).

> +                print_string(attr, 1);
> +                attr += target_strlen(attr) + 1;
> +                if (target_strlen(attr) == 0) {
> +                    break;
> +                }
> +                qemu_log(",");
> +            }
> +        } else {
> +            qemu_log("NULL");
> +        }
> +        qemu_log(")");
> +    }
> +
> +    qemu_log("\n");
> +}

You should do as for the entry functions, and define the ones for
llistxattr and flistxattr:

#define print_syscall_ret_flistxattr print_syscall_ret_listxattr
#define print_syscall_ret_xlistxattr print_syscall_ret_listxattr

I have no preference on that but it's to be homogeneous with the rest of
the code.

> +#endif
> +
>  UNUSED static struct flags access_flags[] = {
>      FLAG_GENERIC(F_OK),
>      FLAG_GENERIC(R_OK),
> @@ -1637,6 +1676,93 @@ print_fcntl(const struct syscallname *name,
>  #define print_fcntl64   print_fcntl
>  #endif
>  
> +#ifdef TARGET_NR_fgetxattr
> +static void
> +print_fgetxattr(const struct syscallname *name,
> +    abi_long arg0, abi_long arg1, abi_long arg2,
> +    abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);
> +    print_string(arg1, 0);
> +    print_pointer(arg2, 0);
> +    print_raw_param(TARGET_FMT_lu, arg3, 1);
> +    print_syscall_epilogue(name);
> +}
> +#endif
> +
> +#ifdef TARGET_NR_flistxattr
> +static void
> +print_flistxattr(const struct syscallname *name,
> +    abi_long arg0, abi_long arg1, abi_long arg2,
> +    abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);
> +    print_pointer(arg1, 0);
> +    print_raw_param(TARGET_FMT_lu, arg2, 1);
> +    print_syscall_epilogue(name);
> +}
> +#endif
> +
> +#if defined(TARGET_NR_getxattr) || defined(TARGET_NR_lgetxattr)
> +static void
> +print_getxattr(const struct syscallname *name,
> +    abi_long arg0, abi_long arg1, abi_long arg2,
> +    abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_string(arg0, 0);
> +    print_string(arg1, 0);
> +    print_pointer(arg2, 0);
> +    print_raw_param(TARGET_FMT_lu, arg3, 1);
> +    print_syscall_epilogue(name);
> +}
> +#define print_lgetxattr     print_getxattr
> +#endif
> +
> +#if defined(TARGET_NR_listxattr) || defined(TARGET_NR_llistxattr)
> +static void
> +print_listxattr(const struct syscallname *name,
> +    abi_long arg0, abi_long arg1, abi_long arg2,
> +    abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_string(arg0, 0);
> +    print_pointer(arg1, 0);
> +    print_raw_param(TARGET_FMT_lu, arg2, 1);
> +    print_syscall_epilogue(name);
> +}
> +#define print_llistxattr     print_listxattr
> +#endif
> +
> +#if defined(TARGET_NR_fremovexattr)
> +static void
> +print_fremovexattr(const struct syscallname *name,
> +    abi_long arg0, abi_long arg1, abi_long arg2,
> +    abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_raw_param("%d", arg0, 0);
> +    print_string(arg1, 1);
> +    print_syscall_epilogue(name);
> +}
> +#endif
> +
> +#if defined(TARGET_NR_removexattr) || defined(TARGET_NR_lremovexattr)
> +static void
> +print_removexattr(const struct syscallname *name,
> +    abi_long arg0, abi_long arg1, abi_long arg2,
> +    abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +    print_syscall_prologue(name);
> +    print_string(arg0, 0);
> +    print_string(arg1, 1);
> +    print_syscall_epilogue(name);
> +}
> +#define print_lremovexattr     print_removexattr
> +#endif
> +
>  #ifdef TARGET_NR_futimesat
>  static void
>  print_futimesat(const struct syscallname *name,
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index fb9799e7e6..05a72370c1 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -218,13 +218,14 @@
>  { TARGET_NR_fdatasync, "fdatasync" , "%s(%d)", NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_fgetxattr
> -{ TARGET_NR_fgetxattr, "fgetxattr" , NULL, NULL, NULL },
> +{ TARGET_NR_fgetxattr, "fgetxattr" , NULL, print_fgetxattr, NULL },
>  #endif
>  #ifdef TARGET_NR_finit_module
>  { TARGET_NR_finit_module, "finit_module" , NULL, NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_flistxattr
> -{ TARGET_NR_flistxattr, "flistxattr" , NULL, NULL, NULL },
> +{ TARGET_NR_flistxattr, "flistxattr" , NULL, print_flistxattr,
> +                        print_syscall_ret_listxattr},

print_syscall_ret_flistxattr

>  #endif
>  #ifdef TARGET_NR_flock
>  { TARGET_NR_flock, "flock" , NULL, NULL, NULL },
> @@ -233,7 +234,7 @@
>  { TARGET_NR_fork, "fork" , "%s()", NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_fremovexattr
> -{ TARGET_NR_fremovexattr, "fremovexattr" , NULL, NULL, NULL },
> +{ TARGET_NR_fremovexattr, "fremovexattr" , NULL, print_fremovexattr, NULL },
>  #endif
>  #ifdef TARGET_NR_fsetxattr
>  { TARGET_NR_fsetxattr, "fsetxattr" , NULL, NULL, NULL },
> @@ -396,7 +397,7 @@
>  { TARGET_NR_getuid32, "getuid32" , NULL, NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_getxattr
> -{ TARGET_NR_getxattr, "getxattr" , NULL, NULL, NULL },
> +{ TARGET_NR_getxattr, "getxattr" , NULL, print_getxattr, NULL },
>  #endif
>  #ifdef TARGET_NR_getxgid
>  { TARGET_NR_getxgid, "getxgid" , NULL, NULL, NULL },
> @@ -480,7 +481,7 @@
>  { TARGET_NR_lchown32, "lchown32" , NULL, NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_lgetxattr
> -{ TARGET_NR_lgetxattr, "lgetxattr" , NULL, NULL, NULL },
> +{ TARGET_NR_lgetxattr, "lgetxattr" , NULL, print_lgetxattr, NULL },
>  #endif
>  #ifdef TARGET_NR_link
>  { TARGET_NR_link, "link" , NULL, print_link, NULL },
> @@ -495,10 +496,12 @@
>  { TARGET_NR_listen, "listen" , "%s(%d,%d)", NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_listxattr
> -{ TARGET_NR_listxattr, "listxattr" , NULL, NULL, NULL },
> +{ TARGET_NR_listxattr, "listxattr" , NULL, print_listxattr,
> +                       print_syscall_ret_listxattr},
>  #endif
>  #ifdef TARGET_NR_llistxattr
> -{ TARGET_NR_llistxattr, "llistxattr" , NULL, NULL, NULL },
> +{ TARGET_NR_llistxattr, "llistxattr" , NULL, print_llistxattr,
> +                        print_syscall_ret_listxattr},

print_syscall_ret_llistxattr

>  #endif
>  #ifdef TARGET_NR__llseek
>  { TARGET_NR__llseek, "_llseek" , NULL, print__llseek, NULL },
> @@ -510,7 +513,7 @@
>  { TARGET_NR_lookup_dcookie, "lookup_dcookie" , NULL, NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_lremovexattr
> -{ TARGET_NR_lremovexattr, "lremovexattr" , NULL, NULL, NULL },
> +{ TARGET_NR_lremovexattr, "lremovexattr" , NULL, print_lremovexattr, NULL },
>  #endif
>  #ifdef TARGET_NR_lseek
>  { TARGET_NR_lseek, "lseek" , NULL, NULL, NULL },
> @@ -1116,7 +1119,7 @@
>  { TARGET_NR_remap_file_pages, "remap_file_pages" , NULL, NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_removexattr
> -{ TARGET_NR_removexattr, "removexattr" , NULL, NULL, NULL },
> +{ TARGET_NR_removexattr, "removexattr" , NULL, print_removexattr, NULL },
>  #endif
>  #ifdef TARGET_NR_rename
>  { TARGET_NR_rename, "rename" , NULL, print_rename, NULL },
> 

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-06-10 19:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-08 16:43 [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls Filip Bozuta
2020-06-08 16:43 ` [PATCH v2 1/6] linux-user: Extend strace support to enable argument printing after syscall execution Filip Bozuta
2020-06-10 17:33   ` Laurent Vivier
2020-06-08 16:43 ` [PATCH v2 2/6] linux-user: Add strace support for a group of syscalls Filip Bozuta
2020-06-08 16:43 ` [PATCH v2 3/6] linux-user: Add strace support for printing argument of syscalls used for extended attributes Filip Bozuta
2020-06-10 19:06   ` Laurent Vivier
2020-06-08 16:43 ` [PATCH v2 4/6] linux-user: Add strace support for printing arguments of lseek() Filip Bozuta
2020-06-10 17:58   ` Laurent Vivier
2020-06-08 16:43 ` [PATCH v2 5/6] linux-user: Add strace support for printing arguments of chown()/lchown() Filip Bozuta
2020-06-08 16:43 ` [PATCH v2 6/6] linux-user: Add strace support for printing arguments of fallocate() Filip Bozuta
2020-06-10 17:49   ` Laurent Vivier
2020-06-08 18:59 ` [PATCH v2 0/6] Add strace support for printing arguments of selected syscalls no-reply
2020-06-08 21:50 ` no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.