[PATCH 0/5] Add strace support for printing arguments of selected syscalls

[PATCH 0/5] Add strace support for printing arguments of selected syscalls

[Filip Bozuta]

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

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

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

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.

Filip Bozuta (5):
  linux-user: Add strace support for a group of syscalls
  linux-user: Add strace support for printing argument of syscalls used
    for extend 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/strace.c    | 174 +++++++++++++++++++++++++++++++++++++++++
 linux-user/strace.list |  28 +++----
 2 files changed, 188 insertions(+), 14 deletions(-)

-- 
2.17.1

[PATCH 1/5] linux-user: Add strace support for a group of syscalls

[Filip Bozuta]

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>
---
 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 0d9095c674..c578876d22 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1353,6 +1353,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,
@@ -1617,7 +1629,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

[PATCH 2/5] linux-user: Add strace support for printing argument of syscalls used for extend attributes

[Filip Bozuta]

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

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_strig()" - 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()".

Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
---
 linux-user/strace.c    | 60 ++++++++++++++++++++++++++++++++++++++++++
 linux-user/strace.list | 12 ++++-----
 2 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index c578876d22..5cf419989c 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1629,6 +1629,66 @@ 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("%u", 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("%u", 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("%u", 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("%u", arg2, 1);
+    print_syscall_epilogue(name);
+}
+#define print_llistxattr     print_listxattr
+#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..8d51c54bca 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -218,13 +218,13 @@
 { 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, NULL },
 #endif
 #ifdef TARGET_NR_flock
 { TARGET_NR_flock, "flock" , NULL, NULL, NULL },
@@ -396,7 +396,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 +480,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 +495,10 @@
 { 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, NULL },
 #endif
 #ifdef TARGET_NR_llistxattr
-{ TARGET_NR_llistxattr, "llistxattr" , NULL, NULL, NULL },
+{ TARGET_NR_llistxattr, "llistxattr" , NULL, print_llistxattr, NULL },
 #endif
 #ifdef TARGET_NR__llseek
 { TARGET_NR__llseek, "_llseek" , NULL, print__llseek, NULL },
-- 
2.17.1

[PATCH 3/5] linux-user: Add strace support for printing arguments of lseek()

[Filip Bozuta]

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    | 32 ++++++++++++++++++++++++++++++++
 linux-user/strace.list |  2 +-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 5cf419989c..b7f012f1b5 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1768,6 +1768,38 @@ 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("%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);
+        qemu_log(" /* SEEK_??? */");
+    }
+    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 8d51c54bca..5a56212532 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -513,7 +513,7 @@
 { TARGET_NR_lremovexattr, "lremovexattr" , NULL, NULL, 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

[PATCH 4/5] linux-user: Add strace support for printing arguments of chown()/lchown()

[Filip Bozuta]

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>
---
 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 b7f012f1b5..542bfdeb91 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1414,6 +1414,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 5a56212532..b72f757d3f 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 },
@@ -474,7 +474,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

[PATCH 5/5] linux-user: Add strace support for printing arguments of fallocate()

[Filip Bozuta]

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/strace.c    | 56 ++++++++++++++++++++++++++++++++++++++++++
 linux-user/strace.list |  2 +-
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 542bfdeb91..3998a00bb4 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -34,6 +34,22 @@ struct syscallname {
 #define UNUSED
 #endif
 
+#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 */
+
 /*
  * Structure used to translate flag values into strings.  This is
  * similar that is in the actual strace tool.
@@ -1097,6 +1113,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
@@ -1514,6 +1550,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("%ld", target_offset64(arg2, arg3), 0);
+    print_raw_param("%ld", target_offset64(arg4, arg5), 1);
+#else
+    print_raw_param("%ld", arg2, 0);
+    print_raw_param("%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 b72f757d3f..d7458ce884 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 },
-- 
2.17.1

Re: [PATCH 0/5] Add strace support for printing arguments of selected syscalls

[no-reply]

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



Hi,

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

Message-id: 20200602115331.1659-1-filip.bozuta@syrmia.com
Subject: [PATCH 0/5] 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
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200529221450.26673-1-alxndr@bu.edu -> patchew/20200529221450.26673-1-alxndr@bu.edu
Switched to a new branch 'test'
b550765 linux-user: Add strace support for printing arguments of fallocate()
5330146 linux-user: Add strace support for printing arguments of chown()/lchown()
989e4e7 linux-user: Add strace support for printing arguments of lseek()
b783ad4 linux-user: Add strace support for printing argument of syscalls used for extend attributes
3e8a5b0 linux-user: Add strace support for a group of syscalls

=== OUTPUT BEGIN ===
1/5 Checking commit 3e8a5b0eb48a (linux-user: Add strace support for a group of syscalls)
2/5 Checking commit b783ad4bf62e (linux-user: Add strace support for printing argument of syscalls used for extend attributes)
3/5 Checking commit 989e4e773fae (linux-user: Add strace support for printing arguments of lseek())
WARNING: Block comments use a leading /* on a separate line
#64: FILE: linux-user/strace.c:1797:
+        qemu_log(" /* SEEK_??? */");

total: 0 errors, 1 warnings, 46 lines checked

Patch 3/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/5 Checking commit 53301467fe82 (linux-user: Add strace support for printing arguments of chown()/lchown())
5/5 Checking commit b55076575a2a (linux-user: Add strace support for printing arguments of fallocate())
ERROR: storage class should be at the beginning of the declaration
#65: FILE: linux-user/strace.c:1116:
+UNUSED static struct flags falloc_flags[] = {

total: 1 errors, 0 warnings, 82 lines checked

Patch 5/5 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/20200602115331.1659-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

Re: [PATCH 1/5] linux-user: Add strace support for a group of syscalls

[Laurent Vivier]

Le 02/06/2020 à 13:53, Filip Bozuta a écrit :
> 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>
> ---
>  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 0d9095c674..c578876d22 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1353,6 +1353,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,
> @@ -1617,7 +1629,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 },
> 

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

Re: [PATCH 3/5] linux-user: Add strace support for printing arguments of lseek()

[Laurent Vivier]

Le 02/06/2020 à 13:53, 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    | 32 ++++++++++++++++++++++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 5cf419989c..b7f012f1b5 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1768,6 +1768,38 @@ 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("%ld", arg1, 0);

TARGET_ABI_FMT_ld is better than "%ld", because abi_long can be a 32bit
or a 64bit value, and be different between host and target.

> +    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);
> +        qemu_log(" /* SEEK_??? */");

remove this line ^^^

> +    }
> +    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 8d51c54bca..5a56212532 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -513,7 +513,7 @@
>  { TARGET_NR_lremovexattr, "lremovexattr" , NULL, NULL, 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 },
> 

Thanks,
Laurent

Re: [PATCH 0/5] Add strace support for printing arguments of selected syscalls

[Alex Bennée]

Filip Bozuta <filip.bozuta@syrmia.com> writes:

> From: Filip Bozuta <Filip.Bozuta@syrmia.com>
>
> This series covers strace support for printing arguments of following syscalls:
>
>     *acct()         *lgetxattr()        *lseek()
>     *fsync()        *fgetxattr()        *chown()
>     *fdatasync()    *listxattr()        *lchown()
>     *listen()       *llistxattr()       *fallocate()
>     *getxattr()     *flistxattr()
>
> 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:

How big is this mini-test? Is it worth adding to tests/tcg?

>
>         * 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.

If we have reference traces from real HW we can compare them using
diff-out or conditional-diff-out make rules. See the run-float_%:
float_% rules in tests/tcg/multiarch/Makefile.target.

>
> Filip Bozuta (5):
>   linux-user: Add strace support for a group of syscalls
>   linux-user: Add strace support for printing argument of syscalls used
>     for extend 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/strace.c    | 174 +++++++++++++++++++++++++++++++++++++++++
>  linux-user/strace.list |  28 +++----
>  2 files changed, 188 insertions(+), 14 deletions(-)


-- 
Alex Bennée

Re: [PATCH 4/5] linux-user: Add strace support for printing arguments of chown()/lchown()

[Laurent Vivier]

Le 02/06/2020 à 13:53, Filip Bozuta a écrit :
> 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>
> ---
>  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 b7f012f1b5..542bfdeb91 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1414,6 +1414,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 5a56212532..b72f757d3f 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 },
> @@ -474,7 +474,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 },
> 

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

