angr-management
angr-management copied to clipboard
Remove the cache for FunctionTableModel
This reverts commit b7b7bf7184ce3340191e493a673e3d1eb9b6b2b9.
Bug
Since this commit, if you rename a function in angr then the functions table does not refresh. To reproduce:
- Rename a function through the Pseudocode view
- Notice the functions table does not refresh
Open to changes to this PR.
Test Results
19 tests ±0 19 :heavy_check_mark: ±0 1m 44s :stopwatch: +11s 10 suites ±0 0 :zzz: ±0 10 files ±0 0 :x: ±0
Results for commit 8a4e073f. ± Comparison against base commit 7b1960e9.
:recycle: This comment has been updated with latest results.
The user experience without the cache is not great, see comparisons here https://github.com/angr/angr-management/pull/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 Function object within an ObjectContainer class.
@ltfish I think this change will require things to change in angr, since, again, things can be accessed from the kb, which will not trigger any known handler we have now. I don't have time currently to reimplement this cache with the correct handlers everywhere. However, I would say it is critical that the decompiler actually shows renamed function in the function tab. This is still on master where anyone using the decompiler sees no change to the function in the function tab.
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.