[Lldb-commits] [PATCH] D93874: [process] fix exec support on Linux

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

Sorry for returning late to this diff, but I have some additional information. This is what's happening:

- Before the exec, the base thread plan holds a reference to the thread pointer
- During the exec, the original thread is destroyed in ProcessGDBRemote::SetLastStopPacket, and later a new Thread pointer is created for the same tid. Notice that the base thread plan is still holding a reference to the previous pointer even though there's a new Thread pointer for the same tid. On Linux, exec preserves the tid. This step is where I think there's a wrong invariant.
- Then, at some point, the base thread plan is asked its stop reason, and it'll try to use the destroyed pointer, thus crashing.

I've found three possible fixes for this (I'm pretty sure that you can come up with more):

- The first one is what this diff does, which is clearing all the thread plans once the gdbremote client knows there's an exec.
- The second one is modifying the ThreadPlan::GetThread method, which currently looks like this

  if (m_thread)
      return *m_thread;
  
    ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
    m_thread = thread_sp.get();
    return *m_thread;

Changing it into

  if (m_thread && m_thread->IsValid()) // IsValid will return false for a deleted thread
      return *m_thread;
  
    ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
    m_thread = thread_sp.get();
    return *m_thread;

also works, as it will make the ThreadPlan grab the new thread pointer.

- Another solution is to create a method ThreadPlan::PrepareForExec() which just clears the m_thread pointer in the the thread plan, and call it from ProcessGDBRemote::SetLastStopPacket, where we handle the exec.

I prefer the first solution, as the thread plans are all cleared in preparation for the new process.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93874/new/

https://reviews.llvm.org/D93874