This is an archive of the discontinued LLVM Phabricator instance.

[lldb] [gdb-remote server] Refactor handling qSupported
ClosedPublic

Authored by mgorny on Apr 8 2021, 2:29 PM.

Details

Summary

Refactor handling qSupported to use a virtual HandleFeatures() method.
The client-provided features are split into an array and passed
to the method. The method returns an array of server features that are
concatenated into the qSupported response to the server.

The base implementation of HandleFeatures()
in GDBRemoteCommunicationServerCommon now includes only flags common
to both platform server and llgs, while llgs-specific flags are inserted
in GDBRemoteCommunicationServerLLGS.

Diff Detail

Event Timeline

mgorny requested review of this revision.Apr 8 2021, 2:29 PM
mgorny created this revision.

This is split from D99864 as a minimal independent change.

rovka added a subscriber: rovka.Apr 12 2021, 4:41 AM
rovka added inline comments.
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
843

This seems to have disappeared, shouldn't it be in GDBRemoteCommunicationServerCommon::HandleFeatures?

mgorny updated this revision to Diff 336808.Apr 12 2021, 5:14 AM
mgorny marked an inline comment as done.

Fix missing QListThreadsInStopReply+.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
843

My mistake, sorry. Fixed now.

mgorny updated this revision to Diff 336977.Apr 12 2021, 3:21 PM

clang-format

labath accepted this revision.Apr 12 2021, 11:36 PM

cool

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
835

remove variable used only once?

1309–1310

I think that these two should also be llgs-specific.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
151

this const is useless

This revision is now accepted and ready to land.Apr 12 2021, 11:36 PM
mgorny marked 2 inline comments as done.Apr 13 2021, 2:38 AM
mgorny added inline comments.
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
1309–1310

I actually left them here because the respective Handle_QThreadSuffixSupported and Handle_QListThreadsInStopReply methods are also in common.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
151

Yeah, I probably meant to do const ...& but I guess it doesn't matter much for ArrayRef.

labath added inline comments.Apr 13 2021, 3:01 AM
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
1309–1310

Fair enough. I'll see what I can do about that once this patch lands.

This revision was landed with ongoing or failed builds.Apr 13 2021, 3:14 AM
This revision was automatically updated to reflect the committed changes.
mgorny marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2021, 3:14 AM