signal.c: Change printsignal to properly handle xlat styles

[Dmitry V. Levin]

Hi Shankara,

On Thu, Dec 20, 2018 at 05:32:02AM -0800, shankarapailoor wrote:
> Hi!
> 
> I noticed that strace doesn't properly decode the signum of sigaction
> system calls under different xlat styles. Attached is a patch which
> corrects this issue. I will follow this up with another patch to
> change the decoding of sa_mask field to properly handle xlat styles.
> Let me know what you think.
> 
> Regards,
> Shankara Pailoor

> From 5d1d442be8765e12b955ffc3634c1a9e97849958 Mon Sep 17 00:00:00 2001
> From: Shankara Pailoor <shankarapailoor at gmail.com>
> Date: Thu, 20 Dec 2018 05:27:40 -0800
> Subject: [PATCH v1] signal.c: Change printsignal to properly handle xlat styles
> 	* tests/printsignal.c: New test to make sure signum is printed
>  properly under different xlat styles 
> 	* tests/printsignal-Xabbrev.c: -Xabbrev case 
> 	* tests/printsignal-Xraw.c: Likewise 
> 	* tests/printsignal-Xverbose.c: Likewise 
> 	* tests/pure_executables.list: Add new tests 
> 	* tests/.gitignore: Likewise

Please add an empty line after the first line of commit message,
otherwise it becomes a single line of enormous length.

> ---
>  signal.c                     | 12 +++++++++
>  tests/.gitignore             |  3 +++
>  tests/gen_tests.in           |  3 +++
>  tests/printsignal-Xabbrev.c  |  1 +
>  tests/printsignal-Xraw.c     |  2 ++
>  tests/printsignal-Xverbose.c |  2 ++
>  tests/printsignal.c          | 48 ++++++++++++++++++++++++++++++++++++
>  tests/pure_executables.list  |  3 +++
>  8 files changed, 74 insertions(+)
>  create mode 100644 tests/printsignal-Xabbrev.c
>  create mode 100644 tests/printsignal-Xraw.c
>  create mode 100644 tests/printsignal-Xverbose.c
>  create mode 100644 tests/printsignal.c
> 
> diff --git a/signal.c b/signal.c
> index 17a3f79c..25002e60 100644
> --- a/signal.c
> +++ b/signal.c
> @@ -228,7 +228,19 @@ sprint_old_sigmask_val(const char *const prefix, const unsigned long mask)
>  void
>  printsignal(int nr)
>  {
> +	if (xlat_verbose(xlat_verbosity) != XLAT_STYLE_ABBREV)
> +		tprintf("%d", nr);
> +
> +	if (xlat_verbose(xlat_verbosity) == XLAT_STYLE_RAW)
> +		return;
> +
> +	if (xlat_verbose(xlat_verbosity) == XLAT_STYLE_VERBOSE)
> +		tprints(" /* ");
> +
>  	tprints(signame(nr));
> +
> +	if (xlat_verbose(xlat_verbosity) == XLAT_STYLE_VERBOSE)
> +		tprints(" */");

Eugene in his branch has the following expressive statement for this case:

	(xlat_verbose(xlat_verbosity) == XLAT_STYLE_VERBOSE
		? tprints_comment : tprints)(signame(nr));

> +#include <asm/unistd.h>
> +
> +#ifdef __NR_rt_sigaction

I think you can safely assume that __NR_rt_sigaction is universally
available.

> +#include <assert.h>

I don't see any use of assert here.

> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <signal.h>
> +#include <stdio.h>
> +
> +static long
> +k_sigaction(int signum, const struct sigaction *act,
> +                     struct sigaction *oldact, size_t sigsetsize)
> +{
> +	return syscall(__NR_rt_sigaction, signum, act, oldact, sigsetsize);
> +}
> +
> +int
> +main(void)
> +{
> +	k_sigaction(SIGUSR2, NULL, NULL, sizeof(sigset_t));
> +#if XLAT_RAW
> +	printf("rt_sigaction(12, NULL, NULL, %lu)"
> +		" = -1 EINVAL (Invalid argument)\n", sizeof(sigset_t));
> +#elif XLAT_VERBOSE
> +	printf("rt_sigaction(12 /* SIGUSR2 */, NULL, NULL, %lu)"
> +		" = -1 EINVAL (Invalid argument)\n", sizeof(sigset_t));
> +#else /* XLAT_ABBREV */
> +	printf("rt_sigaction(SIGUSR2, NULL, NULL, %lu) ="
> +		" -1 EINVAL (Invalid argument)\n", sizeof(sigset_t));
> +#endif

The value of SIGUSR2 differs across architectures, see linux/signalent.h
and linux/*/signalent.h files.

The only reason why this syscall fails with EINVAL is the size of sigset_t
which in this case depends on libc.

As in this test it doesn't matter whether the syscall succeeded or failed,
you can use "errstr = sprintrc(rc)" idiom instead.

btw, have you tried to invoke k_sigaction with an invalid signal number,
e.g. -1?  Does the output look good in verbose mode?


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20181222/95135014/attachment.bin>