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.

Sign up for GitHub

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

Jump to bottom

x86 FXSAVE & FXRSTOR support #2511

Merged
merged 19 commits into from
Feb 4, 2022
Merged

x86 FXSAVE & FXRSTOR support #2511

merged 19 commits into from
Feb 4, 2022

Conversation

Boyan-MILANOV
Copy link
Contributor

This PR adds support for the FXSAVE and FXRSTOR instructions

@Boyan-MILANOV
Copy link
Contributor Author

The emulation tests are currently failing because the new x87 registers that this PR adds are not supported by unicorn. I opened an dedicated issue to ask about this.

@Boyan-MILANOV
Copy link
Contributor Author

Note to myself: when merging this PR we should open a new manticore issue that reminds us to track unicorn releases so that when they merge the support for the additional floating point registers (FIP, FDP, ...) we can stop ignoring them in the fallback emulator and include them in the manticore <-> unicorn context switch.

@Boyan-MILANOV
Copy link
Contributor Author

This is finally ready for review!
I apologise for the terrible commit history on the PR - it's mostly due to me testing using Github's CI before figuring how to run it locally.

Summary of this PR:

  • Adds support for FXSAVE and FXRSTOR x86 instructions
  • Adds support for reading floating point registers as bitfields. For Unicorn-Engine compatibility reasons, Manticore represents floating point registers as tuples (mantissa,exponent). However in order to write FP registers to memory (as done in FXSAVE) we need to translate the floating point tuple value into actual bits. I have added the X86CPU.read_register_as_bitfield() method for this purpose, which is a simple wrapper around read_register() that packs FP tuple values to a 80-bit python int if needed
  • Adds a simple test that checks that doing FXSAVE -> clobber CPU -> FXRSTOR restores the CPU registers properly

@Boyan-MILANOV Boyan-MILANOV marked this pull request as ready for review January 26, 2022 15:23
ehennenfent
ehennenfent approved these changes Feb 3, 2022
View reviewed changes
Copy link
Contributor

@ehennenfent ehennenfent left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks good! I appreciate the way you encoded the layouts as tables so we'll be able to easily tweak them later. I was almost certain that FOP being 11 bits was a typo, but upon googling it turns out that x87 is just weirder than I expected.

@Boyan-MILANOV Boyan-MILANOV merged commit bf2fba3 into master Feb 4, 2022
@Boyan-MILANOV Boyan-MILANOV deleted the add-fxsave-x86 branch February 4, 2022 10:08
@Boyan-MILANOV Boyan-MILANOV mentioned this pull request Feb 4, 2022
Open
Boyan-MILANOV added a commit that referenced this pull request Feb 10, 2022
@Boyan-MILANOV
x86 FXSAVE & FXRSTOR support (#2511)
504ec9c 
* Add SMT simplifications for bitvec subtraction

* Add X86 support for FXSAVE and FXRSTOR

* Unicorn emulator: ignore floating point registers than aren't yet supported in unicorn

* Emulator: Also ignore MXCSR_MASK register

* Add logic to translate floating point registers values from (mantissa,exponent) to bitfield
Boyan-MILANOV added a commit that referenced this pull request Feb 15, 2022
@Boyan-MILANOV
Revert "x86 FXSAVE & FXRSTOR support (#2511)"
932ed0c 
This reverts commit 504ec9c. It seems that FXSAVE/FXSTORE on symbolic data causes or triggers some bugs in Manticore in the MATE tests, so we revert the commit until we figure out the root cause of the issue.
@ehennenfent ehennenfent mentioned this pull request Feb 17, 2022
Merged
ekilmer added a commit that referenced this pull request Mar 3, 2022
@ekilmer
Merge branch 'master' into capstone-5-dev
7589f48 
* master: (35 commits)
  Switch to stable Black (#2536)
  Fix typo in Manticore.linux constructor docstring (#2535)
  Revert CI changes made in #2526
  Release Manticore 0.3.7 (#2526)
  Optimise forking when there is only 1 solution (#2527)
  Epoll Implementation (#2529)
  Split off ENDBR32/64 from CHESS branch (#2533)
  Update to crytic-compile 0.2.2 (#2530)
  Also ignore missing unicorn registers in the fallback emulator (#2531)
  x86 FXSAVE & FXRSTOR support (#2511)
  Fix `BitVecExtract` simplification for constant folding (#2524)
  Add pip-audit action workflow (#2513)
  Add EXPLICIT fork policy (#2514)
  Enforce crytic-compile==0.2.1 (#2512)
  Improve namedtuple definition (#2506)
  Add SMT simplifications for bitvec subtraction (#2504)
  Fix handling of the program base address in Linux (#2500)
  Bump Sphinx version to 4.3.0 (#2503)
  Solver Improvements (#2502)
  Improves `namedtuple` definition (#2501)
  ...
ekilmer added a commit that referenced this pull request Apr 21, 2022
@ekilmer
Merge branch 'master' into chess
b762f7d 
* master:
  Switch to stable Black (#2536)
  Fix typo in Manticore.linux constructor docstring (#2535)
  Revert CI changes made in #2526
  Release Manticore 0.3.7 (#2526)
  Optimise forking when there is only 1 solution (#2527)
  Epoll Implementation (#2529)
  Split off ENDBR32/64 from CHESS branch (#2533)
  Update to crytic-compile 0.2.2 (#2530)
  Also ignore missing unicorn registers in the fallback emulator (#2531)
  x86 FXSAVE & FXRSTOR support (#2511)
  Fix `BitVecExtract` simplification for constant folding (#2524)
  Add pip-audit action workflow (#2513)
  Add EXPLICIT fork policy (#2514)
  Enforce crytic-compile==0.2.1 (#2512)
  Improve namedtuple definition (#2506)
  Add SMT simplifications for bitvec subtraction (#2504)
  Fix handling of the program base address in Linux (#2500)
ekilmer pushed a commit that referenced this pull request Apr 21, 2022
@Boyan-MILANOV @ekilmer
Revert "x86 FXSAVE & FXRSTOR support (#2511)"
19ee948 
This reverts commit 504ec9c. It seems that FXSAVE/FXSTORE on symbolic data causes or triggers some bugs in Manticore in the MATE tests, so we revert the commit until we figure out the root cause of the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehennenfent ehennenfent ehennenfent approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Boyan-MILANOV @ehennenfent