-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GDBServer fixes #3592
base: main
Are you sure you want to change the base?
GDBServer fixes #3592
Conversation
This doesn't behave properly anymore now that thread management was moved to the frontend.
This will be necessary to have the frontend track when threads are executing, for gdbserver.
When parsing `comm`, by default it will have a newline which breaks gdb in some cases. Strip out the whitespace to fix that issue.
Otherwise we won't wait for gdbserver connection
This ensures that gdb switches to the correct thread on fault in a multithreaded environment.
This allows us to reconstruct an accurate RIP on fault rather than ending up at the start of the block.
Otherwise some later spin-loops will wait forever.
Don't notify already paused threads. This causes some weird races.
This function was unused.
We can't currently guarantee this condition_variable is always unlocked without more major refactoring. This was causing GDB to wait forever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into the problem! It seems that I still can't do anything useful with gdb-multiarch
connecting to FEXLoader -G ...
. Is there anything specifically that should be working now that I can test?
Otherwise, looks like this PR adds a few similarly named interfaces to already sparely documented APIs. That should be sorted out to keep the interfaces somewhat accessible.
@@ -117,6 +117,7 @@ class ThreadManager final { | |||
void Run(); | |||
void Step(); | |||
void Stop(bool IgnoreCurrentThread = false); | |||
void Pausing(FEXCore::Core::InternalThreadState* Thread); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between this and "Pause"?
uint64_t HostPC {}; | ||
FEXCore::Core::InternalThreadState* Thread {}; | ||
}; | ||
ThreadBreakEventInfoStruct ThreadBreakEventInfo {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using std::optional
here is more explicit than setting Thread to nullptr. That also protects us against precondition violations, such as if we accidentally read this state when not at a breakpoint.
@@ -37,6 +37,7 @@ class GdbServer { | |||
|
|||
private: | |||
void Break(int signal); | |||
void BreakThread(FEXCore::Core::InternalThreadState* Thread, int signal); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between this and Break
?
@@ -111,6 +111,8 @@ class ThreadManager final { | |||
void StopThread(FEXCore::Core::InternalThreadState* Thread); | |||
void RunThread(FEXCore::Core::InternalThreadState* Thread); | |||
|
|||
void RunPrimaryThread(FEXCore::Context::Context* CTX, FEXCore::Core::InternalThreadState* Thread); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between this and "RunThread"?
This gets gdbserver working for simpler cases again. Some of the work is working around #3535 not being complete since some of the thread state management is still split between FEXCore and the frontend.
I'm still working on fixing that code, but until then lets at least get gdbserver working again so we can get x86 debugging.