[lldb] Don't invalid register context after setting thread pc's (#109499) #9315
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…#109499) Some gdb remote serial protocol stubs will send the thread IDs and PCs for all threads in a process in the stop-reply packet. lldb often needs to know the pc values for all threads while at a private stop, and that results in <n-1> read-register packets for <n> threads, and can be a big performance problem when this is a hot code path. GDBRemoteRegisterContext tracks the StopID of when its values were set, and when the thread's StopID has incremented, it marks all values it has as Invalid, and knows to refetch them. We have a code path that resulted in setting the PCs for all the threads, and then `ProcessGDBRemote::CalculateThreadStopInfo` *forcing* an invalidation of all the register contexts, forcing us to re-read the pc values for all threads except the one that stopped. There are times when it is valid to force an invalidation of the regsiter cache - for instance, if the layout of the registers has changed because the processor state is different, or we've sent a write-all-registers packet to the inferior and we want to make sure we stay in sync with the inferior. But there was no reason for this method to be forcing the register context to be invalid. I added a test when running on Darwin systems, where debugserver always sends the thread IDs and PCs, which turns on packet logging. The test runs against an inferior which has 4 threads; it steps over a dlopen() call, steps in to a user function with debug info, steps-over and steps-in across source lines with multiple function calls, and then examines the packet log and flags it as an error if lldb asked for the pc value of any thread at any point in the debug session. For this program and the operations we're doing, with debugserver that provides thread IDs and PCs, we should never ask for the value of a pc register. rdar://136247381 (cherry picked from commit 6e6d5ea)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some gdb remote serial protocol stubs will send the thread IDs and PCs for all threads in a process in the stop-reply packet. lldb often needs to know the pc values for all threads while at a private stop, and that results in read-register packets for threads, and can be a big performance problem when this is a hot code path.
GDBRemoteRegisterContext tracks the StopID of when its values were set, and when the thread's StopID has incremented, it marks all values it has as Invalid, and knows to refetch them.
We have a code path that resulted in setting the PCs for all the threads, and then
ProcessGDBRemote::CalculateThreadStopInfoforcing an invalidation of all the register contexts, forcing us to re-read the pc values for all threads except the one that stopped.There are times when it is valid to force an invalidation of the regsiter cache - for instance, if the layout of the registers has changed because the processor state is different, or we've sent a write-all-registers packet to the inferior and we want to make sure we stay in sync with the inferior. But there was no reason for this method to be forcing the register context to be invalid.
I added a test when running on Darwin systems, where debugserver always sends the thread IDs and PCs, which turns on packet logging. The test runs against an inferior which has 4 threads; it steps over a dlopen() call, steps in to a user function with debug info, steps-over and steps-in across source lines with multiple function calls, and then examines the packet log and flags it as an error if lldb asked for the pc value of any thread at any point in the debug session.
For this program and the operations we're doing, with debugserver that provides thread IDs and PCs, we should never ask for the value of a pc register.
rdar://136247381
(cherry picked from commit 6e6d5ea)