Page MenuHomePhabricator

[lldb] [gdb-remote server] Introduce new stop reasons for fork and vfork
ClosedPublic

Authored by mgorny on Apr 9 2021, 7:01 AM.

Details

Summary

Introduce three new stop reasons for fork, vfork and vforkdone events.
This includes server support for serializing fork/vfork events into
gdb-remote protocol. The stop infos for the two base events take a pair
of PID and TID for the newly forked process.

Diff Detail

Event Timeline

mgorny requested review of this revision.Apr 9 2021, 7:01 AM
mgorny created this revision.
mgorny updated this revision to Diff 336668.Apr 11 2021, 8:35 AM
mgorny retitled this revision from [lldb] Introduce new stop reasons: fork and vfork to [lldb] Introduce new stop reasons for fork and vfork.
mgorny edited the summary of this revision. (Show Details)

Add 'vforkdone' stop reason. Move overrideable actions to this patch.

mgorny updated this revision to Diff 339198.Apr 21 2021, 5:46 AM
mgorny retitled this revision from [lldb] Introduce new stop reasons for fork and vfork to [lldb] [gdb-remote server] Introduce new stop reasons for fork and vfork.
mgorny edited the summary of this revision. (Show Details)

@labath, does this subset look better?

For other readers: as discussed in D100191, I've stripped client part of this (except for trivial enumerations). The server part will be used in tests but the stop reasons won't be reported to regular clients as they don't report fork-events support.

labath accepted this revision.Apr 23 2021, 12:28 PM
labath added inline comments.
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
946–948

I just realized this is implicitly assuming the multiprocess extension is enabled/supported. That makes sense since forking always involves multiple processes, though maybe the server should check that the client actually reports it as supported.

This revision is now accepted and ready to land.Apr 23 2021, 12:28 PM
mgorny added inline comments.Apr 23 2021, 2:07 PM
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
946–948

Makes sense. I'll add it to the extension patch since that's where we determine if fork/vfork is supported.

mgorny marked an inline comment as done.Apr 23 2021, 2:46 PM
mgorny updated this revision to Diff 340178.Apr 23 2021, 3:22 PM

Add an assert to make sure that process doesn't report fork/vfork stop reason without multiprocess support.

mgorny updated this revision to Diff 340180.Apr 23 2021, 3:23 PM

Actually, add asserts for fork/vfork extensions too.

This revision was landed with ongoing or failed builds.Apr 24 2021, 2:26 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2021, 2:26 AM

Add an assert to make sure that process doesn't report fork/vfork stop reason without multiprocess support.

I guess this is the reason for that..

What I meant here was to do a sanity check on the client extensions -- like if a client claims to support "vfork-events" but not "multiprocess", then its probably broken (though it's a question what we should actually do about that). In the server our gdb-remote class always (I hope) supports multiprocess, so there's no danger of inconsistency.