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

Aweful RzAsm <-> RzAnalysis pointer sharing and Hexagon unify #4667

Merged
merged 21 commits into from
Oct 17, 2024

Conversation

Rot127
Copy link
Member

@Rot127 Rot127 commented Oct 13, 2024

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

Hacks

Attempt to share pointers between RzAsm and RzAnalysis without any RzArch.

The assumption made here is that RzAsm is always initialized. Also without any RzAnalysis. But RzAnalysis is never initialized without a companion RzAsm.
Hence a plugin requiring a common state, always accesses RzAsm->plugin_data, no matter if it was called from RzAnalysis or RzAsm.

RzAnalysis holds the file buffer. So if this one is present, the plugin should initialize a buffer around it. If it is not there, it should make a byte buffer around the bytes passed to the function.

This mimics future behavior of RzArch which will get either an IO or Bytes buffer.

Plugin configs

With this change there is also the need to refactor the plugin specific configs.
I designed them badly before. And they couldn't be used by a plugin without a static variable.

Plugin specific configs are hold now in a hash mpa in RzCore. The reason to have them separately and not in the RzCore->config is because plugins can be the owner of them. Meaning the config_set functions will pass a pointer with the plugin state. Not the core state. This is required if plugins have specific configuration options they need to know, but have no access to RzCore to get them.

It adds luckily only a few more lines to handle them in cmd_eval.c and for the auto-complete.

Hexagon changes

  • Moves the state from a static variable into RzAsm->plugin_data.
  • Always disassembles the context around an instruction to know the packet properties the instruction is in.

Test plan

For plugin configs
Otherwise: all green with fuzz-dist.

Closing issues

None. Probably opens a few more.

Because static values are no longer allowed, it has to be sure
that the 'user' pointer for the config value setter is always the plugin data.
So the plugin can change its internal state, according to the configuration value.
This was not possible before.
Also it adds a seperated hash map for plugin values. So they are logically more separated.
Also resolves the problems with not knowing what is a valid instruction and which one not.
Leads to way mor stability in decoding.
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are good. I am only in doubt about those hacks. Let's see what others say.

librz/core/core.c Outdated Show resolved Hide resolved
Copy link
Member

@wargio wargio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Hopefully this hack will go away with RzArch

Rot127 and others added 2 commits October 14, 2024 11:42
Co-authored-by: Giovanni <561184+wargio@users.noreply.github.com>
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but only as a temporary measure. We should not make a new release with this hack.

@wargio wargio merged commit 9e2de0d into dev Oct 17, 2024
57 checks passed
@wargio wargio deleted the dist-fuzz-hexagon-unify branch October 17, 2024 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants