Add an optional "reverse" option to the gdb-remote command,
which causes lldb to listen on the specified [host:]port,
waiting for the remote server to reverse-connect back to lldb.
Details
- Reviewers
zturner clayborg tberghammer jasonmolenda
Diff Detail
Event Timeline
Looks like there's a few inconsistencies with our coding style. Can you please run clang-format on this patch?
Perhaps I ran clang-format in the wrong configuration, but it looks like it made the function declaration style in include/lldb/Target/Platform.h a bit inconsistent. Is this the desired behavior?
I think this would be better if you could specify the connection direction in the url. Also, a test for this functionality would be great.
source/Interpreter/CommandInterpreter.cpp | ||
---|---|---|
643 | how about just using listen://... to mean reverse connection? | |
source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h | ||
226 | I don't think we need to pass this reverse=false around everywhere. We already support listen://host:port url scheme (see e.g. ConnectionFileDescriptor::Connect`). Could you use that to specify connection direction? |
Would it be better to add a test in the same commit or a separate one?
source/Interpreter/CommandInterpreter.cpp | ||
---|---|---|
643 | My concern with doing it this way was that ProcessGDBRemote::ConnectToDebugserver() seems to implicitly require a connect://... url, and I didn't want to risk breaking future changes. However, it still works correctly if I pass a listen://... url, so it seems like a reasonable change. | |
source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h | ||
226 | I had considered that, but wasn't sure whether parsing the direction out of the url string every time it was needed would be better or worse than passing it around as a bool. I'll make the change. |
I like this more, but I'd like to take it a bit further. I think we won't have to parse the url string at all (see comments).
I believe the best way to avoid breaking (and being broken by) other changes is to make sure this fits in nicely in the existing architecture. Putting this logic into ConnectToDebugServer seems like the logical place.
You can wait with the test until we have settled on the final implementation, but I'd like the test to go in together with the change.
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | ||
---|---|---|
732 | If we can get ConnectToDebugserver working with a listen url then we will get rid of this url parsing hack... | |
1075 | ConnectionFileDescriptor::Connect seems to be able to handle listen:// urls. What would happen if you just passed the listen url to ConnectToDebugserver |
It looks like passing a listen:// url to ConnectToDebugserver actually works "out of the box". Thanks! I'll work on writing a unit test.
This functionality can already be accessed by using the "process connect" command with a listen url directly.
how about just using listen://... to mean reverse connection?