Bug 1993827
| Summary: | vgdb terminated with oom-kill in s390x | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 9 | Reporter: | Jesus Checa <jchecahi> |
| Component: | valgrind | Assignee: | Mark Wielaard <mjw> |
| valgrind sub component: | system-version | QA Contact: | Jesus Checa <jchecahi> |
| Status: | CLOSED UPSTREAM | Docs Contact: | |
| Severity: | unspecified | ||
| Priority: | unspecified | CC: | fche, fweimer, jakub, ohudlick |
| Version: | 9.0 | Keywords: | Bugfix, Triaged |
| Target Milestone: | beta | Flags: | pm-rhel:
mirror+
|
| Target Release: | --- | ||
| Hardware: | s390x | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2021-10-28 11:32:05 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Jesus Checa
2021-08-16 08:09:32 UTC
It seems we are stuck in the while (1) loop in this function:
/* Wait till the process pid is reported as stopped with signal_expected.
If other signal(s) than signal_expected are received, waitstopped
will pass them to pid, waiting for signal_expected to stop pid.
Returns True when process is in stopped state with signal_expected.
Returns False if a problem was encountered while waiting for pid
to be stopped.
If pid is reported as being dead/exited, waitstopped will return False.
*/
static
Bool waitstopped (pid_t pid, int signal_expected, const char *msg)
{
pid_t p;
int status = 0;
int signal_received;
int res;
while (1) {
DEBUG(1, "waitstopped %s before waitpid signal_expected %d\n",
msg, signal_expected);
p = waitpid(pid, &status, __WALL);
DEBUG(1, "after waitpid pid %d p %d status 0x%x %s\n", pid, p,
status, status_image (status));
if (p != pid) {
ERROR(errno, "%s waitpid pid %d in waitstopped %d status 0x%x %s\n",
msg, pid, p, status, status_image (status));
return False;
}
/* The process either exited or was terminated by a (fatal) signal. */
if (WIFEXITED(status) || WIFSIGNALED(status)) {
shutting_down = True;
return False;
}
assert (WIFSTOPPED(status));
signal_received = WSTOPSIG(status);
if (signal_received == signal_expected)
break;
/* pid received a signal which is not the signal we are waiting for.
If we have not (yet) changed the registers of the inferior
or we have (already) reset them, we can transmit the signal.
If we have already set the registers of the inferior, we cannot
transmit the signal, as this signal would arrive when the
gdbserver code runs. And valgrind only expects signals to
arrive in a small code portion around
client syscall logic, where signal are unmasked (see e.g.
m_syswrap/syscall-x86-linux.S ML_(do_syscall_for_client_WRK).
As ptrace is forcing a call to gdbserver by jumping
'out of this region', signals are not masked, but
will arrive outside of the allowed/expected code region.
So, if we have changed the registers of the inferior, we
rather queue the signal to transmit them when detaching,
after having restored the registers to the initial values. */
if (pid_of_save_regs) {
siginfo_t *newsiginfo;
// realloc a bigger queue, and store new signal at the end.
// This is not very efficient but we assume not many sigs are queued.
signal_queue_sz++;
signal_queue = vrealloc(signal_queue,
sizeof(siginfo_t) * signal_queue_sz);
newsiginfo = signal_queue + (signal_queue_sz - 1);
res = ptrace (PTRACE_GETSIGINFO, pid, NULL, newsiginfo);
if (res != 0) {
ERROR(errno, "PTRACE_GETSIGINFO failed: signal lost !!!!\n");
signal_queue_sz--;
} else
DEBUG(1, "waitstopped PTRACE_CONT, queuing signal %d"
" si_signo %d si_pid %d\n",
signal_received, newsiginfo->si_signo, newsiginfo->si_pid);
res = ptrace (PTRACE_CONT, pid, NULL, 0);
} else {
DEBUG(1, "waitstopped PTRACE_CONT with signal %d\n", signal_received);
res = ptrace (PTRACE_CONT, pid, NULL, signal_received);
}
if (res != 0) {
ERROR(errno, "waitstopped PTRACE_CONT\n");
return False;
}
}
return True;
}
The problem is that signal_received != signal_expected and pid_of_save_regs is non-zero.
We keep calling realloc. The comment says:
// realloc a bigger queue, and store new signal at the end.
// This is not very efficient but we assume not many sigs are queued.
But it seems we are getting SIGSEGV over and over again, while we are expecting SIGSTOP.
Possibly related: https://bugs.kde.org/show_bug.cgi?id=434035 The WIFSIGNALED(status) seems to have been introduced specifically to handle this issue, but it doesn't seem to work here. (In reply to Mark Wielaard from comment #2) > Possibly related: https://bugs.kde.org/show_bug.cgi?id=434035 > The WIFSIGNALED(status) seems to have been introduced specifically to handle > this issue, but it doesn't seem to work here. The reason this doesn't seem to work is because valgrind itself crashes and it tries to catch the SIGSEGV signal (so it isn't a fatal signal). I haven't tracked down why valgrind crashes at this point, but vgdb should back out if there are too many signals pending. So I'll propose at least this patch which makes the testcase fail without consuming all memory: diff --git a/coregrind/vgdb-invoker-ptrace.c b/coregrind/vgdb-invoker-ptrace.c index 389748960..07f3400f9 100644 --- a/coregrind/vgdb-invoker-ptrace.c +++ b/coregrind/vgdb-invoker-ptrace.c @@ -300,6 +300,10 @@ Bool waitstopped (pid_t pid, int signal_expected, const char *msg) // realloc a bigger queue, and store new signal at the end. // This is not very efficient but we assume not many sigs are queued. + if (signal_queue_sz >= 64) { + DEBUG(0, "too many queued signals while waiting for SIGSTOP\n"); + return False; + } signal_queue_sz++; signal_queue = vrealloc(signal_queue, sizeof(siginfo_t) * signal_queue_sz); valgrind 3.18.1 was released with a workaround for the oom issue (but no fix for the, unknown, underlying issue). The workaround for the out of memory has been included in valgrind 3.18.1. The actual crash under s390x is being tracked upstream: |