Re: [PATCH 5/5] linux-user: Add strace support for printing arguments of fallocate()

[Laurent Vivier]

Le 02/06/2020 à 13:53, 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/strace.c    | 56 ++++++++++++++++++++++++++++++++++++++++++
>  linux-user/strace.list |  2 +-
>  2 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 542bfdeb91..3998a00bb4 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -34,6 +34,22 @@ struct syscallname {
>  #define UNUSED
>  #endif
>  
> +#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 */
> +

Rather than copying that from syscall.c perhaps you can move the
definition from linux-user/syscall.c to linux-user/qemu.h?

>  /*
>   * Structure used to translate flag values into strings.  This is
>   * similar that is in the actual strace tool.
> @@ -1097,6 +1113,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
> @@ -1514,6 +1550,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("%ld", target_offset64(arg2, arg3), 0);
> +    print_raw_param("%ld", target_offset64(arg4, arg5), 1);

target_offset64 returns uint64_t so use PRIu64 rather than "%ld"

> +#else
> +    print_raw_param("%ld", arg2, 0);
> +    print_raw_param("%ld", arg3, 1);

TARGET_ABI_FMT_ld rather than "%ld"

> +#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 b72f757d3f..d7458ce884 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 },
> 

Thanks,
Laurent

Re: [PATCH 2/5] linux-user: Add strace support for printing argument of syscalls used for extend attributes

[Laurent Vivier]

Le 02/06/2020 à 13:53, 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
> 
> 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_strig()" - 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()".
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/strace.c    | 60 ++++++++++++++++++++++++++++++++++++++++++
>  linux-user/strace.list | 12 ++++-----
>  2 files changed, 66 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index c578876d22..5cf419989c 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1629,6 +1629,66 @@ 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("%u", arg3, 1);

size_t is generally "unsigned long", so you should use TARGET_FMT_lu

> +    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("%u", arg2, 1);

TARGET_FMT_lu

> +    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("%u", arg3, 1);

TARGET_FMT_lu

> +    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("%u", arg2, 1);

TARGET_FMT_lu

> +    print_syscall_epilogue(name);
> +}
> +#define print_llistxattr     print_listxattr
> +#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..8d51c54bca 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -218,13 +218,13 @@
>  { 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, NULL },
>  #endif
>  #ifdef TARGET_NR_flock
>  { TARGET_NR_flock, "flock" , NULL, NULL, NULL },
> @@ -396,7 +396,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 +480,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 +495,10 @@
>  { 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, NULL },
>  #endif
>  #ifdef TARGET_NR_llistxattr
> -{ TARGET_NR_llistxattr, "llistxattr" , NULL, NULL, NULL },
> +{ TARGET_NR_llistxattr, "llistxattr" , NULL, print_llistxattr, NULL },
>  #endif
>  #ifdef TARGET_NR__llseek
>  { TARGET_NR__llseek, "_llseek" , NULL, print__llseek, NULL },
> 

For the listxattr functions perhaps you can add a
print_syscall_ret_listxattr function to dump the returned list of
attributes.

It would be interesting to have also a print_syscall_ret_XXX function
for the getxattr functions to dump the returned values but it is not as
easy as for the list as value can be textual or binary data.

Thanks,
Laurent