search

gaasedelen / lucid

An Interactive Hex-Rays Microcode Explorer
MIT License
548 stars 50 forks source link

[Lucid Reloaded]: New features, QoL updates, fixes + enhancements #5

Open Fireboyd78 opened 10 months ago

Fireboyd78 commented 10 months ago

Originally this started out as a personal side project of mine that I didn't expect to do much work on. But just like that, before you know it.. I ended up rewriting a majority of Lucid and majorly improved performance! Lucid is one of my most-used plugins in IDA, so this is my way of showing appreciation to @gaasedelen for his amazing work :)

Considering I never planned on doing this much work, there may be some changes that are not listed below.

Changelist Pt. I

Changelist Pt. II

Changelist Pt. III

Fireboyd78 commented 10 months ago

https://github.com/gaasedelen/lucid/assets/2906983/1a434f6e-59ae-4007-a673-772bab308a98

https://github.com/gaasedelen/lucid/assets/2906983/b6ebbcb9-3448-45be-ab52-f9828a284c67

https://github.com/gaasedelen/lucid/assets/2906983/61ea079f-26f3-4c8a-b305-38aa664521b5

gaasedelen commented 10 months ago

This is an awesome PR! Thank you for the kind words, I am glad the plugin was so impactful towards your work that you found yourself actually digging in to extend it.

I honestly can't believe it has been 4 years since I've looked at this. If anything, you are probably more of an expert on both Lucid and IDA microcode than I am at this point. I barely remember writing half of the code behind this thing 😂

Luckily, you have caught me at a good time. I am trying to make a basic 'servicing' pass on all my old tools/plugins and release a basic bugfix/compatibility update for everything so this has come in at a good time.

Two things have caught my eye though:

I see some Python 3-isms (eg ":=") in the PR, so I assume this is going to break Lucid's Python 2 compatibility. I don't blame you, it's 2024 but I think there is still a reasonable contingent using IDA 7.7 + Py2 (hopefully, not for much longer...)

But it'd be nice if we could put out just one last update of Lucid (eg, 0.2) with all your fixes / perf updates for anyone that may be holding out a few more years on older environments.

In a month if you come back with another large feature release / update (eg 0.3) I'm happy to draw the line of supporting only IDA 8+ and Python 3.6+ or so then (as that's what I want to do for all my tools).

(PS: Could you clarify what version of IDA / Python you are using right now?)

I saw the TODO where you mention the cursor sync being broken between maturity layers - how broken is this? Or how hard do you think it will be to restore that functionality? I'll check it out and see how bad it is when I have some time in the next week or so but I figured I would call it out now.

I remember spending quite a bit of time on the cursor 'projection' (?) across the layers, I always thought it was kind of magic to watch the code melt and re-constitute in-place while swapping layers. It was pretty central to my vision of the plugin hehe, so I'd like to keep that working best we can.

Thanks again for the PR and I am sure many people will come to appreciate your contribution once we get it merged in.

Fireboyd78 commented 10 months ago

This is an awesome PR! Thank you for the kind words, I am glad the plugin was so impactful towards your work that you found yourself actually digging in to extend it.

Of course! I've actually been inspired to write my own decompiler hook plugins because of your tool. It helped me visualize the great unknown behind how the Hex-Rays decompiler works internally, and I've even made attempts to fix a lot of the bugs that are present because of how it aggressively optimizes existing microcode ☠️ so I definitely have you to thank for that! 🙏

I honestly can't believe it has been 4 years since I've looked at this. If anything, you are probably more of an expert on both Lucid and IDA microcode than I am at this point. I barely remember writing half of the code behind this thing 😂

I've got several projects of my own I haven't touched in years either. I feel your pain, lol. And yeah, I kinda am an expert on microcode now.. I was waaay too obsessive about it, and fell down the rabbit hole big time. I quite literally couldn't have done it without Lucid! 👌 🔥 💯

I see some Python 3-isms (eg ":=") in the PR, so I assume this is going to break Lucid's Python 2 compatibility. I don't blame you, it's 2024 but I think there is still a reasonable contingent using IDA 7.7 + Py2 (hopefully, not for much longer...)

