refactor: Simplify pkg/proc

[derekparker]

I think that pkg/proc has gotten way too big in scope, making it difficult to understand especially for those new to the project. I believe this clutter also makes it more complicated to add new arches and OSes. I propose we simplify pkg/proc by limiting its scope exclusively to low-level process manipulation, and move everything else into independent packages, and move any higher-level logic into service/debugger. As it stands right now, pkg/proc is aware of: low-level process execution / memory manipulation, DWARF logic (compilation units, line info, etc), various binary sections including moduledata, disassembly, variables and variable types, parsing the Go AST, the function call protocol, AND implementing other even lower-level backends.

I propose that pkg/proc implement a simple interface which abstracts what backend we are using, and is only responsible for manipulation of the process execution and memory. All other functionality should be split up into independent packages, and used by a higher-level concept which would be the debugger package.

The logic that should be moved out into independent packages, as I see it, is as follows:

• Variable eval / AST parsing code

I think this logic can be implemented in its own package. Functions can take a proc.MemReadWriter in order to actually get the values from memory and turn them into something useful, but this shouldn't be a part of the proc package itself.

• BinaryInfo

A lot of this code is higher level than proc as it deals with parsing the various sections in the binary we need to gather info from in order to unwind the stack, read variables, etc. I think it can be moved into its own package and used by debugger instead of proc.

• Stack unwinding

I believe this can be implemented in service/debugger by taking advantage of the new packages above and low-level functionality provided by the proc package.

• Next'ing, function call protocol and other high-level logic

This can be implemented by the debugger package as well. At a process level there is no concept of source code lines. This logic should be implemented at a higher level, leaving proc to implement only the low-level functionality that the logic builds on top of.

• Disassembly

Again, this is high level, this could be its own standalone package.

• Breakpoints

The concept of breakpoints is only of use to the debugger, an actual process knows nothing about them. This concept should be moved higher up.

I am creating this issue for awareness and discussion. At the time of writing there are also arch ports in play which this refactor would disrupt, so it should be completed after.

[aarzilli]

I agree that the disassembler could and should be split, and also that 'pkg/proc' is a bad name for what's going on inside 'pkg/proc'. I'd also suggest that the loclist code in bininfo.go is a good candidate for a new 'pkg/dwarf' subpackage.

That said I think if you look past the name, pkg/proc makes a lot of sense; at the moment the package 'pkg/proc' (and only proc, not its "subpackages") is mostly concerned with only one thing: translation between source level concepts (variables, lines of code, goroutines) and low level concepts (memory addresses, threads, slices of bytes).

I could see renaming it and leaving its subpackages inside pkg/proc, but aside from the disassembler and a few other things I think most stuff inside pkg/proc is a tightly couple unit. Splitting it would result in a set of tightly coupled packages, I don't think we'd gain anything from it and I'm not convinced that having code in 4 packages makes it more readable than 1 package.

In particular stack unwinding, variable evaluation, call injection and BinaryInfo all work together (call injection happens during variable evaluation which can't work without a stack frame context and BinaryInfo, and it's hooked pretty deeply inside the Continue loop). Next and step use breakpoints which use variable evaluation for their conditions.

I think the recent ports to freebsd and arm64 support this estimation since they mostly touch the subpackages of pkg/proc and leave mostly alone pkg/proc itself. The exception are:

1. the disassembler, where we could do better
2. implementations of arch

So Arch could be another target, maybe, but I suspect that will be painful to do.

I don't like putting code inside service/debugger because it's inside the service directory, but if it was pkg/debugger that'd be fine.

[derekparker]

I think your assessment is spot on and I think this discussion has lead to the idea that maybe instead of moving logic to service/debugger perhaps we have a new concept that sits in between pkg/proc and service/debugger. Aside from what we both agree should be moved out into independent package, I also agree some of the concepts make sense staying coupled for now, as you mentioned, however I think pkg/proc is the wrong place for them still. Maybe we introduce a new concept such as pkg/debugtarget (for lack of a better name, I'm definitely open to suggestions). That new concept will hold some of the higher level logic that pkg/proc currently handles. What do you think about that?

[aarzilli]

think your assessment is spot on and I think this discussion has lead to the idea that maybe instead of moving logic to service/debugger perhaps we have a new concept that sits in between pkg/proc and service/debugger

Ok.

Maybe we introduce a new concept such as pkg/debugtarget (for lack of a better name, I'm definitely open to suggestions).

We're going to type it a lot so I'd like it to be something shorter than debugtarget. pkg/dbg? pkg/sym (because it's a symbolic debugger)?

[chainhelen]

Oh, is the new branch named introduce-debug-target related to this issue?