delve icon indicating copy to clipboard operation
delve copied to clipboard

Support for instruction reordering / jumping

Open dlsniper opened this issue 6 years ago • 8 comments

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 avatar Dec 14 '17 23:12 dlsniper

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

dlsniper avatar Dec 14 '17 23:12 dlsniper

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.

derekparker avatar Dec 15 '17 17:12 derekparker

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.

dlsniper avatar Dec 15 '17 22:12 dlsniper

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

derekparker avatar Dec 18 '17 18:12 derekparker

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.

heschi avatar Dec 18 '17 18:12 heschi

  • 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 https://github.com/golang/go/issues/22558, in particular https://github.com/golang/go/issues/22558#issuecomment-348286326 and also the associated CL -- this means that skipping instruction can skip invisible initialization.

aarzilli avatar Dec 18 '17 19:12 aarzilli

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 avatar Dec 28 '17 23:12 dlsniper

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 avatar May 05 '18 20:05 dlsniper