Yes, I personally don't see a point in continuing support for Python 2 as my PR doesn't really add much more to Lucid than what was previously available. As much as I'd like to add Py2 support, there is a good chance that not many people are running it anymore anyways. They have a good working version of Lucid in 0.1.1 already 😄 if someone wishes to backport my code to be Py2 compatible, they're more than welcome to do so just as I contributed.

In a month if you come back with another large feature release / update (eg 0.3) I'm happy to draw the line of supporting only IDA 8+ and Python 3.6+ or so then (as that's what I want to do for all my tools).

If there were to be a v0.3, it would likely be something that is a complete and total rewrite of the existing system. I had to do a LOT of work to even feel comfortable bumping it up to 0.2 🤣

(PS: Could you clarify what version of IDA / Python you are using right now?)

I developed this using IDA Pro 7.6 SP1 + Python 3.8.6. Aside from the walrus operator being sprinkled about (and type hinting to aid in development), it should be 100% compatible as it was before.

I saw the TODO where you mention the cursor sync being broken between maturity layers - how broken is this?

It simply refuses to work anymore, period. Which is very strange. I've tried every which way to get it working, but there's a fundamental flaw regarding the view cursor system in the way it was implemented. It always expected that every maturity level would be loaded at once, which is why Lucid could take upwards of a minute or more to display the microcode. I did my best to understand your code, but it's also frustrating having to close IDA and reopen it just to test little changes here and there, lol.

I wish your reloading code had been more helpful for me, but sadly it just doesn't play nicely anymore I guess. Probably worked fine for older versions of IDA, but not for me.

...how hard do you think it will be to restore that functionality?

Depends on how easy it is to rewrite the logic from relying on the need to have all maturity levels loaded at once, to where it simply calculates from one maturity to the next. I honestly don't fully understand what all is happening, even with all of the comments there out in the open.. lol

I remember spending quite a bit of time on the cursor 'projection' (?) across the layers, I always thought it was kind of magic to watch the code melt and re-constitute in-place while swapping layers. It was pretty central to my vision of the plugin hehe, so I'd like to keep that working best we can.

I'm gonna be completely honest with you.. your code really was/IS magic. I have no clue how you came up with what you did, but it was incredible what you cooked up. My brain doesn't understand how it even works in the first place. All that's left to do is figure out how to calculate projections with the assumption that not all microcode text is available at once, and that they will "stream" in as the user changes maturity levels.

UPDATE: The cursor synchronization and reloading issues have been resolved! 😄

Hopefully that answers your questions! It's neat how I caught you at just the right time, because I've been meaning to get around to this for a good long while, and it only took me the last 2 weeks to get it done. Very cool how we lined up revisiting stuff together!

Fireboyd78 commented 9 months ago

@gaasedelen Just wanted to make sure you were aware of the latest fixes I've implemented 👍

gaasedelen commented 9 months ago

Thank you for addressing some of my feedback. I spent a few days getting the lighthouse release up to snuff and pushed out. This is next on my list.

I look forward to testing this and getting it merged in hopefully this week.

gaasedelen commented 9 months ago

I spent some time tonight getting your branch setup (on 8.3) and just trying to re-familiarize myself with the codebase.

After looking at how extensively your changes rely upon the python walrus, I don't think it makes sense to try and go back to supporting Python 2, but I personally had to actually update my IDA from using Python 3.6 up to 3.8 to test these changes. Not a huge deal as this is kind of a more niche plugin and not really under active development, but it's important to be cognizant of the stated compatibility when making contributions. I know some IDA people on Python 3.4 even :-)

That said, I started doing some basic formatting cleanup / tiny refactoring. Honestly so much has changed that it's going to be hard for me to really go through and give explicit feedback or understand why much of it was necessary -- but that's not a bad thing. I trust your judgement, it's clear you have given this plugin more time and attention than I have the past several years.

I love the live subtree graph changes, great work. I love how alive it feels. I think I mostly tacked on the subtree view last second in the original release so this is an obvious improvement.

