Skip to content
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

Fix and improve vc4 plugin #337

Merged
merged 3 commits into from
May 11, 2024
Merged

Fix and improve vc4 plugin #337

merged 3 commits into from
May 11, 2024

Conversation

graux-pierre
Copy link
Contributor

Checklist

  • Closing issues: VC4 compilation error #325
  • Mark this if you consider it ready to merge
  • I've added tests (optional)
  • I wrote some documentation

Description

Hello,

Theses commits translate the vc4 plugin that is no longer compatible with radare2 libr (see the above issue) and add analysis information. They also remove previous warnings. ESIL information remains empty. The plugin has been tested using r2pm and corresponding modifications for radare2-pm repository are ready (here) because the make command has changed.

This pull request is marked as draft because it has the following flaws:

  • retrieval of the registers and immediate values used in an instruction is done manually, as for the blackfin architecture (libr/asm/p/arch_blackfin.c). That is, the instruction strings itself is parsed. This is of course sub-optimal but is way easier than digging into the CGEN code of the architecture to retrieve the required information.

  • The following code, that seems to be frequently used in vc4 binaries, isn't handled: the target of the branch isn't detected. I'm not sure if it's because analysis information isn't enough and constant propagation requires ESIL information.

mov reg, immediate
b reg
  • The following code, that also seems to be frequently used in vc4 binaries, isn't handled. I have the same interrogations regarding the ESIL information. In this case, I could mark all pop pc instructions as return instructions but it's too restrictive.
; start of the function
push lr
...
pop pc

Best,

@sylvainpelissier
Copy link
Collaborator

Thank you for the contribution. Can you reenable the plugin build in the CI https://github.com/radareorg/radare2-extras/blob/master/.github/workflows/ci.yml please?

@trufae trufae merged commit 26d8b75 into radareorg:master May 11, 2024
4 checks passed
@trufae
Copy link
Contributor

trufae commented May 11, 2024

Love it! Thanks for the fixes! Let’s get them merged before enabling it in the ci. So we can have some extra testing later on.

@graux-pierre
Copy link
Contributor Author

Thank you for the merge! Pull requests for the CI are made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants