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

RzArch and plugins #4656

Draft
wants to merge 9 commits into
base: dev
Choose a base branch
from
Draft

RzArch and plugins #4656

wants to merge 9 commits into from

Conversation

wargio
Copy link
Member

@wargio wargio commented Sep 30, 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

Initial proposal and conversion of all the RzAsm/RzAnalysis plugins towards the unified RzArch* structures

Partially addresses #4334

@github-actions github-actions bot added the API label Sep 30, 2024
@wargio wargio force-pushed the fuzz-distro-arch-api branch from fc9dec1 to 24f9882 Compare September 30, 2024 19:20
librz/include/rz_arch.h Outdated Show resolved Hide resolved
@XVilka XVilka requested review from ret2libc, imbillow and Rot127 October 1, 2024 01:52
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.

One big thing is missing in here - a target description, e.g.

typedef struct {
    char *arch;
    char *cpu;
    ut16 bits;
} RzArchTarget;

This is where we can pack also the "features", e.g. particular ISA features or anything like that. Maybe also have a separate address_width member, e.g. see function rz_analysis_get_address_bits()

} RzArchPlugin;

typedef struct rz_arch_t {
RzPVector /*<RzArchPlugin*>*/ *plugins;
HtSP /*<char*, RzConfig*>*/ plugins_config;
Copy link
Member

Choose a reason for hiding this comment

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

Is it a plugin-specific configuration?

@XVilka XVilka requested a review from DMaroo October 1, 2024 02:18
Copy link
Member

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

Added quite some stuff which probably shouldn't be done in the first implementation.
I think necessary is though:

  • The RzArchPacket/RzArchInsn distinction.
  • Starting enums always with invalid = 0 to prevent false positives.

librz/include/rz_arch.h Outdated Show resolved Hide resolved
librz/include/rz_arch.h Outdated Show resolved Hide resolved
librz/include/rz_arch.h Outdated Show resolved Hide resolved
librz/include/rz_arch.h Show resolved Hide resolved
librz/include/rz_arch.h Outdated Show resolved Hide resolved
bool (*init)(RZ_NONNULL RzConfig *config); ///< Global constructor for the plugin to fill the configuration values.
bool (*fini)(); ///< Global destructor for the plugin
bool (*can_xcode_in)(RZ_NONNULL RzArchXCodeMember input); ///< Returns true if the plugin can support the given RzArchXCodeMember in input.
bool (*can_xcode_out)(RZ_NONNULL RzArchXCodeMember output); ///< Returns true if the plugin can support the given RzArchXCodeMember in ouput.
Copy link
Member

Choose a reason for hiding this comment

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

An can_xcode_transform would be nice. To test if the plugin supports a given in/out combination. Because can_xcode_in() && can_xcode_out() == true but context_xcode(in, out) could be unsupported.

Copy link
Member Author

Choose a reason for hiding this comment

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

i don't think this is an issue because we can define the transform, for example:
asm > details could become internally (in rzarch not plugins) asm > bytes > details.

@Rot127
Copy link
Member

Rot127 commented Oct 1, 2024

If we allow to query for specific properties via xcode(), we should implement also a common buffer mechanism for disassembled instructions of a plugin.
So the plugin doesn't need to disassemble everything again and again, just because the code asks for call refs, stack refs, asm_text in a row.

@Rot127 Rot127 force-pushed the fuzz-distro-arch-api branch from 86ee203 to 4da3252 Compare October 7, 2024 13:01
@Rot127 Rot127 requested a review from pelijah October 7, 2024 13:40
@XVilka

This comment was marked as resolved.

@Rot127 Rot127 force-pushed the fuzz-distro-arch-api branch from 4da3252 to 6594a29 Compare October 23, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants