Refactor layers in Undo Delve fork #43
Draft
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
In Undo, debuggee state is immutable during replay, so reading the G pointer via code injection won't work; and in any case, this approach is simpler.
* Update the documentation to mention UDB or LiveRecorder alongside rr. * Add makefile support for testing the Undo backend. * Add DLV_RECORD_REPLAY_BACKEND environment variable which overrides the record-replay backend, for use in GoLand. * Implement checkpoints as a map from checkpoint number to time. * Suppress resume call after restart. * Implement "when" command using "get_time" serial command. * Implement call injection using "set_debuggee_volatile". * Implement "restart" using "goto_time". * Enable or disable Undo backend appropriately in test cases.
This changeset fixes a bug in which, after continuing to the end of a loaded LiveRecorder recording, it became impossible to reverse into the debug session. By using the same technique already implemented for rr support it is now possible to reach the end of time and then continue debugging. In addition, the exit code (where available) be reported correctly - although it will be reported as zero at the end of time if no exit was recorded. A workaround for apparent SIGCHILD handling (actually SIGSTOP, due to how the gdbserial protocol encodes signals) is removed as it is not required with this new implementation.
This fixes a bug in which it was not possible to reverse debug after a stepping operation e.g. next encountered the end of recorded history.
This ensures that the Undo backend can always find the server when `udb` is on the `PATH`. System-wide installs of UDB don't put the server on `PATH` by default, so this simplifies Delve setup in those cases.
The error handling here was a bit painful but only needs doing in these two places (for now).
This makes it possible to apply a standard formatting to Undo times from anywhere in the code that requires it - this will later be used to display checkpoint times.
This is for readability and for consistency with the existing UDB convention.
Allow fast navigation to extremes of history.
This changeset adds validation for the `start` / `end` magic arguments to the `restart` command, plus adds parsing for directly-specified bbcounts or bbcount:pc times. Doing the validation early allows us to bail out before the function makes any changes to global state. Parsing time specifications directly allows the user to jump to a time without it having a pre-existing checkpoint.
This is made compatible with UDB's bookmarks by using the UDB bookmark name as the "Note" field in Delve. To save Delve checkpoints, the "Note" must be made unique, so as to be a valid bookmark name in UDB.
This is in line with the actual use of this variable.
Previously we were trying to interpret an empty command as a time, which was not correct.
During inferior calls we are in volatile mode and should not attempt to use progress indicators. By tracking volatile mode we are able to correctly skip progress indicator commands.
This layer doesn't need to concern itself with gdbProcess structures for now.
mark-undoio
force-pushed
the
undo-v1.20.2-4-refactor-layers
branch
from
a58829f to
b4fea42
Compare
Move undoSession struct down to the gdbConn level.
The current implementation of `restart()` is trying to fit us into the model used by `rr` but it's not a very natural fit. It's also convenient to concentrate our code in `undo.go` where possible, since we're still maintaining a fork.
If we were a fully integrated backend then the undoCmd probably should live with the gdbConn code - but, for now, we're maintaining a fork and this should be easier to handle.
The exact details of when we'll activate them and how we do so are not relevant to the `gdbserver.go` code.
mark-undoio
force-pushed
the
undo-v1.20.2-4-refactor-layers
branch
from
b4fea42 to
3678774
Compare
Contributor
Author
|
I've moved on with this - I'll be rebasing and squashing these changes on top of |
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.
This PR reworks the way we've layered code in our fork, without changing functionality.
Goals:
undo.go.undo.goas part of maintaining the ongoing fork.Notable changes:
isUndoServerboolean is removed from thegdbConnstructure.*undoSessionpointer is moved from thegdbProcessstructure down into thegdbConnlevel.undoCmdis moved fromgdbserver_conn.gointoundo.goContinueOnceandRestartis moved as far as possible intoundo.go.Follow-on plans:
undo.go- I think they could be in a better order.v1.21.0release.