Skip to content

Conversation

hainest
Copy link
Contributor

@hainest hainest commented Sep 10, 2025

This should have been part of #2680

Your checklist for this pull request

  • I've documented or updated the documentation of every API function and struct this PR changes.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

These are updates to the x87 comparison instructions that includes more detail on segment registers and FPU flags. It also fixes bugs introduced in #2680 that incorrectly removed FPU flag modifications from the fcomi family of instructions. Testing is substantially expanded.

...

Test plan

Tests are included.

Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

I am not very much into x86. But checking the ISA it seems to not match?
Do I mis-understand something?

@hainest hainest requested a review from Rot127 September 13, 2025 23:58
There is now one test case for each operand input for each opcode (e.g.,
DE/2 and DE/3).
@Rot127 Rot127 requested review from wargio, jiegec and kabeor September 15, 2025 12:57
@hainest
Copy link
Contributor Author

hainest commented Sep 18, 2025

@Rot127 Sorry about the late changes. I try not to do updates after a PR has been approved, but I discovered today that Chapter 5 of the SDM has a nice listing of all of the instructions broken down by category where I found fxam hiding in 5.2.3 X87 FPU Comparison Instructions.

Comment on lines +2459 to +2460
0xd9, 0xe4, # ftest
0xd9, 0xe5 # fxam
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just as advice. It is usually a little easier if the instructions are in a separated test case.
The cstest output currently doesn't report which instruction failed.
So if the instructions don't depend on each other, having them separated is simpler to debug..

@Rot127 Rot127 merged commit be9a4d2 into capstone-engine:next Sep 30, 2025
22 checks passed
@hainest hainest deleted the thaines/x87_comp branch September 30, 2025 20:45
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.

2 participants