Page MenuHomePhabricator

Launch lldb-gdbserver in same process group when launched remotely using lldb-platform.
ClosedPublic

Authored by flackr on Jan 27 2015, 1:41 PM.

Details

Summary

When launched from lldb-platform, lldb-gdbserver's lifetime should be tied to the lldb-platform process. Otherwise the platform can die and leave a lingering lldb-gdbserver process. The existing launch argument LaunchInSeparateProcessGroup is used to denote this as this seems to be exactly what it is intended for.

Diff Detail

Repository
rL LLVM

Event Timeline

flackr updated this revision to Diff 18847.Jan 27 2015, 1:41 PM
flackr retitled this revision from to Launch lldb-gdbserver in same process group when launched remotely using lldb-platform..
flackr updated this object.
flackr edited the test plan for this revision. (Show Details)
flackr added a reviewer: zturner.
flackr set the repository for this revision to rL LLVM.
flackr added a subscriber: Unknown Object (MLST).

Looks fine to me. But I'm neither a linux person nor an llgs person, so I'm fine with it as long as Siva and Oleksiy are.

No, this is how the server is launched when run from lldb-platform, where we don't want it to run in a separate process group. Note the m_is_platform check here:
https://github.com/llvm-mirror/lldb/blob/0d9ea6ba74541fe4a73df96122dc7a5913ac91e5/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp#L1887

I could call SetLaunchInSeparateProcessGroup(false) to make that explicit.

ovyalov accepted this revision.Jan 27 2015, 2:39 PM
ovyalov edited edge metadata.

Do we need to call SetLaunchInSeparateProcessGroup(true) here https://github.com/llvm-mirror/lldb/blob/0d9ea6ba74541fe4a73df96122dc7a5913ac91e5/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp#L1918?

No, this is how the server is launched when run from lldb-platform, where we don't want it to run in a separate process group. Note the m_is_platform check here:
https://github.com/llvm-mirror/lldb/blob/0d9ea6ba74541fe4a73df96122dc7a5913ac91e5/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp#L1887

I could call SetLaunchInSeparateProcessGroup(false) to make that explicit.

Yes, I think it makes sense to do - otherwise we may rely on default value of LaunchInSeparateProcessGroup.

This revision is now accepted and ready to land.Jan 27 2015, 2:39 PM
flackr updated this revision to Diff 18897.Jan 28 2015, 8:53 AM
flackr edited edge metadata.

Explicitly set LaunchInSeparateProcessGroup to false from GDBRemoteCommunicationServer::Handle_qLaunchGDBServer to make this explicit and not rely on default value.

I don't have commit access so if/when this is good can someone land it for me? Thanks.