Skip to content
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

Support for instruction reordering / jumping #1045

Open
dlsniper opened this issue Dec 14, 2017 · 9 comments
Open

Support for instruction reordering / jumping #1045

dlsniper opened this issue Dec 14, 2017 · 9 comments

Comments

@dlsniper
Copy link
Contributor

This was received on GoLands issue tracker, see issue GO-5109.

The original request is:

a Visual Studio/C#-like "Set Next Statement" debug feature would be amazing.
It basically allows you to move the pointer to change the execution flow.
See: https://msdn.microsoft.com/en-us/library/y740d9d3.aspx

From my understanding of the page documentation:

While the debugger is paused, you can move the instruction pointer to set the next statement of code to be executed. A yellow arrowhead in the margin of a source or Disassembly window marks the location of the next statement to be executed. By moving this arrowhead, you can skip over a portion of code or return to a line previously executed. You can use this for situations such as skipping a section of code that contains a known bug.

I believe this effectively means that we'd need to insert jumps in the source code to allow the execution to skip over code portions.

Now, given Go's nature of things, I don't expect this to be 100% bulletproof in the first version, so basically no validations would be required (such as making sure that there's no new memory allocation done by creating a variable and so on). Later on we could think about that.

As I don't know how complex this is to do, in general or in Go in particular, I'll leave this with you. Please let me know if there's anything that I can help.

Thank you.

@dlsniper
Copy link
Contributor Author

As an additional question, do you think this would be a simple enough feature for a newbie contributor such as me? Thank you.

@dlsniper dlsniper changed the title Support for instruction reordering Support for instruction reordering / jumping Dec 14, 2017
@derekparker
Copy link
Member

derekparker commented Dec 15, 2017

This is definitely an interesting feature, but it can also be dangerous during a debug session.

I believe this effectively means that we'd need to insert jumps in the source code to allow the execution to skip over code portions.

We wouldn't have to modify any of the instructions, we would simply have to set the program counter register to point to the address of the instruction we want to start executing.

There are some considerations for this feature, as we could easily corrupt the program being executed. We could attempt to detect moves that would very likely corrupt the program and warn / stop that from happening.

As an additional question, do you think this would be a simple enough feature for a newbie contributor such as me? Thank you.

The basic feature would be fairly easy to implement. The user would click on a file location in the IDE (or describe it in the CLI via linespec) and then we would translate that file:line to an instruction address in the program, and then set the value of the program counter register to that instruction. We have plenty of functions to be able to accomplish this part easily.

As we'd only be modifying the program counter, going to another function would be dangerous as the stack frame would still be setup from the old function, etc. There may be GC implications here as well. I could see having this feature warn if the user decides to make a move that is outside of the current function (we could also pretty easily detect this with functions we already have) and have the opportunity to cancel the action.

@dlsniper
Copy link
Contributor Author

This is definitely an interesting feature, but it can also be dangerous during a debug session.

I agree on how dangerous it can be.

We wouldn't have to modify any of the instructions, we would simply have to set the program counter register to point to the address of the instruction we want to start executing.

Oh, ok, my apologies for speculating on this.

There are some considerations for this feature, as we could easily corrupt the program being executed. We could attempt to detect moves that would very likely corrupt the program and warn / stop that from happening.

Yes, I agree that this might be the case. I would say just declining to run the operation could be enough, with an error message as to why it's not possible to do so. Both the CLI and the API clients could then act on this as needed.

going to another function would be dangerous as the stack frame would still be setup from the old function, etc.

I know I'll make this sound more trivial than it is but what I have in mind is allowing jumps only in the current scope and return an error telling the user that this operation is not permitted.

Regarding the GC complexity, should I try to get someone from the Go team pinged about this issue to jump in as well? My knowledge is very limited around these parts of Go unfortunately.

@derekparker
Copy link
Member

@heschik @aarzilli any more thoughts / considerations here regarding potential program corruption when this feature is used?

@heschi
Copy link
Contributor

heschi commented Dec 18, 2017

Jumping to a completely unrelated function is clearly asking for trouble. I wonder whether it'd be okay to force a function to return early to allow people to jump in non-leaf functions.

I can't think of any reason the GC will give any trouble here, presuming you're not doing something crazy like jumping out of the middle of the GC itself.

Offhand, defers are the thing I'd be concerned about. I think it might be okay because the runtime just maintains a stack, without the compiler doing anything special, but if I'm wrong and the compiler generates code that has assumptions about where defers can and cannot exist, strange stuff will happen if you jump around.

@aarzilli
Copy link
Member

  • I would definitely disallow moving to a different function.
  • This is indeed an easy feature to add, just look up in debugger.go how we resolve locspecs when setting breakpoints and do the same, then use proc.Registers.SetPC.
  • I don't think there is any problem in particular with the GC besides the general problem that you could use this feature to skip the initialization of a variable and that could cause the GC to fail (but it could cause anything to fail)
  • The big problem is that the compiler does a fair bit of reordering even when optimizations are disabled, see for example the comments towards the end of cmd/compile: bad line number attached to LEA instruction golang/go#22558, in particular cmd/compile: bad line number attached to LEA instruction golang/go#22558 (comment) and also the associated CL -- this means that skipping instruction can skip invisible initialization.

@dlsniper
Copy link
Contributor Author

I'll try and come up with something for this in the near future, just so that I get more familiar with Delve's inner code / how a debugger works in general and so on.

dlsniper added a commit to dlsniper/delve that referenced this issue May 5, 2018
@dlsniper
Copy link
Contributor Author

dlsniper commented May 5, 2018

Sorry for the delay. The patch for this is almost done, I have a few things to polish then I'll send the PR.

dlsniper added a commit to dlsniper/delve that referenced this issue May 14, 2018
dlsniper added a commit to dlsniper/delve that referenced this issue May 14, 2018
dlsniper added a commit to dlsniper/delve that referenced this issue May 17, 2018
This currently is a more or less work in progress, it depends on the
feature set we want to support this feature. In my opinion, this could
be flagged as an experimental feature and if the user has an issue then
it's their fault. Maybe it's not a 100% good statement to have. There
are cases where this could be useful to have and a user that understands
how to use it, will be able to use it to the full extent.

I've tested this only on Windows, I'll let the integration test do the
job on the other systems.

Cross function jumping works as well, but only in the toy example below,
I have no clue how the runtime will be affected by this.

Fixes go-delve#1045
@nour-s
Copy link

nour-s commented Sep 24, 2024

Hi @dlsniper , I just came across this discussion and I wonder if we are planning to support it or it is deemed impractical?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants