-
Notifications
You must be signed in to change notification settings - Fork 2
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 in v7m/v7em support #1
base: main
Are you sure you want to change the base?
Conversation
Added operatins to: - Count the number of ones. - Count the number of zeroes. - Count the number of leading ones. - Count the number of leading Zeroes.
I think it might be a good thing to consider changing the rotations into one single instruction /// General rotation or shift
Shift {
destination: Operand,
operand: Operand,
shift_n: Operand,
shift_t: Shift,
} Where Shift is one of #[derive(Debug, Clone)]
pub enum Shift {
/// Logical left shift
Lsl,
/// Logical right sift
Lsr,
/// Arithmetic right shift
Asr,
/// Rotate right with extend
Rrx,
/// Rotate right
Ror,
} as it would shorten the instructionset a bit, I have been using this in my implementations but if this is not how you want to go with it, I will refactor mine to reflect the old syntax. |
In regards to breaking out the general assembly operations into a separate crate, I have done it with the following structure .
├── Cargo.lock
├── Cargo.toml
└── src
├── condition.rs
├── lib.rs
├── operand.rs
├── operation.rs
└── shift.rs Thus leaving the Instruction type in Symex since that is the only place where the Instruction type makes sense. Reason for refactoringTo be able to use the inline transpiler we need a proc macro, as this requires a separate crate, we either get circular dependencies or break the common elements out into a separate crate. |
I think that it might be worth adding a debugging instruction Print {
info: &str,
operand:Operand
} to allow future implementors to debug their translations during runtime without modifying the Symex source. I have already implemented this but I am not sure if you want to include this in the project. If we add it, it might be worth adding a compile flag to the Symex source and the future general_assembly crate to disallow the use of the |
* Adds DIV/UDIV instruction. * Adds generic shift/rotation.
From my point of view this does not matter at all, and therefore do as you find best.
As a separate crate is needed for the proc macro this is a good idea I do not like however to split GA operations and GA instructions and propose that the intstruction definition too is moved to this crate to keep all of that coherent. And for this crate to always remain in tree to keep it in sync with the rest of symex. |
I can not find any realy good reason to not have this included at east until better tools for debugging GA instructions are created. |
Troubleshoot and correct errors + formatting
reverts all changes made to the v6 decoder, leaving it as it was
As there seems to exist some flaw in the implementation of the v6 or in my v7 implementation and you have no preference of which implementation is used I think it would be best to go with my general shift or rotation implementation but keep the previous ones as to not make to large changes in the v6 before troubleshooting it.
But I would recommend going with the generic instruction after troubleshooting is concluded. |
As for this, the reasoning behind leaving the Instruction in the main crate is that it contains a |
Saturating operations will be left as future work as it goes beyond the needed scope for now. |
I think that the transpiler should be included right in this repo instead of leaving it as a separate repo, what do you think of this? |
I think that that is a good Idea but possibly as a separate pill request. |
I will just download and verify that thing seem to work before I merge this. In all amazing work with all of this! |
Generally, I agree here, but as I depend on it for the v7em translation I think it might be a good idea to leave it as a git submodule and then in a separate PR merge that directory in its entirety. |
As we discussed briefly I included the transpiler in the repo directly. I also removed the LICENSE file from the transpiler (this is fine by me as it is my project) and it now uses the same license as this project for simplicity's sake. |
You are right about this the divide makes sense. |
Add in v7m/v7em support.
This PR aims to do a few things
!InITBlock
which need access to the executor state.Testing
The only testing done so far is the previous test suite using
cargo test
which still passes with flying colours.
This PR also adds some test coverage for the v7 translator but the testing is non-exhaustive due to a lack of time. My recommendation is to leave this as future work and do the same for testing the v6 translator.
Side-effects
This PR also applies
to the project and resolve the errors reported by Clippy. After this PR is merged we should do something like #2 to ensure that future PRs follow similar linting rules.