The cursor projection code was probably mind melting because I'm pretty sure I literally spent weeks on making the cursor mappings 'feel' natural or intuitive based on the direction of the motions (up or down layers), the boundaries of the window, the type of selection, etc. I wanted the end user experience to be magical, but I enjoyed your sentiment that the algorithm was equally mind boggling. I studied lots of weird edge cases and motions that probably were hard to explicitly document.

While your work on trying to repair the cursor sync does appear mostly working, there are some issues still:

cursors

It looks like there's a bug somewhere in the tracking of X calculations for where to place the cursor between layers. And that's causing it to lose track or maybe switch targets entirely? Lots of cases I see it resetting the X position to the start of the line...

I strived really hard in the original implementation to maintain the cursor position within the individual text cells/tokens (the same X offset within a token, even) while doing my best to also keep things visually on the same Y / line. This is from the original release / Readme, in contrast to the above:

old

Perhaps another important point to stress about the design decisions that went into the cursor projection was that scrolling up and down through all of the layers without actually moving your cursor should always bring you back exactly on the same path (cursor locations) you ride through the layers.

When the projections would knowingly become ambiguous (which honestly was not too often tbh, I think it was mostly cases that would get optimized away) the cursor would change from green to red.

Removed assertion preventing moves from far-away maturity levels

As originally stated in the comments, it wasn't strictly necessary, and the code works just fine without it 👍

For context, I think part of the reason I enforced this was precision of mappings / cursor projection. By forcing my algorithms to compute projections from one layer to the next you implicitly had a more natural evolution (small steps) of the cursor. At least maybe it reduced the number of 'hard decisions' I had to make, can't remember!

Thanks again for your contributions and consideration of my feedback.

gaasedelen commented 9 months ago

Oh also RE: hot reloading -- all that is highly experimental and only used for dev/testing. When you are making massive changes per much of this PR, creating new classes, moving stuff around -- reload definitely will not work in those cycles.

But If you are just tweaking a few lines within functions, tracking down bugs, updating calculations, working on UI tweaks (or.. the cursor projection!) reloading usually works. I wouldn't worry about trying to ship reloading perfectly cuz it never will. It's just a secret development helper.

Edit: my patching plugin also has hot reloading and might be a slightly more refined implementation as it was developed after lucid, but probably not by much.

Fireboyd78 commented 9 months ago

My apologies for not realizing that the Python version used in IDA can differ from 3.4 - 3.8, that is definitely something I did not account for. I wouldn't mind getting rid of the walrus operator to help have better 3.x support.

I guess I'll need your help with fixing the cursor projection this time around, since I've not managed to fully fix it despite my best efforts.

Let me know what you'd like me to do to help, or if there's any information you would like to know about. Most of my code refactoring was just reducing the sheer amount of copy-pasta, and making better use of generators instead of initializing entire lists that, in some cases, would only use the first element and discard the rest.

My primary focus was to improve performance, which I achieved quite well. Now it's just a matter of fixing the cursor projection issues 😅

Fireboyd78 commented 9 months ago

Oh also RE: hot reloading -- all that is highly experimental and only used for dev/testing. When you are making massive changes per much of this PR, creating new classes, moving stuff around -- reload definitely will not work in those cycles.

But If you are just tweaking a few lines within functions, tracking down bugs, updating calculations, working on UI tweaks (or.. the cursor projection!) reloading usually works. I wouldn't worry about trying to ship reloading perfectly cuz it never will. It's just a secret development helper.

Edit: my patching plugin also has hot reloading and might be a slightly more refined implementation as it was developed after lucid, but probably not by much.

I figured the unloading portions of code I added were simply expanding upon the Core's ability to unload itself and unhook from everything. I'm actually quite happy with how it came out, because it works seamlessly and will actually reload all of the code safely. Makes debugging extremely easy and intuitive.

Fireboyd78 commented 9 months ago

OH! I just realized it's regenerating the cursor projection for all of the cursors every time you change the maturity level. It should not be clearing any of the old cursors that have already been initialized. I think addressing that will fix the issues we're having.

nyx0 commented 4 months ago

This PR looks amazing @Fireboyd78, would it be possible to have access to your 0.3 branch @gaasedelen? I would love to test and report bugs if any or even create a PR. Cheers.