| Message ID | 20240807124306.52903-6-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: Trace sendto/recvfrom | expand |
On Wed, 2024-08-07 at 14:43 +0200, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > linux-user/strace.c | 19 +++++++++++++++++++ > linux-user/strace.list | 2 +- > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/linux-user/strace.c b/linux-user/strace.c > index 98ef26b917..d76907fdc9 100644 > --- a/linux-user/strace.c > +++ b/linux-user/strace.c > @@ -3127,6 +3127,25 @@ print_bind(CPUArchState *cpu_env, const struct > syscallname *name, > } > #endif > > +#ifdef TARGET_NR_recvfrom > +static void > +print_recvfrom(CPUArchState *cpu_env, const struct syscallname > *name, > + abi_long arg0, abi_long arg1, abi_long arg2, > + abi_long arg3, abi_long arg4, abi_long arg5) > +{ > + abi_ulong addrlen; > + > + get_user_ualx(addrlen, arg5, 0); > + > + print_syscall_prologue(name); > + print_sockfd(arg0, 0); > + print_buf_len(arg1, arg2, 0); > + print_flags(msg_flags, arg3, 0); > + print_sockaddr(arg4, addrlen, 1); > + print_syscall_epilogue(name); > +} > +#endif > + > #ifdef TARGET_NR_sendto > static void > print_sendto(CPUArchState *cpu_env, const struct syscallname *name, > diff --git a/linux-user/strace.list b/linux-user/strace.list > index 5a86419e7d..77ca824f9c 100644 > --- a/linux-user/strace.list > +++ b/linux-user/strace.list > @@ -1135,7 +1135,7 @@ > { TARGET_NR_recv, "recv" , "%s(%d,%p,%u,%d)", NULL, NULL }, > #endif > #ifdef TARGET_NR_recvfrom > -{ TARGET_NR_recvfrom, "recvfrom" , NULL, NULL, NULL }, > +{ TARGET_NR_recvfrom, "recvfrom" , NULL, print_recvfrom, NULL }, > #endif > #ifdef TARGET_NR_recvmmsg > { TARGET_NR_recvmmsg, "recvmmsg" , NULL, NULL, NULL }, I needed to implement read()/write() tracing and stumbled upon this series, which overall looks like a good thing to have. I spotted a few issues though. I get the following build error: qemu/linux-user/strace.c:3138:5: error: implicit declaration of function ‘get_user_ualx’; did you mean ‘get_user_ual’? [-Wimplicit- function-declaration] 3138 | get_user_ualx(addrlen, arg5, 0); | ^~~~~~~~~~~~~ | get_user_ual The following helps: --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -2666,11 +2666,15 @@ static void print_sockfd(abi_long sockfd, int last) #endif -#if defined(TARGET_NR_socketcall) +#if defined(TARGET_NR_socketcall) || defined(TARGET_NR_recvfrom) #define get_user_ualx(x, gaddr, idx) \ get_user_ual(x, (gaddr) + (idx) * sizeof(abi_long)) +#endif + +#if defined(TARGET_NR_socketcall) + static void do_print_socket(const char *name, abi_long arg1) { abi_ulong domain, type, protocol; With this I get the following output, which w.r.t. its shape looks good: 598730 recvfrom(3,"HTTP/1.1 400 Bad request\15\12Connection: Cl"...,8192,0,{sa_family=0, sa_data={00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00}},128) = 0 However: aren't we printing the contents of the buffer before the syscall returns? Same with sockaddr. This would make the output not very useful.
On 10/1/24 23:55, Ilya Leoshkevich wrote: > On Wed, 2024-08-07 at 14:43 +0200, Philippe Mathieu-Daudé wrote: >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> linux-user/strace.c | 19 +++++++++++++++++++ >> linux-user/strace.list | 2 +- >> 2 files changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/linux-user/strace.c b/linux-user/strace.c >> index 98ef26b917..d76907fdc9 100644 >> --- a/linux-user/strace.c >> +++ b/linux-user/strace.c >> @@ -3127,6 +3127,25 @@ print_bind(CPUArchState *cpu_env, const struct >> syscallname *name, >> } >> #endif >> >> +#ifdef TARGET_NR_recvfrom >> +static void >> +print_recvfrom(CPUArchState *cpu_env, const struct syscallname >> *name, >> + abi_long arg0, abi_long arg1, abi_long arg2, >> + abi_long arg3, abi_long arg4, abi_long arg5) >> +{ >> + abi_ulong addrlen; >> + >> + get_user_ualx(addrlen, arg5, 0); >> + >> + print_syscall_prologue(name); >> + print_sockfd(arg0, 0); >> + print_buf_len(arg1, arg2, 0); >> + print_flags(msg_flags, arg3, 0); >> + print_sockaddr(arg4, addrlen, 1); >> + print_syscall_epilogue(name); >> +} >> +#endif >> + >> #ifdef TARGET_NR_sendto >> static void >> print_sendto(CPUArchState *cpu_env, const struct syscallname *name, >> diff --git a/linux-user/strace.list b/linux-user/strace.list >> index 5a86419e7d..77ca824f9c 100644 >> --- a/linux-user/strace.list >> +++ b/linux-user/strace.list >> @@ -1135,7 +1135,7 @@ >> { TARGET_NR_recv, "recv" , "%s(%d,%p,%u,%d)", NULL, NULL }, >> #endif >> #ifdef TARGET_NR_recvfrom >> -{ TARGET_NR_recvfrom, "recvfrom" , NULL, NULL, NULL }, >> +{ TARGET_NR_recvfrom, "recvfrom" , NULL, print_recvfrom, NULL }, >> #endif >> #ifdef TARGET_NR_recvmmsg >> { TARGET_NR_recvmmsg, "recvmmsg" , NULL, NULL, NULL }, > > I needed to implement read()/write() tracing and stumbled upon this > series, which overall looks like a good thing to have. I spotted a few > issues though. > > > I get the following build error: > > qemu/linux-user/strace.c:3138:5: error: implicit declaration of > function ‘get_user_ualx’; did you mean ‘get_user_ual’? [-Wimplicit- > function-declaration] > 3138 | get_user_ualx(addrlen, arg5, 0); > | ^~~~~~~~~~~~~ > | get_user_ual > > > The following helps: > > --- a/linux-user/strace.c > +++ b/linux-user/strace.c > @@ -2666,11 +2666,15 @@ static void print_sockfd(abi_long sockfd, int > last) > > #endif > > -#if defined(TARGET_NR_socketcall) > +#if defined(TARGET_NR_socketcall) || defined(TARGET_NR_recvfrom) > > #define get_user_ualx(x, gaddr, idx) \ > get_user_ual(x, (gaddr) + (idx) * sizeof(abi_long)) > > +#endif > + > +#if defined(TARGET_NR_socketcall) Or just not use get_user_ualx. > However: aren't we printing the contents of the buffer before the > syscall returns? Same with sockaddr. This would make the output not > very useful. Yes indeed. Two of the three pointers are output buffers; the last is in/out. r~
diff --git a/linux-user/strace.c b/linux-user/strace.c index 98ef26b917..d76907fdc9 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -3127,6 +3127,25 @@ print_bind(CPUArchState *cpu_env, const struct syscallname *name, } #endif +#ifdef TARGET_NR_recvfrom +static void +print_recvfrom(CPUArchState *cpu_env, const struct syscallname *name, + abi_long arg0, abi_long arg1, abi_long arg2, + abi_long arg3, abi_long arg4, abi_long arg5) +{ + abi_ulong addrlen; + + get_user_ualx(addrlen, arg5, 0); + + print_syscall_prologue(name); + print_sockfd(arg0, 0); + print_buf_len(arg1, arg2, 0); + print_flags(msg_flags, arg3, 0); + print_sockaddr(arg4, addrlen, 1); + print_syscall_epilogue(name); +} +#endif + #ifdef TARGET_NR_sendto static void print_sendto(CPUArchState *cpu_env, const struct syscallname *name, diff --git a/linux-user/strace.list b/linux-user/strace.list index 5a86419e7d..77ca824f9c 100644 --- a/linux-user/strace.list +++ b/linux-user/strace.list @@ -1135,7 +1135,7 @@ { TARGET_NR_recv, "recv" , "%s(%d,%p,%u,%d)", NULL, NULL }, #endif #ifdef TARGET_NR_recvfrom -{ TARGET_NR_recvfrom, "recvfrom" , NULL, NULL, NULL }, +{ TARGET_NR_recvfrom, "recvfrom" , NULL, print_recvfrom, NULL }, #endif #ifdef TARGET_NR_recvmmsg { TARGET_NR_recvmmsg, "recvmmsg" , NULL, NULL, NULL },
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- linux-user/strace.c | 19 +++++++++++++++++++ linux-user/strace.list | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-)