Skip to content
This repository has been archived by the owner on Mar 15, 2021. It is now read-only.

Implement FMOV, MRS, MSR #3

Merged
merged 5 commits into from
Apr 22, 2020
Merged

Implement FMOV, MRS, MSR #3

merged 5 commits into from
Apr 22, 2020

Conversation

nshp
Copy link

@nshp nshp commented Apr 18, 2020

Only the same-size/non-converting form of FMOV is lifted, not the
rounding or extending forms.

Also fixed a NULL-deref of instr

I'm sure my C++ is atrocious, I hardly ever use the STL.

Copy link
Owner

@meme meme left a comment

Choose a reason for hiding this comment

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

Firstly, thank you very much for your work. I've got a few questions with your implementations and some C++-specific stuff but overall it's looking good

aarch64_extension.cpp Outdated Show resolved Hide resolved
@@ -275,6 +328,108 @@ class AArch64ArchitectureExtension : public ArchitectureHook {
return false;
}

bool LiftFMOV(cs_insn* instr, LowLevelILFunction& il) {
Copy link
Owner

Choose a reason for hiding this comment

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

This instruction has 3 variants, FMOV (register), FMOV (general) and FMOV (scalar, immediate). We should definitely be handling these cases (or at a minimum just the register and general case).

aarch64_extension.cpp Outdated Show resolved Hide resolved
aarch64_extension.cpp Outdated Show resolved Hide resolved
aarch64_extension.cpp Show resolved Hide resolved
aarch64_extension.cpp Outdated Show resolved Hide resolved
aarch64_extension.cpp Outdated Show resolved Hide resolved
@meme
Copy link
Owner

meme commented Apr 18, 2020

Please also add your new instructions to the README

@meme meme mentioned this pull request Apr 18, 2020
@nshp nshp force-pushed the master branch 3 times, most recently from 4056127 to a264e3e Compare April 20, 2020 02:43
@nshp
Copy link
Author

nshp commented Apr 20, 2020

I believe I've addressed all your comments except for the other FMOV variants. That's a tomorrow problem.

Only the same-size/non-converting form of FMOV is lifted, not the
rounding or extending forms.

Also fixed a NULL-deref of `instr`
@meme
Copy link
Owner

meme commented Apr 20, 2020

Just did some more STL clean-up, MRS is lookin' good in the HLIL view

@nshp
Copy link
Author

nshp commented Apr 22, 2020

I believe I've implemented the rest of FMOV, though I'm not 100% convinced all the cases are accurately lifted (esp. the truncating / extending / shifting). My understanding of fmov could be off.

Also note that the version of capstone pinned in the submodule here does not disassemble some of the instructions in the test file, but I tested those by bumping to capstone's next branch locally.

I'm sure this lifting logic could use some factoring, but that'll likely come more easily while implementing other floating point instructions in the future.

@meme
Copy link
Owner

meme commented Apr 22, 2020

Looks great. We'll get this merged and deal with Capstone soon.

@meme meme merged commit 3b3bbea into meme:master Apr 22, 2020
@meme meme mentioned this pull request Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants