-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove the cache for FunctionTableModel #1023
base: master
Are you sure you want to change the base?
Conversation
The user experience without the cache is not great, see comparisons here #853 How about triggering a cache update? |
@mborgerson I would if I knew how. When I first went for this fix, I triggered the cache to refresh whenever a Rename event happened. This will not work because there are many ways to change the function name. You can even do it programmatically as binsync does by just writing directly to the kb's functions (which may not be am containers). I am not sure I understand your mental model when you designed this cache since I don't know what you want to pipeline to get this event to trigger. If it's just AM containers being updated, we still need to catch all the programmatic ways to update names. I see removing the cache as the best alternative right now, even compared to lag. |
I think the best way is to manually trigger the refresh-cache event after programmatically setting function names. |
We can also wrap each |
@ltfish I think this change will require things to change in angr, since, again, things can be accessed from the I suggest this PR is merged, and we open an issue for #853 as a separate task. |
I think we should fix it in the right way. I'll give it a try tomorrow. |
This reverts commit b7b7bf7.
8a4e073
to
ab5ba2d
Compare
This reverts commit b7b7bf7.
Bug
Since this commit, if you rename a function in angr then the functions table does not refresh.
To reproduce:
Open to changes to this PR.