[Qemu-devel] [PATCH v7 0/9] linux-user: strace improvements

[Qemu-devel] [PATCH v7 0/9] linux-user: strace improvements

[Philippe Mathieu-Daudé]

Hi Laurent,

Few patches I'v been writting while trying to figure out this issue:
http://lists.nongnu.org/archive/html/qemu-arm/2018-01/msg00514.html

As usual with linux-user files, this series will trigger some checkpatch
benign warnings.

Regards,

Phil.

Since v6:
- Use ABI types in sockaddr

Since v5:
- dropped 'Verify recvfrom(addr)' since failing LTP testsuite (see [1])
- also define print_sockfd() for bind() (patches #6 and #7)

Since v4:
- rebased on master (no change)

Since v3:
- addressed Laurent comments
- added print_sockfd()
- removed the print_sockaddr_ptr() patch, also the two
  getsockname()/recvfrom() patches for after 3.0.

Since v2:
- display invalid pointer in print_timeval() and print_timezone()
- do not display gettimeofday() arguments

Since v1:
- addressed Laurent comments
- added 'last' argument to print_sockaddr()
- reordered series, so patches already correct can get applied directly
- dropped "linux-user/syscall: simplify recvfrom()" for now

v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg05855.html
v2: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08216.html
v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg00411.html
v5: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02067.html
v6: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg01346.html
[1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02807.html

Philippe Mathieu-Daudé (9):
  linux-user/strace: Display invalid pointer in print_timeval()
  linux-user/strace: Add print_timezone()
  linux-user/strace: Improve settimeofday()
  linux-user/syscall: Introduce target_sockaddr_nl
  linux-user/strace: Dump AF_NETLINK sockaddr content
  linux-user/strace: Add print_sockfd()
  linux-user/strace: Improve bind() output
  linux-user/strace: Let print_sockaddr() have a 'last' argument
  linux-user/syscall: Align target_sockaddr fields using ABI types

 linux-user/strace.c       | 120 +++++++++++++++++++++++++++++++++-----
 linux-user/strace.list    |   4 +-
 linux-user/syscall.c      |   6 +-
 linux-user/syscall_defs.h |  41 +++++++------
 4 files changed, 137 insertions(+), 34 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v7 3/9] linux-user/strace: Improve settimeofday()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/strace.c    | 13 +++++++++++++
 linux-user/strace.list |  2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 2cd6687cd9..fd5596a640 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1583,6 +1583,19 @@ print_futimesat(const struct syscallname *name,
 }
 #endif
 
+#ifdef TARGET_NR_settimeofday
+static void
+print_settimeofday(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_timeval(arg0, 0);
+    print_timezone(arg1, 1);
+    print_syscall_epilogue(name);
+}
+#endif
+
 #ifdef TARGET_NR_link
 static void
 print_link(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 63a946642d..1ff9168369 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1345,7 +1345,7 @@
 { TARGET_NR_set_tid_address, "set_tid_address" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_settimeofday
-{ TARGET_NR_settimeofday, "settimeofday" , NULL, NULL, NULL },
+{ TARGET_NR_settimeofday, "settimeofday" , NULL, print_settimeofday, NULL },
 #endif
 #ifdef TARGET_NR_setuid
 { TARGET_NR_setuid, "setuid" , NULL, NULL, NULL },
-- 
2.20.1

[Qemu-devel] [PATCH v7 4/9] linux-user/syscall: Introduce target_sockaddr_nl

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
---
v7: use abi_ushort and abi_uint to keep alignments good (lvivier)
---
 linux-user/syscall.c      | 6 ++++--
 linux-user/syscall_defs.h | 7 +++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e2af3c1494..f1ab81b917 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1496,8 +1496,10 @@ static inline abi_long host_to_target_sockaddr(abi_ulong target_addr,
         sizeof(target_saddr->sa_family)) {
         target_saddr->sa_family = tswap16(addr->sa_family);
     }
-    if (addr->sa_family == AF_NETLINK && len >= sizeof(struct sockaddr_nl)) {
-        struct sockaddr_nl *target_nl = (struct sockaddr_nl *)target_saddr;
+    if (addr->sa_family == AF_NETLINK &&
+        len >= sizeof(struct target_sockaddr_nl)) {
+        struct target_sockaddr_nl *target_nl =
+               (struct target_sockaddr_nl *)target_saddr;
         target_nl->nl_pid = tswap32(target_nl->nl_pid);
         target_nl->nl_groups = tswap32(target_nl->nl_groups);
     } else if (addr->sa_family == AF_PACKET) {
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index fa69c6ab8d..7694d72446 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -153,6 +153,13 @@ struct target_sockaddr_un {
     uint8_t sun_path[108];
 };
 
+struct target_sockaddr_nl {
+    abi_ushort nl_family;   /* AF_NETLINK */
+    abi_ushort __pad;
+    abi_uint nl_pid;
+    abi_uint nl_groups;
+};
+
 struct target_in_addr {
     uint32_t s_addr; /* big endian */
 };
-- 
2.20.1

[Qemu-devel] [PATCH v7 6/9] linux-user/strace: Add print_sockfd()

[Philippe Mathieu-Daudé]

Extract common print_sockfd() from various socket related syscalls.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v6: use another ifdef TARGET_NR_socketcall
---
 linux-user/strace.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 22dd453d26..01c802c4e3 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1709,6 +1709,15 @@ print_socket(const struct syscallname *name,
 
 #if defined(TARGET_NR_socketcall)
 
+static void print_sockfd(abi_long sockfd, int last)
+{
+    print_raw_param(TARGET_ABI_FMT_ld, sockfd, last);
+}
+
+#endif
+
+#if defined(TARGET_NR_socketcall)
+
 #define get_user_ualx(x, gaddr, idx) \
         get_user_ual(x, (gaddr) + (idx) * sizeof(abi_long))
 
@@ -1741,7 +1750,7 @@ static void do_print_sockaddr(const char *name, abi_long arg1)
     get_user_ualx(addrlen, arg1, 2);
 
     gemu_log("%s(", name);
-    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
+    print_sockfd(sockfd, 0);
     print_sockaddr(addr, addrlen);
     gemu_log(")");
 }
@@ -1754,7 +1763,7 @@ static void do_print_listen(const char *name, abi_long arg1)
     get_user_ualx(backlog, arg1, 1);
 
     gemu_log("%s(", name);
-    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
+    print_sockfd(sockfd, 0);
     print_raw_param(TARGET_ABI_FMT_ld, backlog, 1);
     gemu_log(")");
 }
@@ -1789,7 +1798,7 @@ static void do_print_sendrecv(const char *name, abi_long arg1)
     get_user_ualx(flags, arg1, 3);
 
     gemu_log("%s(", name);
-    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
+    print_sockfd(sockfd, 0);
     print_buf(msg, len, 0);
     print_raw_param(TARGET_ABI_FMT_ld, len, 0);
     print_flags(msg_flags, flags, 1);
@@ -1808,7 +1817,7 @@ static void do_print_msgaddr(const char *name, abi_long arg1)
     get_user_ualx(addrlen, arg1, 5);
 
     gemu_log("%s(", name);
-    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
+    print_sockfd(sockfd, 0);
     print_buf(msg, len, 0);
     print_raw_param(TARGET_ABI_FMT_ld, len, 0);
     print_flags(msg_flags, flags, 0);
@@ -1824,7 +1833,7 @@ static void do_print_shutdown(const char *name, abi_long arg1)
     get_user_ualx(how, arg1, 1);
 
     gemu_log("shutdown(");
-    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
+    print_sockfd(sockfd, 0);
     switch (how) {
     case SHUT_RD:
         gemu_log("SHUT_RD");
@@ -1851,7 +1860,7 @@ static void do_print_msg(const char *name, abi_long arg1)
     get_user_ualx(flags, arg1, 2);
 
     gemu_log("%s(", name);
-    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
+    print_sockfd(sockfd, 0);
     print_pointer(msg, 0);
     print_flags(msg_flags, flags, 1);
     gemu_log(")");
@@ -1868,7 +1877,7 @@ static void do_print_sockopt(const char *name, abi_long arg1)
     get_user_ualx(optlen, arg1, 4);
 
     gemu_log("%s(", name);
-    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
+    print_sockfd(sockfd, 0);
     switch (level) {
     case SOL_TCP:
         gemu_log("SOL_TCP,");
-- 
2.20.1

[Qemu-devel] [PATCH v7 8/9] linux-user/strace: Let print_sockaddr() have a 'last' argument

[Philippe Mathieu-Daudé]

If the format is not the syscall last argument, a comma is append.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
checkpatch error:
ERROR: storage class should be at the beginning of the declaration
---
 linux-user/strace.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 6e82f6197a..3ccefb9839 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -67,7 +67,7 @@ UNUSED static void print_timeval(abi_ulong, int);
 UNUSED static void print_timezone(abi_ulong, int);
 UNUSED static void print_number(abi_long, int);
 UNUSED static void print_signal(abi_ulong, int);
-UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen);
+UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen, int);
 UNUSED static void print_socket_domain(int domain);
 UNUSED static void print_socket_type(int type);
 UNUSED static void print_socket_protocol(int domain, int type, int protocol);
@@ -336,7 +336,7 @@ static void print_siginfo(const target_siginfo_t *tinfo)
 }
 
 static void
-print_sockaddr(abi_ulong addr, abi_long addrlen)
+print_sockaddr(abi_ulong addr, abi_long addrlen, int last)
 {
     struct target_sockaddr *sa;
     int i;
@@ -418,7 +418,7 @@ print_sockaddr(abi_ulong addr, abi_long addrlen)
     } else {
         print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 0);
     }
-    gemu_log(", "TARGET_ABI_FMT_ld, addrlen);
+    gemu_log(", "TARGET_ABI_FMT_ld"%s", addrlen, get_comma(last));
 }
 
 static void
@@ -1751,7 +1751,7 @@ static void do_print_sockaddr(const char *name, abi_long arg1)
 
     gemu_log("%s(", name);
     print_sockfd(sockfd, 0);
-    print_sockaddr(addr, addrlen);
+    print_sockaddr(addr, addrlen, 0);
     gemu_log(")");
 }
 
@@ -1821,7 +1821,7 @@ static void do_print_msgaddr(const char *name, abi_long arg1)
     print_buf(msg, len, 0);
     print_raw_param(TARGET_ABI_FMT_ld, len, 0);
     print_flags(msg_flags, flags, 0);
-    print_sockaddr(addr, addrlen);
+    print_sockaddr(addr, addrlen, 0);
     gemu_log(")");
 }
 
-- 
2.20.1

[Qemu-devel] [RFC PATCH v7 9/9] linux-user/syscall: Align target_sockaddr fields using ABI types

[Philippe Mathieu-Daudé]

Target architectures align types differently for instance
m68k aligns on 16bit whereas others on 32bit).
Use ABI types to keep alignments good.

Suggested-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC: Is target_sockaddr_ll.sll_ifindex of type abi_int?

 linux-user/syscall_defs.h | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 7694d72446..852d4498e0 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -134,22 +134,22 @@
 #define TARGET_IOWRU(type,nr)	TARGET_IOC(TARGET_IOC_READ|TARGET_IOC_WRITE,(type),(nr),TARGET_IOC_SIZEMASK)
 
 struct target_sockaddr {
-    uint16_t sa_family;
+    abi_ushort sa_family;
     uint8_t sa_data[14];
 };
 
 struct target_sockaddr_ll {
-    uint16_t sll_family;   /* Always AF_PACKET */
-    uint16_t sll_protocol; /* Physical layer protocol */
-    int      sll_ifindex;  /* Interface number */
-    uint16_t sll_hatype;   /* ARP hardware type */
-    uint8_t  sll_pkttype;  /* Packet type */
-    uint8_t  sll_halen;    /* Length of address */
-    uint8_t  sll_addr[8];  /* Physical layer address */
+    abi_ushort sll_family;   /* Always AF_PACKET */
+    abi_ushort sll_protocol; /* Physical layer protocol */
+    int        sll_ifindex;  /* Interface number */
+    abi_ushort sll_hatype;   /* ARP hardware type */
+    uint8_t    sll_pkttype;  /* Packet type */
+    uint8_t    sll_halen;    /* Length of address */
+    uint8_t    sll_addr[8];  /* Physical layer address */
 };
 
 struct target_sockaddr_un {
-    uint16_t su_family;
+    abi_ushort su_family;
     uint8_t sun_path[108];
 };
 
@@ -161,24 +161,24 @@ struct target_sockaddr_nl {
 };
 
 struct target_in_addr {
-    uint32_t s_addr; /* big endian */
+    abi_uint s_addr; /* big endian */
 };
 
 struct target_sockaddr_in {
-  uint16_t sin_family;
-  int16_t sin_port; /* big endian */
+  abi_ushort sin_family;
+  abi_short sin_port; /* big endian */
   struct target_in_addr sin_addr;
   uint8_t __pad[sizeof(struct target_sockaddr) -
-                sizeof(uint16_t) - sizeof(int16_t) -
+                sizeof(abi_ushort) - sizeof(abi_short) -
                 sizeof(struct target_in_addr)];
 };
 
 struct target_sockaddr_in6 {
-    uint16_t sin6_family;
-    uint16_t sin6_port; /* big endian */
-    uint32_t sin6_flowinfo; /* big endian */
+    abi_ushort sin6_family;
+    abi_ushort sin6_port; /* big endian */
+    abi_uint sin6_flowinfo; /* big endian */
     struct in6_addr sin6_addr; /* IPv6 address, big endian */
-    uint32_t sin6_scope_id;
+    abi_uint sin6_scope_id;
 };
 
 struct target_sock_filter {
-- 
2.20.1

Re: [PATCH v7 0/9] linux-user: strace improvements

[Philippe Mathieu-Daudé]

On 9/15/19 11:39 PM, Philippe Mathieu-Daudé wrote:
> Hi Laurent,
> 
> Few patches I'v been writting while trying to figure out this issue:
> http://lists.nongnu.org/archive/html/qemu-arm/2018-01/msg00514.html
> 
> As usual with linux-user files, this series will trigger some checkpatch
> benign warnings.
> 
> Regards,
> 
> Phil.
> 
> Since v6:
> - Use ABI types in sockaddr
> 
> Since v5:
> - dropped 'Verify recvfrom(addr)' since failing LTP testsuite (see [1])
> - also define print_sockfd() for bind() (patches #6 and #7)
> 
> Since v4:
> - rebased on master (no change)
> 
> Since v3:
> - addressed Laurent comments
> - added print_sockfd()
> - removed the print_sockaddr_ptr() patch, also the two
>   getsockname()/recvfrom() patches for after 3.0.
> 
> Since v2:
> - display invalid pointer in print_timeval() and print_timezone()
> - do not display gettimeofday() arguments
> 
> Since v1:
> - addressed Laurent comments
> - added 'last' argument to print_sockaddr()
> - reordered series, so patches already correct can get applied directly
> - dropped "linux-user/syscall: simplify recvfrom()" for now
> 
> v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg05855.html
> v2: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08216.html
> v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg00411.html
> v5: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02067.html
> v6: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg01346.html
> [1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02807.html

Ping?

Re: [PATCH v7 0/9] linux-user: strace improvements

[Philippe Mathieu-Daudé]

ping ping?

On 9/30/19 5:44 PM, Philippe Mathieu-Daudé wrote:
> On 9/15/19 11:39 PM, Philippe Mathieu-Daudé wrote:
>> Hi Laurent,
>>
>> Few patches I'v been writting while trying to figure out this issue:
>> http://lists.nongnu.org/archive/html/qemu-arm/2018-01/msg00514.html
>>
>> As usual with linux-user files, this series will trigger some checkpatch
>> benign warnings.
>>
>> Regards,
>>
>> Phil.
>>
>> Since v6:
>> - Use ABI types in sockaddr
>>
>> Since v5:
>> - dropped 'Verify recvfrom(addr)' since failing LTP testsuite (see [1])
>> - also define print_sockfd() for bind() (patches #6 and #7)
>>
>> Since v4:
>> - rebased on master (no change)
>>
>> Since v3:
>> - addressed Laurent comments
>> - added print_sockfd()
>> - removed the print_sockaddr_ptr() patch, also the two
>>    getsockname()/recvfrom() patches for after 3.0.
>>
>> Since v2:
>> - display invalid pointer in print_timeval() and print_timezone()
>> - do not display gettimeofday() arguments
>>
>> Since v1:
>> - addressed Laurent comments
>> - added 'last' argument to print_sockaddr()
>> - reordered series, so patches already correct can get applied directly
>> - dropped "linux-user/syscall: simplify recvfrom()" for now
>>
>> v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg05855.html
>> v2: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08216.html
>> v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg00411.html
>> v5: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02067.html
>> v6: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg01346.html
>> [1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02807.html
> 
> Ping?

Re: [PATCH v7 1/9] linux-user/strace: Display invalid pointer in print_timeval()

[Laurent Vivier]

Le 15/09/2019 à 23:39, Philippe Mathieu-Daudé a écrit :
> Suggested-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  linux-user/strace.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index c80e93b5db..f326c357a2 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1243,8 +1243,10 @@ print_timeval(abi_ulong tv_addr, int last)
>          struct target_timeval *tv;
>  
>          tv = lock_user(VERIFY_READ, tv_addr, sizeof(*tv), 1);
> -        if (!tv)
> +        if (!tv) {
> +            print_pointer(tv_addr, last);
>              return;
> +        }
>          gemu_log("{" TARGET_ABI_FMT_ld "," TARGET_ABI_FMT_ld "}%s",
>              tswapal(tv->tv_sec), tswapal(tv->tv_usec), get_comma(last));
>          unlock_user(tv, tv_addr, 0);
> 

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

Re: [PATCH v7 2/9] linux-user/strace: Add print_timezone()

[Laurent Vivier]

Le 15/09/2019 à 23:39, Philippe Mathieu-Daudé a écrit :
> Suggested-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> v7: tz_minuteswest and tz_dsttime are abi_int -> %d/tswap32 (lvivier)
> checkpatch error:
> ERROR: storage class should be at the beginning of the declaration
> ---
>  linux-user/strace.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index f326c357a2..2cd6687cd9 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -63,6 +63,7 @@ UNUSED static void print_string(abi_long, int);
>  UNUSED static void print_buf(abi_long addr, abi_long len, int last);
>  UNUSED static void print_raw_param(const char *, abi_long, int);
>  UNUSED static void print_timeval(abi_ulong, int);
> +UNUSED static void print_timezone(abi_ulong, int);
>  UNUSED static void print_number(abi_long, int);
>  UNUSED static void print_signal(abi_ulong, int);
>  UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen);
> @@ -1254,6 +1255,25 @@ print_timeval(abi_ulong tv_addr, int last)
>          gemu_log("NULL%s", get_comma(last));
>  }
>  
> +static void
> +print_timezone(abi_ulong tz_addr, int last)
> +{
> +    if (tz_addr) {
> +        struct target_timezone *tz;
> +
> +        tz = lock_user(VERIFY_READ, tz_addr, sizeof(*tz), 1);
> +        if (!tz) {
> +            print_pointer(tz_addr, last);
> +            return;
> +        }
> +        gemu_log("{%d,%d}%s", tswap32(tz->tz_minuteswest),
> +                 tswap32(tz->tz_dsttime), get_comma(last));
> +        unlock_user(tz, tz_addr, 0);
> +    } else {
> +        gemu_log("NULL%s", get_comma(last));
> +    }
> +}
> +
>  #undef UNUSED
>  
>  #ifdef TARGET_NR_accept
> 

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

Re: [PATCH v7 4/9] linux-user/syscall: Introduce target_sockaddr_nl

[Laurent Vivier]

Le 15/09/2019 à 23:39, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
> v7: use abi_ushort and abi_uint to keep alignments good (lvivier)
> ---
>  linux-user/syscall.c      | 6 ++++--
>  linux-user/syscall_defs.h | 7 +++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e2af3c1494..f1ab81b917 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1496,8 +1496,10 @@ static inline abi_long host_to_target_sockaddr(abi_ulong target_addr,
>          sizeof(target_saddr->sa_family)) {
>          target_saddr->sa_family = tswap16(addr->sa_family);
>      }
> -    if (addr->sa_family == AF_NETLINK && len >= sizeof(struct sockaddr_nl)) {
> -        struct sockaddr_nl *target_nl = (struct sockaddr_nl *)target_saddr;
> +    if (addr->sa_family == AF_NETLINK &&
> +        len >= sizeof(struct target_sockaddr_nl)) {
> +        struct target_sockaddr_nl *target_nl =
> +               (struct target_sockaddr_nl *)target_saddr;
>          target_nl->nl_pid = tswap32(target_nl->nl_pid);
>          target_nl->nl_groups = tswap32(target_nl->nl_groups);
>      } else if (addr->sa_family == AF_PACKET) {
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index fa69c6ab8d..7694d72446 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -153,6 +153,13 @@ struct target_sockaddr_un {
>      uint8_t sun_path[108];
>  };
>  
> +struct target_sockaddr_nl {
> +    abi_ushort nl_family;   /* AF_NETLINK */
> +    abi_ushort __pad;
> +    abi_uint nl_pid;
> +    abi_uint nl_groups;
> +};
> +
>  struct target_in_addr {
>      uint32_t s_addr; /* big endian */
>  };
> 

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

Re: [PATCH v7 5/9] linux-user/strace: Dump AF_NETLINK sockaddr content

[Laurent Vivier]

Le 15/09/2019 à 23:39, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
>  linux-user/strace.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index fd5596a640..22dd453d26 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -8,6 +8,7 @@
>  #include <arpa/inet.h>
>  #include <netinet/tcp.h>
>  #include <linux/if_packet.h>
> +#include <linux/netlink.h>
>  #include <sched.h>
>  #include "qemu.h"
>  
> @@ -398,6 +399,12 @@ print_sockaddr(abi_ulong addr, abi_long addrlen)
>              gemu_log("}");
>              break;
>          }
> +        case AF_NETLINK: {
> +            struct target_sockaddr_nl *nl = (struct target_sockaddr_nl *)sa;
> +            gemu_log("{nl_family=AF_NETLINK,nl_pid=%u,nl_groups=%u}",
> +                     nl->nl_pid, nl->nl_groups);

You need to use tswap32() here.

Thanks,
Laurent

Re: [PATCH v7 6/9] linux-user/strace: Add print_sockfd()

[Laurent Vivier]

Le 15/09/2019 à 23:39, Philippe Mathieu-Daudé a écrit :
> Extract common print_sockfd() from various socket related syscalls.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> v6: use another ifdef TARGET_NR_socketcall
> ---
>  linux-user/strace.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 22dd453d26..01c802c4e3 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1709,6 +1709,15 @@ print_socket(const struct syscallname *name,
>  
>  #if defined(TARGET_NR_socketcall)
>  
> +static void print_sockfd(abi_long sockfd, int last)
> +{
> +    print_raw_param(TARGET_ABI_FMT_ld, sockfd, last);
> +}
> +
> +#endif
> +
> +#if defined(TARGET_NR_socketcall)
> +
>  #define get_user_ualx(x, gaddr, idx) \
>          get_user_ual(x, (gaddr) + (idx) * sizeof(abi_long))
>  
> @@ -1741,7 +1750,7 @@ static void do_print_sockaddr(const char *name, abi_long arg1)
>      get_user_ualx(addrlen, arg1, 2);
>  
>      gemu_log("%s(", name);
> -    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
> +    print_sockfd(sockfd, 0);
>      print_sockaddr(addr, addrlen);
>      gemu_log(")");
>  }
> @@ -1754,7 +1763,7 @@ static void do_print_listen(const char *name, abi_long arg1)
>      get_user_ualx(backlog, arg1, 1);
>  
>      gemu_log("%s(", name);
> -    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
> +    print_sockfd(sockfd, 0);
>      print_raw_param(TARGET_ABI_FMT_ld, backlog, 1);
>      gemu_log(")");
>  }
> @@ -1789,7 +1798,7 @@ static void do_print_sendrecv(const char *name, abi_long arg1)
>      get_user_ualx(flags, arg1, 3);
>  
>      gemu_log("%s(", name);
> -    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
> +    print_sockfd(sockfd, 0);
>      print_buf(msg, len, 0);
>      print_raw_param(TARGET_ABI_FMT_ld, len, 0);
>      print_flags(msg_flags, flags, 1);
> @@ -1808,7 +1817,7 @@ static void do_print_msgaddr(const char *name, abi_long arg1)
>      get_user_ualx(addrlen, arg1, 5);
>  
>      gemu_log("%s(", name);
> -    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
> +    print_sockfd(sockfd, 0);
>      print_buf(msg, len, 0);
>      print_raw_param(TARGET_ABI_FMT_ld, len, 0);
>      print_flags(msg_flags, flags, 0);
> @@ -1824,7 +1833,7 @@ static void do_print_shutdown(const char *name, abi_long arg1)
>      get_user_ualx(how, arg1, 1);
>  
>      gemu_log("shutdown(");
> -    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
> +    print_sockfd(sockfd, 0);
>      switch (how) {
>      case SHUT_RD:
>          gemu_log("SHUT_RD");
> @@ -1851,7 +1860,7 @@ static void do_print_msg(const char *name, abi_long arg1)
>      get_user_ualx(flags, arg1, 2);
>  
>      gemu_log("%s(", name);
> -    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
> +    print_sockfd(sockfd, 0);
>      print_pointer(msg, 0);
>      print_flags(msg_flags, flags, 1);
>      gemu_log(")");
> @@ -1868,7 +1877,7 @@ static void do_print_sockopt(const char *name, abi_long arg1)
>      get_user_ualx(optlen, arg1, 4);
>  
>      gemu_log("%s(", name);
> -    print_raw_param(TARGET_ABI_FMT_ld, sockfd, 0);
> +    print_sockfd(sockfd, 0);
>      switch (level) {
>      case SOL_TCP:
>          gemu_log("SOL_TCP,");
> 

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

Re: [PATCH v7 7/9] linux-user/strace: Improve bind() output

[Laurent Vivier]

Le 15/09/2019 à 23:39, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> ---
> v6: use TARGET_NR_socketcall || TARGET_NR_bind (lvivier)
> ---
>  linux-user/strace.c    | 15 ++++++++++++++-
>  linux-user/strace.list |  2 +-
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 01c802c4e3..6e82f6197a 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1707,7 +1707,7 @@ print_socket(const struct syscallname *name,
>  
>  #endif
>  
> -#if defined(TARGET_NR_socketcall)
> +#if defined(TARGET_NR_socketcall) || defined(TARGET_NR_bind)
>  
>  static void print_sockfd(abi_long sockfd, int last)
>  {
> @@ -2054,6 +2054,19 @@ print_socketcall(const struct syscallname *name,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_bind)
> +static void
> +print_bind(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_sockfd(arg0, 0);
> +    print_sockaddr(arg1, arg2, 1);
> +    print_syscall_epilogue(name);
> +}
> +#endif
> +
>  #if defined(TARGET_NR_stat) || defined(TARGET_NR_stat64) || \
>      defined(TARGET_NR_lstat) || defined(TARGET_NR_lstat64)
>  static void
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index 1ff9168369..957aa720af 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -41,7 +41,7 @@
>  { TARGET_NR_bdflush, "bdflush" , NULL, NULL, NULL },
>  #endif
>  #ifdef TARGET_NR_bind
> -{ TARGET_NR_bind, "bind" , NULL, NULL, NULL },
> +{ TARGET_NR_bind, "bind" , NULL, print_bind, NULL },
>  #endif
>  #ifdef TARGET_NR_bpf
>  { TARGET_NR_bpf, "bpf" , NULL, NULL, NULL },
> 

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

Re: [PATCH v7 8/9] linux-user/strace: Let print_sockaddr() have a 'last' argument

[Laurent Vivier]

Le 15/09/2019 à 23:39, Philippe Mathieu-Daudé a écrit :
> If the format is not the syscall last argument, a comma is append.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> checkpatch error:
> ERROR: storage class should be at the beginning of the declaration
> ---
>  linux-user/strace.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 6e82f6197a..3ccefb9839 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -67,7 +67,7 @@ UNUSED static void print_timeval(abi_ulong, int);
>  UNUSED static void print_timezone(abi_ulong, int);
>  UNUSED static void print_number(abi_long, int);
>  UNUSED static void print_signal(abi_ulong, int);
> -UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen);
> +UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen, int);

You mix argument declaration with a name and without a name, make choice.

Thanks,
Laurent

Re: [RFC PATCH v7 9/9] linux-user/syscall: Align target_sockaddr fields using ABI types

[Laurent Vivier]

Le 15/09/2019 à 23:39, Philippe Mathieu-Daudé a écrit :
> Target architectures align types differently for instance
> m68k aligns on 16bit whereas others on 32bit).
> Use ABI types to keep alignments good.
> 
> Suggested-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> RFC: Is target_sockaddr_ll.sll_ifindex of type abi_int?
> 
>  linux-user/syscall_defs.h | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 7694d72446..852d4498e0 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -134,22 +134,22 @@
>  #define TARGET_IOWRU(type,nr)	TARGET_IOC(TARGET_IOC_READ|TARGET_IOC_WRITE,(type),(nr),TARGET_IOC_SIZEMASK)
>  
>  struct target_sockaddr {
> -    uint16_t sa_family;
> +    abi_ushort sa_family;
>      uint8_t sa_data[14];
>  };
>  
>  struct target_sockaddr_ll {
> -    uint16_t sll_family;   /* Always AF_PACKET */
> -    uint16_t sll_protocol; /* Physical layer protocol */
> -    int      sll_ifindex;  /* Interface number */
> -    uint16_t sll_hatype;   /* ARP hardware type */
> -    uint8_t  sll_pkttype;  /* Packet type */
> -    uint8_t  sll_halen;    /* Length of address */
> -    uint8_t  sll_addr[8];  /* Physical layer address */
> +    abi_ushort sll_family;   /* Always AF_PACKET */
> +    abi_ushort sll_protocol; /* Physical layer protocol */
> +    int        sll_ifindex;  /* Interface number */

abi_int would be better.

Thanks,
Laurent