On 21.11.19 19:43, Aleksandar Markovic wrote: > On Wed, Nov 20, 2019 at 3:57 PM Helge Deller <del...@gmx.de> wrote: >> >> Add the textual representations of some missing target signals. >> >> Signed-off-by: Helge Deller <del...@gmx.de> >> >> diff --git a/linux-user/strace.c b/linux-user/strace.c >> index 3d4d684450..de43238fa4 100644 >> --- a/linux-user/strace.c >> +++ b/linux-user/strace.c >> @@ -146,6 +146,19 @@ print_signal(abi_ulong arg, int last) >> case TARGET_SIGSTOP: signal_name = "SIGSTOP"; break; >> case TARGET_SIGTTIN: signal_name = "SIGTTIN"; break; >> case TARGET_SIGTTOU: signal_name = "SIGTTOU"; break; >> + case TARGET_SIGIO: signal_name = "SIGIO"; break; >> + case TARGET_SIGBUS: signal_name = "SIGBUS"; break; >> + case TARGET_SIGPWR: signal_name = "SIGPWR"; break; >> + case TARGET_SIGURG: signal_name = "SIGURG"; break; >> + case TARGET_SIGSYS: signal_name = "SIGSYS"; break; >> + case TARGET_SIGTRAP: signal_name = "SIGTRAP"; break; >> + case TARGET_SIGXCPU: signal_name = "SIGXCPU"; break; >> + case TARGET_SIGPROF: signal_name = "SIGPROF"; break; >> + case TARGET_SIGTSTP: signal_name = "SIGTSTP"; break; >> + case TARGET_SIGXFSZ: signal_name = "SIGXFSZ"; break; >> + case TARGET_SIGWINCH: signal_name = "SIGWINCH"; break; >> + case TARGET_SIGVTALRM: signal_name = "SIGVTALRM"; break; >> + case TARGET_SIGSTKFLT: signal_name = "SIGSTKFLT"; break; > > What about TARGET_SIGEMT, TARGET_SIGIOT ? Those are missing from > MIPS-specific list of signals, and they won't be printed as strings. I > think you should add: > > #if defined TARGET_SIGEMT > case TARGET_SIGEMT: signal_name = "SIGEMT"; break; > #endif > case TARGET_SIGIOT: signal_name = "SIGIOT"; break;
Please see my first version of the patch. There I added the TARGET_SIGIOT as comment. I assume there is a build issue if you add a case for TARGET_SIGIOT, since it probably conflicts with TARGET_SIGABRT. That's probably why TARGET_SIGIOT is commented out in linux-user/signal.c as well? For SIGEMT I don't know. It's only defined for MIPS, and I can't test-compile it. I'm happy to add the #ifdef mentioned above if you could test it. > (I believe "#if defined"s is needed only for SIG_EMT, but doublecheck.) Yes, it's not defined for any other arch. > Without this, this patch favors other platforms over MIPS, which is > certainly not a good/fair thing. Everyone loves the own baby :-) > There might be some similar case or two for other platforms too > (alpha, sparc perhaps). I think I checked with the table in linux-user/signal.c before sending. > Your reference should be kernel files: > arch/<platform>/include/uapi/asm/sighal.h. > > In fact, there is some peace of kernell code that exactly deal with > the same problem - getting the names of the signals. It is in > security/apparmor/include/sig_names.h > > ( > https://elixir.bootlin.com/linux/v5.4-rc8/source/security/apparmor/include/sig_names.h > ) > > Since the file is short, I am inserting the whole content here: > > #include <linux/signal.h> > > #define SIGUNKNOWN 0 > #define MAXMAPPED_SIG 35 > #define MAXMAPPED_SIGNAME (MAXMAPPED_SIG + 1) > #define SIGRT_BASE 128 > > /* provide a mapping of arch signal to internal signal # for mediation > * those that are always an alias SIGCLD for SIGCLHD and SIGPOLL for SIGIO > * map to the same entry those that may/or may not get a separate entry > */ > static const int sig_map[MAXMAPPED_SIG] = { > [0] = MAXMAPPED_SIG, /* existence test */ > [SIGHUP] = 1, > [SIGINT] = 2, > [SIGQUIT] = 3, > [SIGILL] = 4, > [SIGTRAP] = 5, /* -, 5, - */ > [SIGABRT] = 6, /* SIGIOT: -, 6, - */ > [SIGBUS] = 7, /* 10, 7, 10 */ > [SIGFPE] = 8, > [SIGKILL] = 9, > [SIGUSR1] = 10, /* 30, 10, 16 */ > [SIGSEGV] = 11, > [SIGUSR2] = 12, /* 31, 12, 17 */ > [SIGPIPE] = 13, > [SIGALRM] = 14, > [SIGTERM] = 15, > #ifdef SIGSTKFLT > [SIGSTKFLT] = 16, /* -, 16, - */ > #endif > [SIGCHLD] = 17, /* 20, 17, 18. SIGCHLD -, -, 18 */ > [SIGCONT] = 18, /* 19, 18, 25 */ > [SIGSTOP] = 19, /* 17, 19, 23 */ > [SIGTSTP] = 20, /* 18, 20, 24 */ > [SIGTTIN] = 21, /* 21, 21, 26 */ > [SIGTTOU] = 22, /* 22, 22, 27 */ > [SIGURG] = 23, /* 16, 23, 21 */ > [SIGXCPU] = 24, /* 24, 24, 30 */ > [SIGXFSZ] = 25, /* 25, 25, 31 */ > [SIGVTALRM] = 26, /* 26, 26, 28 */ > [SIGPROF] = 27, /* 27, 27, 29 */ > [SIGWINCH] = 28, /* 28, 28, 20 */ > [SIGIO] = 29, /* SIGPOLL: 23, 29, 22 */ > [SIGPWR] = 30, /* 29, 30, 19. SIGINFO 29, -, - */ > #ifdef SIGSYS > [SIGSYS] = 31, /* 12, 31, 12. often SIG LOST/UNUSED */ > #endif > #ifdef SIGEMT > [SIGEMT] = 32, /* 7, - , 7 */ > #endif > #if defined(SIGLOST) && SIGPWR != SIGLOST /* sparc */ > [SIGLOST] = 33, /* unused on Linux */ > #endif > #if defined(SIGUNUSED) && \ > defined(SIGLOST) && defined(SIGSYS) && SIGLOST != SIGSYS > [SIGUNUSED] = 34, /* -, 31, - */ > #endif > }; > > /* this table is ordered post sig_map[sig] mapping */ > static const char *const sig_names[MAXMAPPED_SIGNAME] = { > "unknown", > "hup", > "int", > "quit", > "ill", > "trap", > "abrt", > "bus", > "fpe", > "kill", > "usr1", > "segv", > "usr2", > "pipe", > "alrm", > "term", > "stkflt", > "chld", > "cont", > "stop", > "stp", > "ttin", > "ttou", > "urg", > "xcpu", > "xfsz", > "vtalrm", > "prof", > "winch", > "io", > "pwr", > "sys", > "emt", > "lost", > "unused", > > "exists", /* always last existence test mapped to MAXMAPPED_SIG */ > }; > > I think you should mirror the functionality from that file. Simply mirroring files isn't good IMHO. The copy from bootlin above might not have all cases/fixes for all platforms (e.g. s390x, parisc, sparc, ...)? Helge
