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

x86: update read/write registers for transfer instructions #2578

Merged

Conversation

hainest
Copy link
Contributor

@hainest hainest commented Dec 7, 2024

This is mostly a self-serving update to simplify migrating https://github.com/dyninst/dyninst to Capstone. I the move to Zydis (#2505), but I need these updates sooner than that is likely to happen.

There are still missing implicit memory writes to the stack pointer for most of these instructions (e.g., ret pops the stack), but I won't add them here since that's more involved than just updating the instruction tables. Zydis does have that information.

@Rot127
Copy link
Collaborator

Rot127 commented Dec 8, 2024

Some tests of project/tests/details/x86.yaml failed.
You can use the Python cstest or the cstest tool written in C.

@Rot127
Copy link
Collaborator

Rot127 commented Dec 8, 2024

Just saw that I haven't fixed the build instructions for cstest yet.
Please see #2580 how to use it.

Essentially you can build cstest if you are on Linux (requires a libyaml-dev package installed and -DCAPSTONE_BUILD_CSTEST). Or you can use the cstest_py Python cstest.

Install with:

pip install bindings/python/
pip install bindings/python/cstest_py

Every form of the call instruction reads and writes the stack pointer
and program counter (instruction pointer).
In 64-bit context, this should read/write RBP and RSP but I couldn't
get that to work by just adding another version with the is64bit field
set.
@hainest hainest force-pushed the thaines/x86_transfer_insn_semantics branch from 6710599 to bc11b5e Compare December 10, 2024 22:58
@hainest
Copy link
Contributor Author

hainest commented Dec 10, 2024

Tests are now passing. I needed to update the expected results for call instructions (they now all write the IP).

$ ctest --test-dir build --output-on-failure
Internal ctest changing into directory: /capstone-engine/capstone/build
Test project /capstone-engine/capstone/build
      Start  1: MCTests
 1/13 Test  #1: MCTests ..........................   Passed    1.62 sec
      Start  2: DetailTests
 2/13 Test  #2: DetailTests ......................   Passed    0.02 sec
      Start  3: IssueTests
 3/13 Test  #3: IssueTests .......................   Passed    0.01 sec
      Start  4: FeaturesTests
 4/13 Test  #4: FeaturesTests ....................   Passed    0.00 sec
      Start  5: unit_cstest
 5/13 Test  #5: unit_cstest ......................   Passed    0.00 sec
      Start  6: integration_cstest
 6/13 Test  #6: integration_cstest ...............   Passed    0.04 sec
      Start  7: legacy_test_skipdata
 7/13 Test  #7: legacy_test_skipdata .............   Passed    0.00 sec
      Start  8: legacy_test_iter
 8/13 Test  #8: legacy_test_iter .................   Passed    0.00 sec
      Start  9: legacy_test_customized_mnem
 9/13 Test  #9: legacy_test_customized_mnem ......   Passed    0.00 sec
      Start 10: integration_compat_headers
10/13 Test #10: integration_compat_headers .......   Passed    0.00 sec
      Start 11: integration_test_litbase
11/13 Test #11: integration_test_litbase .........   Passed    0.00 sec
      Start 12: unit_sstream
12/13 Test #12: unit_sstream .....................   Passed    0.00 sec
      Start 13: unit_utils
13/13 Test #13: unit_utils .......................   Passed    0.00 sec

100% tests passed, 0 tests failed out of 13

@XVilka
Copy link
Contributor

XVilka commented Dec 14, 2024

@kabeor

Copy link
Member

@kabeor kabeor left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

@kabeor kabeor merged commit be6be78 into capstone-engine:next Dec 15, 2024
20 checks passed
@hainest hainest deleted the thaines/x86_transfer_insn_semantics branch December 16, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants