GDB server stub for remote debugging (using upstream debugging interfaces) #1244
Conversation
boricj
changed the title
boricj
force-pushed
the
gdbserver
branch
4 times, most recently
from
fa7d7d3
to
5abf528
Compare
There was a problem hiding this comment.
Thanks again for working through this.
I have some threading concerns, mainly with the commands - if they execute only after sending an interrupt first, replacing those PauseSystem calls will do the trick. But if breakpoints can be added during execution, you'll probably want to use executeOnEmulationThread if the system isn't paused.
I still think we're better off implementing this in the core rather than Qt, but that conversion can happen later, or I can do it myself :) But otherwise, looks good!
src/duckstation-qt/gdbconnection.cpp
Outdated
| connect(&m_socket, &QTcpSocket::disconnected, this, &GDBConnection::gotDisconnected); | ||
|
|
||
| if (m_socket.setSocketDescriptor(m_descriptor)) { | ||
| g_host_interface->PauseSystem(true); |
There was a problem hiding this comment.
This should be pauseSystem(true, true), since it will be executed on a different thread AFAICT (and we want to wait for the system to finish pausing).
| @@ -814,6 +815,16 @@ void MainWindow::updateEmulationActions(bool starting, bool running) | |||
| } | |||
| } | |||
|
|
|||
| if (g_settings.debugging.enable_gdb_server) { | |||
There was a problem hiding this comment.
Probably not the best place for this, but we can move it later if we move the server to the core.
src/common/string_util.h
Outdated
| @@ -90,22 +113,26 @@ inline std::optional<bool> FromChars(const std::string_view& str) | |||
|
|
|||
| return std::nullopt; | |||
| } | |||
|
|
|||
| #if 0 | |||
There was a problem hiding this comment.
If this is replaced by the above implementation, can just remove it rather than #if 0'ing it out.
src/duckstation-qt/gdbconnection.cpp
Outdated
|
|
||
| if (GDBProtocol::IsPacketInterrupt(m_readBuffer)) { | ||
| Log_DebugPrintf("(%u) > Interrupt request", m_descriptor); | ||
| g_host_interface->PauseSystem(true); |
There was a problem hiding this comment.
Same here - this should use pauseSystem. Specifically pauseSystem(true, true), that way it blocks the server thread until the emu thread actually pauses, and doesn't race when executing other commands such as adding breakpoints.
src/duckstation-qt/gdbconnection.cpp
Outdated
| } | ||
| else if (GDBProtocol::IsPacketContinue(m_readBuffer)) { | ||
| Log_DebugPrintf("(%u) > Continue request", m_descriptor); | ||
| g_host_interface->PauseSystem(false); |
There was a problem hiding this comment.
pauseSystem(false, false) should be okay here I think, as the client won't send any other commands before another interrupt request?
Just like #1192, but rebased on top of upstream's debugging interfaces and without watchpoints.