Bug 434035 - vgdb might crash if valgrind is killed
Summary: vgdb might crash if valgrind is killed
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-03-06 00:38 UTC by Mark Wielaard
Modified: 2021-03-09 17:55 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2021-03-06 00:38:51 UTC
This is an odd corner case, but happens specifically with the gdb testcase make check TESTS=gdb.base/valgrind-infcall-2.exp. At the end valgrind gets killed with SIGKILL (-9) which cannot be blocked. But vgdb at the time is inside waitstopped. It sees the process wasn't exited (WIFEXITED(status) is false) and so assumes the process was stopped by a signal. Which it asserts:

      assert (WIFSTOPPED(status));
      signal_received = WSTOPSIG(status);
      if (signal_received == signal_expected)
         break;

But the assert fails and vgdb dumps core. The gdb testcase doesn't care, because it already finished its test and just makes sure all processes are gone. But it slowly fills your disk with core files (if you have enabled them) when running the testsuite.

Proposed fix is to simply check first whether the program has termined normally or by getting a fatal signal:

diff --git a/coregrind/vgdb-invoker-ptrace.c b/coregrind/vgdb-invoker-ptrace.c
index cb37677d5..389748960 100644
--- a/coregrind/vgdb-invoker-ptrace.c
+++ b/coregrind/vgdb-invoker-ptrace.c
@@ -267,7 +267,8 @@ Bool waitstopped (pid_t pid, int signal_expected, const char *msg)
          return False;
       }
 
-      if (WIFEXITED(status)) {
+      /* The process either exited or was terminated by a (fatal) signal. */
+      if (WIFEXITED(status) || WIFSIGNALED(status)) {
          shutting_down = True;
          return False;
       }
Comment 1 Philippe Waroquiers 2021-03-09 17:28:40 UTC
Patch looks ok to me.
Comment 2 Mark Wielaard 2021-03-09 17:55:13 UTC
commit 0f0205f683f70400406276936b1e0d1a7fa9cf72
Author: Mark Wielaard <mark@klomp.org>
Date:   Tue Mar 9 18:51:57 2021 +0100

    vgdb might crash if valgrind is killed
    
    This is an odd corner case, but happens specifically with the gdb
    testcase make check TESTS=gdb.base/valgrind-infcall-2.exp. At the
    end valgrind gets killed with SIGKILL (-9) which cannot be blocked.
    But vgdb at the time is inside waitstopped. It sees the process wasn't
    exited (WIFEXITED(status) is false) and so assumes the process was
    stopped by a signal. Which it asserts:
    
          assert (WIFSTOPPED(status));
          signal_received = WSTOPSIG(status);
          if (signal_received == signal_expected)
             break;
    
    But the assert fails and vgdb dumps core. The gdb testcase doesn't care,
    because it already finished its test and just makes sure all processes
    are gone. But it slowly fills your disk with core files (if you have
    enabled them) when running the testsuite.
    
    The fix is to simply check first whether the program has termined
    normally or by getting a fatal signal.
    
    https://bugs.kde.org/show_bug.cgi?id=434035