This is an archive of the discontinued LLVM Phabricator instance.

[LLGS] Spawned process handling cleanup
ClosedPublic

Authored by labath on Jul 28 2015, 3:08 AM.

Details

Summary

This commit moves the m_spawned_pids member from the common LLGS/Platform class to the plaform
specific part. This enables us to remove LLGS code, which was attempting to manage the
m_spawned_pids contents, but at the same time making sure, there is only one debugged process. If
we ever want to do multi-process debugging, we will probably want to replace this with a set of
NativeProcessProtocolSP anyway. The only functional change is that support for
qKillSpawnedProcess packet is removed from LLGS, but this was not used there anyway (we have the
k packet for that).

Diff Detail

Event Timeline

labath updated this revision to Diff 30797.Jul 28 2015, 3:08 AM
labath retitled this revision from to [LLGS] Spawned process handling cleanup.
labath updated this object.
labath added reviewers: ovyalov, clayborg.
labath added a subscriber: lldb-commits.
ovyalov accepted this revision.Jul 28 2015, 7:32 AM
ovyalov edited edge metadata.
ovyalov added inline comments.
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
280

We may want to check for GetID() != LLDB_INVALID_PROCESS_ID becauseNativeProcessProtocol::Attach doesn't nullify native_process_sp if AttachToInferior fails.

Considering how often we check something like m_debugged_process_sp && (m_debugged_process_sp->GetID () != LLDB_INVALID_PROCESS_ID)" having a separate method for this might be pretty convenient - we can do it in another CL.

299–300

return Error(...)?

This revision is now accepted and ready to land.Jul 28 2015, 7:32 AM
clayborg requested changes to this revision.Jul 28 2015, 9:09 AM
clayborg edited edge metadata.

Fix the m_debugged_process_sp check and this is good to go.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
280

Or put ask the process to validate itself with IsBeingDebugged() or some other function that verifies the process is being debugged:

if (m_debugged_process_sp && m_debugged_process_sp->IsBeingDebugged())
This revision now requires changes to proceed.Jul 28 2015, 9:09 AM
labath updated this revision to Diff 30828.Jul 28 2015, 9:25 AM
labath edited edge metadata.
labath marked an inline comment as done.
  • Add a check for process validity
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
280

I have added a check for LLDB_INVALID_PROCESS_ID. For the future I would like to clean this up more so that m_debugged_process_sp is non-null if and only if we have a valid debugged process.

clayborg accepted this revision.Jul 28 2015, 9:48 AM
clayborg edited edge metadata.

Looks good as long as the process ID is set to invalid when the process exits?

This revision is now accepted and ready to land.Jul 28 2015, 9:48 AM
This revision was automatically updated to reflect the committed changes.