Page MenuHomePhabricator

[lldb] Fix incorrect error handling in GDBRemoteCommunicationClient::SendGetSupportedTraceType
ClosedPublic

Authored by teemperor on Nov 19 2020, 9:17 AM.

Details

Summary

GDBRemoteCommunicationClient::SendGetSupportedTraceType is checking whether the
response is !response.IsNormalResponse() and infers from that that it is an error response.
However, it could be either "unsupported" or "error". If we get an unsupported response,
the code then tries to generate an llvm::Expected from the non-error response which then asserts.

Debugserver doesn't implement jLLDBTraceSupportedType, so we get an unsupported response
whenever this function is called on macOS.

This fixes the TestAproposWithProcess on macOS (where the apropos command will query
the CommandObjectTraceStart which then sends the trace type query package).

Diff Detail

Event Timeline

teemperor requested review of this revision.Nov 19 2020, 9:17 AM
teemperor created this revision.
teemperor edited the summary of this revision. (Show Details)
shafik accepted this revision.Nov 19 2020, 9:54 AM

LGTM, there are no tests currently?

This revision is now accepted and ready to land.Nov 19 2020, 9:54 AM
wallace accepted this revision.Nov 19 2020, 10:02 AM

Thanks

Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2020, 10:14 AM

From what understand this is Linux exclusive feature at the moment, and the command test itself is testing the command but missing the edge case of testing this against a running process. Not sure if it's worth starting a process just to test that the command still just prints the "unsupported feature" error. I'll leave a dedicated test up to @wallace .