[Lldb-commits] [PATCH] Fix debuggee termination from ProcessPOSIX::DoDestroy

[Todd Fiala]

  Hey Ed,

  I gave this a try.  I fixed up a few minor bits (at least one compile error, commented in the code, and fixed all the warnings I got on gcc 4.8.2).  Something isn't right since I had 60+ tests fail after including this bit.

  I'll need to dig in a bit more since my sign casting could conceivably be part of the issue.  (I'll try again with just your test plus necessary syntax issue fix).

  Here is the complete patch I was using:

  Index: source/Plugins/Process/FreeBSD/ProcessMonitor.h
  ===================================================================
  --- source/Plugins/Process/FreeBSD/ProcessMonitor.h	(revision 204750)
  +++ source/Plugins/Process/FreeBSD/ProcessMonitor.h	(working copy)
  @@ -194,11 +194,9 @@
       bool
       SingleStep(lldb::tid_t unused, uint32_t signo);

  -    /// Sends the inferior process a PTRACE_KILL signal.  The inferior will
  -    /// still exists and can be interrogated.  Once resumed it will exit as
  -    /// though it received a SIGKILL.
  +    /// Terminate the traced process.
       bool
  -    BringProcessIntoLimbo();
  +    Kill();

       lldb_private::Error
       Detach(lldb::tid_t tid);
  Index: source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
  ===================================================================
  --- source/Plugins/Process/FreeBSD/ProcessMonitor.cpp	(revision 204750)
  +++ source/Plugins/Process/FreeBSD/ProcessMonitor.cpp	(working copy)
  @@ -702,7 +702,7 @@

   //------------------------------------------------------------------------------
   /// @class KillOperation
  -/// @brief Implements ProcessMonitor::BringProcessIntoLimbo.
  +/// @brief Implements ProcessMonitor::Kill.
   class KillOperation : public Operation
   {
   public:
  @@ -1648,7 +1648,7 @@
   }

   bool
  -ProcessMonitor::BringProcessIntoLimbo()
  +ProcessMonitor::Kill()
   {
       bool result;
       KillOperation op(result);
  Index: source/Plugins/Process/Linux/ProcessMonitor.h
  ===================================================================
  --- source/Plugins/Process/Linux/ProcessMonitor.h	(revision 204750)
  +++ source/Plugins/Process/Linux/ProcessMonitor.h	(working copy)
  @@ -172,11 +172,9 @@
       bool
       SingleStep(lldb::tid_t tid, uint32_t signo);

  -    /// Sends the inferior process a PTRACE_KILL signal.  The inferior will
  -    /// still exists and can be interrogated.  Once resumed it will exit as
  -    /// though it received a SIGKILL.
  +    /// Terminate the traced process.
       bool
  -    BringProcessIntoLimbo();
  +    Kill();

       lldb_private::Error
       Detach(lldb::tid_t tid);
  Index: source/Plugins/Process/Linux/ProcessMonitor.cpp
  ===================================================================
  --- source/Plugins/Process/Linux/ProcessMonitor.cpp	(revision 204750)
  +++ source/Plugins/Process/Linux/ProcessMonitor.cpp	(working copy)
  @@ -904,31 +904,6 @@
   }

   //------------------------------------------------------------------------------
  -/// @class KillOperation
  -/// @brief Implements ProcessMonitor::BringProcessIntoLimbo.
  -class KillOperation : public Operation
  -{
  -public:
  -    KillOperation(bool &result) : m_result(result) { }
  -
  -    void Execute(ProcessMonitor *monitor);
  -
  -private:
  -    bool &m_result;
  -};
  -
  -void
  -KillOperation::Execute(ProcessMonitor *monitor)
  -{
  -    lldb::pid_t pid = monitor->GetPID();
  -
  -    if (PTRACE(PTRACE_KILL, pid, NULL, NULL, 0))
  -        m_result = false;
  -    else
  -        m_result = true;
  -}
  -
  -//------------------------------------------------------------------------------
   /// @class DetachOperation
   /// @brief Implements ProcessMonitor::Detach.
   class DetachOperation : public Operation
  @@ -1167,7 +1142,7 @@
       if (envp == NULL || envp[0] == NULL)
           envp = const_cast<const char **>(environ);

  -    if ((pid = terminal.Fork(err_str, err_len)) == -1)
  +    if ((pid = terminal.Fork(err_str, err_len)) == static_cast<lldb::pid_t> (-1))
       {
           args->m_error.SetErrorToGenericError();
           args->m_error.SetErrorString("Process fork failed.");
  @@ -1268,7 +1243,7 @@
           }
           goto FINISH;
       }
  -    assert(WIFSTOPPED(status) && wpid == pid &&
  +    assert(WIFSTOPPED(status) && (static_cast<lldb::pid_t> (wpid) == pid) &&
              "Could not sync with inferior process.");

       if (!SetDefaultPtraceOpts(pid))
  @@ -1771,7 +1746,7 @@
           if (log)
               log->Printf ("ProcessMonitor::%s(bp) waitpid, pid = %" PRIu64 ", status = %d", __FUNCTION__, wait_pid, status);

  -        if (wait_pid == -1)
  +        if (wait_pid == static_cast<lldb::pid_t> (-1))
           {
               // If we got interrupted by a signal (in our process, not the
               // inferior) try again.
  @@ -1909,6 +1884,11 @@
                   if (wait_pid == tid)
                       Resume(wait_pid, eResumeSignalNone);
                   break;
  +
  +            case ProcessMessage::eExecMessage:
  +                if (log)
  +                    log->Warning ("ProcessMonitor::%s: eExecMessage not handled", __FUNCTION__);
  +                break;
           }
       }
       return false;
  @@ -2229,12 +2209,9 @@
   }

   bool
  -ProcessMonitor::BringProcessIntoLimbo()
  +ProcessMonitor::Kill ()
   {
  -    bool result;
  -    KillOperation op(result);
  -    DoOperation(&op);
  -    return result;
  +    return kill(GetPID(), SIGKILL) == 0;
   }

   bool
  Index: source/Plugins/Process/POSIX/ProcessPOSIX.cpp
  ===================================================================
  --- source/Plugins/Process/POSIX/ProcessPOSIX.cpp	(revision 204750)
  +++ source/Plugins/Process/POSIX/ProcessPOSIX.cpp	(working copy)
  @@ -337,11 +337,9 @@

       if (!HasExited())
       {
  -        // Drive the exit event to completion (do not keep the inferior in
  -        // limbo).
  +        assert (m_monitor);
           m_exit_now = true;
  -
  -        if ((m_monitor == NULL || kill(m_monitor->GetPID(), SIGKILL)) && error.Success())
  +        if (m_monitor->Kill())
           {
               error.SetErrorToErrno();
               return error;


================
Comment at: source/Plugins/Process/Linux/ProcessMonitor.cpp:2207
@@ -2231,3 +2206,3 @@
 bool
-ProcessMonitor::BringProcessIntoLimbo()
+ProcessMonitor::Kill
 {
----------------
This needs parens at the end


http://llvm-reviews.chandlerc.com/D3159