search

radareorg / r2retdec

RetDec plugin for Radare2
https://retdec.com/
MIT License
125 stars 24 forks source link

Patched with changes required after migrating RAnnotatedCode to radare2 #16

Closed NirmalManoj closed 4 years ago

NirmalManoj commented 4 years ago

This PR is from the radare2 team. We have migrated RAnnotatedCode used in retdec-r2plugin to radare2. In the future, RAnnotatedCode will work as the standard decompiler interface for all decompiler plugins in Cutter. We have a series of work going on for improving the decompiler widget. This PR contains necessary changes required to make retdec-r2plugin work with updated radare2.

NirmalManoj commented 4 years ago

It seems it checks out an old version of radare2 in the automated tests. That's the reason why tests failed.

xkubov commented 4 years ago

Hi, thank you for the PR. As you have pointed out, the reason why tests failed is that this plugin is tested on an older Radare2 release (4.3.1) which is missing the interface for RAnnotatedCode. So these changes won't work on older releases that are available from package managers of different Linux distributions.

Don't you think that we should maintain some kind of compatibility with older versions? I can think of a few solutions: we can fetch the latest radare2 sources in CMake as external project - but the r_annotated_code would need to be linked statically. Another solution would be to conditionally include r_util/r_annotated_code.h.

Do you think it would make sense to do it? Or we should change the requirements of the radare2 version to the newest one?

xkubov commented 4 years ago

I will merge this PR into branch win-support where I am preparing changes for the next release. I will test the options I mentioned above. And perhaps provide support for previous releases.

thestr4ng3r commented 4 years ago

Conditional include could work temporarily, however it will break at some point too if the API in r2 changes. What we do in r2ghidra is to always develop the master against r2 master and also run the tests against it. When r2 creates a release, we also tag r2ghidra's latest commit, so it is transparent which r2ghidra version is guaranteed to work with a certain r2 version, see https://github.com/radareorg/r2ghidra-dec/issues/92 On the other hand, this model also means previous r2 versions will not receive updates of the plugin.

xkubov commented 4 years ago

I agree, this solution sounds fine. I have merged this PR into master now with other changes and we'll tag commits with radare2 version supported so that installer could implement the logic of choosing appropriate commit.