Thanks for the feedback! Trying to limit time spent at the computer during summertime, but I'll certainly fix the issues before the next meetup.
niedz., 4 sie 2019, 18:37 użytkownik Peter Bortas bortas@gmail.com napisał:
Hi Mateusz,
We had a long discussion about this, and the main reservation was if having one global debug object in master is the correct final solution. But no one had plans on an alternative solution right now, and we absolutely do not want to stand in the way of something that finally provides a new debugger.
So the compromise is this: Your changes should be merged, but if someone comes up with a better design this might be ripped out and replaced, don't consider that part a stable API.
Before it's merged, this should be done in your branch:
- Divide the changes into semantic commits, something like:
- One commit with the master changes
- At least one with the src/* changes'
- One with the Debug module
- set_debugger() should probably not be an efun.
- Most to all of the stuff in builtin.cmod could probably live in modules/_Debug.
- printf() within PIKE_DEBUG (or otherwise) should be fprintf() to stderr.
- In a PIKECLASS, CVARS are automagically initialised to zero, there is no need to manually do that in INIT.
- The introduction of low_low_backtrace seems completely redundant, there is already a flags argument to low_backtrace, and low_bracktrace already keeps a ref to the bp.
- f_cq___debug_backtrace seems unnecessary, backtrace(1) already exists.
- The preference would be to have a function in the master that returns the debugger handler instead of calling resolv in the master and then calling the result of resolve().
- The malloc on sizeof(svalue) line in interpret.c is broken and needs to just be removed. The code should be good if that line is removed. Then feel free to remove the TODO above.
- The change in low_mega_apply; s -> obj_name is a bugfix and is now commited separately on master.
- There is no need for an extra "stepping_mode" in struct thread_state, a new debug_flags can be used instead.
- There is no need for the program pointer in the struct debug_breakpoints, it's already in the program.
- Move low_enable_breakpoint and low_disable_breakpoint to program.c
TODO, but not required for merge:
- Change the comments in DebugAdapterProtocol into autodoc compatible syntax. More autodoc in general would be nice.
- Update the pike.1 man page with the new options.
- The low_get_offset_for_line API should return a set of offsets, since a 1:1 mapping is not guaranteed.
- low_get_offset_for_line is probably broken in regards to local variable information.
- The breakpoint handling is will be very slow, but we'll handle this in a better manner together.
- There is no need for the debug_breakpoints to be a double linked list since it's only traversed in one direction.
Regards,
Peter Bortas
On Sat, Aug 3, 2019 at 3:37 PM Mateusz Krawczuk krawczukmat@gmail.com wrote:
Previously, on Pike Debugger:
we've ended up with a working proof-of-concept debugger with a set of
cool features, including breakpoints, stepping, variable inspection, and an ability to debug a selected thread by firing SIGUSR1 at it (as a way ti debug multi-threaded programs). Unfortunately, the debugger branch got so far away from the master branch, that it became practically impossible to merge it.
I took some (a lot, actually) of my time and rewrote the code manually
on top of a fresh master. It's now available at 'mkrawczuk/debugger' branch in the main Pke repo (not the github mirror, as previously).
Could you guys please take some time on the meetup and review my last
commit on this branch (6386f2de4b1) and consider merging it into master? The core part for breakpoints is (except some minor TODOs) done, and there's much more work in the Debugger module. But having those changes inside master I won't have to worry about future merge conflicts similar to those we've experienced the last time.
As previously, the debugger is available only if Pike is compiled
--with-debug.
To try it out, you need an IDE that supports Microsoft's Debug Adapter
Protocol (Visual Studio Code is one of them, although there are plugins for other popular editors like emacs or vim).
run $ pike --debugger --debugger-wait=(n) (your-program.pike) where n is
a number of seconds the debugger will wait for a connection from the debug client, and your-program.pike is a program you'd like to debug.
If everything goes ok, the debug server will listen on port 3333.
Connect your debug adapter to it.
Voila! Although a little buggy, breakpoints and stack inspection should
work fine.
As I understand, commits e13e50ad and 710fa97fa are work towards proper
local variable inspection and setting in debugger. Previously we've relied on a different mechanism introduced in branch grubba/wop-local-variables-debug-info. I'll re-enable this feature basing on this mechanism in my free time.
I'll provide a more detailed documentation for this feature but what is
the most important for me now is merging the current code into master, and the upcoming meetup (which I'll miss, unfortunately) seems to be a perfect occasion for it. Let me know is there anything standing n the way.
niedz., 7 kwi 2019 o 12:44 Mateusz Krawczuk krawczukmat@gmail.com
napisał(a):
I've verified my hypothesis and made breakpoints in classes work with
commit [1].
The way I did it solves the problem, but I believe it should have been
done on a lower level.
I've also looked up the program cache and verified that there in fact
already is such a structure. I'll work on avoiding the necessity to double it.
[1]
https://github.com/mkrawczuk/Pike/commit/bbdf2c23fb536ef42f0d83d14af84a06849...
wt., 2 kwi 2019 o 14:55 Mateusz Krawczuk krawczukmat@gmail.com
napisał(a):
That's good news, looking forward to see the update!
My branch is a little specific - I've completely disabled the Hilfe
debugger, replacing it by the debug adapter. For this reason you need a DAP-compliant IDE to test on my branch. The examples I provided earlier should suffice, but if you have any problems running it - do not hesitate to ask.
Previously I expressed myself incorrectly. I did not mean class
methods (usually called static methods). What was on my mind were the methods, i.e. functions defined inside a class (nested program, in Pike terminology), that can be called on objects instantiating the given class.
You currently can't break on them. For example, given
class C { int a; void create() { a = 1; werror("%d\n", a); }
void f() { ++a; werror("%d\n", a); } }
int main(int arc, array(string) argv) { C c = C(); c->f(); werror("c->a: %d\n", c->a); }
It will not stop on breakpoints set inside C's create() and f().
Setting breakpoints inside main() should go fine though.
I did a little research and I might have a potential fix for it. Looks like nested programs are getting their own separate entry. So
all the data related to nested programs, like line offsets, are stored in a different object for each defined class.
What is done now in debug_breakpoint class's low_enable_breakpoint, is
calling low_get_offset_for_line only for one program object that is directly related to the file. What should be done instead, is checking all the programs originating from a given file, so the nested programs are also checked for the needed line offset.
So what I need is to get a list of program objects from a provided
file string so I can loop through it. It needs to be available in builtin.cmod.
Nothing comes to my mind on how to do it, any help is strongly
appreciated.
śr., 13 mar 2019 o 15:01 H. William Welliver III william@welliver.org
napisał(a):
That’s cool. I suspect that Parser.LR may require a bit of work to
make it nicer to build a parser of this complexity, but I think it would be worth the effort, as it would allow a lot of interesting information to be extracted from pike classes… dependency graphs, data to allow refactoring, various types of analysis, etc.
I’ll also take a look at your branch; I sort of gave up trying to
keep my branch in sync with some of the changes Grubba has been making to support live backtrace frames and such. I also have some changes to the thread object that permits setting of names on threads (very useful when debugging) and being able to break execution on a thread regardless of whatever it’s running, etc.
Bill
On Mar 13, 2019, at 8:48 AM, Mateusz Krawczuk krawczukmat@gmail.com
wrote:
CCing dozzie (aka Jarowit). He is proficient with parsers and is
eager to put some effort to carry this step on.
pon., 11 mar 2019 o 17:43 Mateusz Krawczuk krawczukmat@gmail.com
napisał(a):
What exactly is this hypothetical parser needed for? Can you give
more detail on what is needed?
I've been working on the debug adapter for a while now and made it
possible to work with DAP-compliant IDEs, like VScode. You can check it out at the 'debugger' branch of my Pike fork [1].
Breakpoints, stack trace and variable browsing seems to work pretty
well. I've also made it possible to attach to a running Pike process and set breakpoints at runtime. To make it possible I've introduced a program cache, which is string-program mapping. It exists in the master object and is updated every time a program is loaded. It is needed because the way we handle breakpoint initialization deferring is insufficient for this scenario. I wonder how does it affect memory optimization.
One major thing that is missing is setting breakpoints on class
methods. Any clues how to implement it?
[1] https://github.com/mkrawczuk/Pike/tree/debugger
czw., 7 lut 2019 o 19:14 H. William Welliver III <
william@welliver.org> napisał(a):
> > Update: > > I took a little time this morning and merged grubba's local
variable names branch with the debugger branch, and it looks pretty good.
> > Seems to me that the next piece of infrastructure that's needed
probably involves some sort of Pike parser. I've looked at the java parser a bit and I'm not really happy with it. Perhaps it would make sense to extend Parser.LR to work with Parser.C.Tokens and generate a pike grammar for use with it?
> > I looked a little bit at Parser.LR and it looks like some features
we'd want might not be complete (like %token) or that would make maintaining similarity with the existing grammar (like |). Does anyone have much experience using Parser.LR or might have some real examples?
> > Bill > > February 6, 2019 8:20 PM, "H. William Welliver III" <
william@welliver.org> wrote:
> > I think there’s a sense that the parts of the debug infrastructure
that run within the process under debug should be as lightweight as possible, and not be active unless the process is being debugged. In keeping with this line of thought, I think the in-process portion of the agent shouldn’t be fully featured, as that would imply that it would be parsing code for metadata like block locations and the like (for stepping over, etc). The —debugger option is intended to be the mechanism which activates the in-process debugging agent.
> If your concern is that connecting to a hilfe session is not an
ideal debugging tool, that is true, but it was never my intention for that to be the final state. Rather I had some code that served a similar purpose and it didn’t make sense to go off building a lot of infrastructure if the premise wasn’t workable.
> Something along these lines is what I had in mind: > Debugging Tool <— DAP —> Debug Agent <— DAP-lite —> Pike process
under debug
> Now, the debugging tool and Debug Agent could theoretically be a
single process, such as a command line debug utility along the lines of gdb, and it could possibly know how to start a pike process for debugging or could also just allow connecting to a running pike process which has the debugger enabled. This is basically how the Java debug agent works, and it allows local and remote debugging, which is pretty handy.
> Bill