eteran / edb-debugger

edb is a cross-platform AArch32/x86/x86-64 debugger.
GNU General Public License v2.0
2.68k stars 322 forks source link

WIP: Session manager update #763

Closed Vic063 closed 3 years ago

Vic063 commented 4 years ago

These commits enable a better support of the Session Manager by now saving:

This is a WIP patch as I don't know if my code/modifications fit your needs; do not hesitate to tell me what's wrong and how I can improve this patch.

On a side note, I wanted to know if it won't be better, instead of saving the fixed address of the comment/label/breakpoint, to save the relative address of the comment/label/breakpoint with the name of the module so the restore process should correctly work even if base addresses change at each process restart (with ASLR for example). What do you think about this idea?

Thanks.

eteran commented 4 years ago

A lot of activity on this PR! Awesome, let us know when you think it's ready for review :-)

Vic063 commented 4 years ago

Ok, I was thinking about a new strategy to handle a session loading/saving that I started to implement before going to a place without any technology (holidays).

The big 'problem' I encounter right now is how to properly save the breakpoint state, which would require to rewrite a lot of stuff I think.

eteran commented 4 years ago

@Vic063 can you elaborate a bit more on the difficulties you have with the current design?

Any plugin should be able to save and load more or less arbitrary data. It just needs to be serialized into something that the session manager can use.

Vic063 commented 4 years ago

@eteran here is my approach to handle session 'objects' (labels, comments, breakpoints, etc...): https://github.com/Vic063/edb-debugger/commit/691532d7077596a92b14cdaa0d14da8c3e062003

A session object can have different kind of type but for the session manager, it is always treated as a 'SessionObject'. An object is always linked to a module, has a relative address and custom properties according to its type. The linked commit shows an example of my approach with labels and comments (I did not have time to implement breakpoint type).

Here is my attempt to write it down on a paper, using my poor UML knowledge: 20-05-29 09-58-21 0075

When the session file is saved, it looks like this: { "id": "edb-session", "objects": { "0000000000001090": { "comment": "Libc entry point", "module": "/lib/x86_64-linux-gnu/ld-2.28.so", "type": "comment" }, "00000000000010c5": { "comment": "Test Libc2", "module": "/lib/x86_64-linux-gnu/ld-2.28.so", "type": "comment" }, "0000000000001e90": { "label": "Label1_Test", "module": "/lib/x86_64-linux-gnu/ld-2.28.so", "type": "label" }, "000000000000f400": { "label": "Label2_Test", "module": "/lib/x86_64-linux-gnu/ld-2.28.so", "type": "label" } }, "plugin-data": { "BookmarksPlugin::Bookmarks": { "bookmarks": [ ] } }, "timestamp": "2020-05-28T20:24:49.880Z", "version": 1 }

And in the rendering in EDB: screenshot_edb

I don't know if it's clear to you?

eteran commented 4 years ago

@Vic063 Sorry for the delay in response. I suppose my question is, can't you implement that structure using the current API? Can't you just have the session manager or a given plugin save an "objects" key and have whatever dictionary you like under it?

Am I missing something?

eteran commented 4 years ago

That being said, I don't object to this, or any other sensible data format we come up with :-)

Vic063 commented 4 years ago

@eteran In my mind, I thought to a mechanism where the Session manager does not have to know which object it has to save/restore, it is completely transparent to it.

On the other side, the intelligence resides in the object itself, which has its own saving/restore mechanism.

Using this method, when declaring a new object that needs to be saved, it would require only 2/3 lines/methods to add to the session manager and the whole logic would be written in a separate class file. I think it might decrease the risk of breaking the saving format and simplify the task when someone want to add or modify a session object. Moreover, one of the issue I encountered (if I remember correctly) when I wanted to properly implement breakpoints saving, is the 2-steps saving update (here is what is currently done) :

  1. First, we create the breakpoint to the given address, it is saved in the session manager
  2. We want to update the breakpoint to add a break condition, it is not saved, and to save it, we have to find the breakpoint in the session manager remove it and save it again with its new property (the break condition)

With what I purpose, the breakpoint is a specific object that can listen to some signals, like the 'update' one, which can be triggered when adding/removing a condition or something else. This would not require to manipulate the session manager until the debugger is closed.

My approach might be a little bit tricky to understand, but I think this is due to the fact that I don't handle comments/labels/breakpoints like specific plugins, but like objects that are strongly attached to the debugger. In my mind, a plugin is something we can use or not according to our usage and I can't imagine using a debugger without comments/labels/breakpoints :)

eteran commented 4 years ago

@Vic063 Sorry for the delay in response.

I think I can see why the design you propose has some merit, where if I understand you correctly, objects themselves are "savable" and "restorable". But I'm not 100% sure why we couldn't achieve breakpoint save/restore with the current design.

I'm not against a new design, but I feel like I'd have to really understand it to want to switch to it.

While I can see how objects that know how to save/load themselves would make them easy to work with. It also tightly couples them to the SessionManager concept in the design. Where I already don't like that we've resorted to a singleton :-/. I certainly don't like the idea of having to pass a session object to each breakpoint constructor. And having them access a hidden global in their constructor feels... yucky too.

So with that said, I think I should explain a few goals of the current session manager.

  1. plugins should kinda have their own namespace. If they want to save/restore something, it's trivial for them to get a copy of what they chose to save, without concern of their keys/values stomping over other plugins.

  2. anything should be in principle, able to be saved in a session by a plugin. To accomplish this, a plugin could serialize any data they like, be it breakpoints, comments, etc.. into a string (JSON works well), and then they can just say "store that with this key". Later, they'll be given an opportunity to load stuff with that key.

  3. I'm not against adding some core concepts to the "global" session namespace, we've already done similar with comments! Why can't we just add things like breakpoints in a similar way? I'd also be just fine with reworking the session system comment API too if that makes sense. Because as you point out, some things are kinda universal for a debugger.

I'd love to hear more of your thoughts on this though! One way or another, we'll sort out what the best approach is!

eteran commented 3 years ago

@Vic063 I'm closing this PR due to lack of activity. But take heart, We are currently discussing moving breakpoint management out of plugins and into the core binary because it is so fundamental as you point out.

Please re-open, or open a new PR if you want to continue efforts on this.