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

Add backend capabilities #64

Merged
merged 4 commits into from
May 22, 2024
Merged

Add backend capabilities #64

merged 4 commits into from
May 22, 2024

Conversation

patacca
Copy link
Collaborator

@patacca patacca commented May 21, 2024

Add a property for the backend loaders to advertise the supported capabilities.
This is useful to catch immediately whether a registered feature has all the information needed from the backend to correctly work. An example of such a feature would be #63 that uses pcode that is not part of the official API

@patacca patacca requested a review from RobinDavid May 21, 2024 18:06
@RobinDavid
Copy link
Collaborator

Ok great! I hope the mechanism is not overly complex for future maintenance.

Just two small remarks:

  • there is no capability for instruction group, do we add one ? (it is a capability of a backend in the end..)
  • the newly created exception is never caught in the main qbindiff cli executable, which means using the tool can result in a Python stack trace. It would be nice to catch it and to exit nicely.

@patacca
Copy link
Collaborator Author

patacca commented May 22, 2024

* there is no capability for instruction group, do we add one ? (it is a capability of a backend in the end..)

Indeed I thought about it, right now the behavior is that instruction groups are mandatory and we have a enum InstructionGroups that all backend have to align to. We could make them optional and add them as a capability but we should still need to rely on the unified InstructionGroups, unless we want to have different capabilities for different group types (like CAP_GROUP_CAPSTONE, CAP_GROUP_IDA, ...) but I feel like it's not a good decision.
In the end I am in favor of making instruction groups optional as, right now, the IDA backend currently doesn't offer them.

* the newly created exception is never caught in the main qbindiff cli executable, which means using the tool can result in a Python stack trace. It would be nice to catch it and to exit nicely.

You are right, I am gonna fix this

@patacca
Copy link
Collaborator Author

patacca commented May 22, 2024

I've chenged a few things:

  • Added the try/catch clause in the CLI to gracefully exiting in case of an unsatisfied capability
  • Add PCODE and INSTR_GROUP capabilities. For now quokka supports both, binexport only INSTR_GROUP and ida none.
  • Adjusted the features that were affected by those capabilities
  • Rewrote the instruction group feature to address the recent changes in Fix the JumpNb feature #61
  • The PR passes the tests

@RobinDavid RobinDavid merged commit 4f0a69c into main May 22, 2024
@patacca patacca deleted the add_backend_traits branch May 23, 2024 14:08
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.

2 participants