Skip to content

fix: compute PF, ZF, SF after AAA and AAS instructions#2300

Open
huehuehuehueing wants to merge 1 commit intounicorn-engine:masterfrom
huehuehuehueing:fix/aaa-aas-flags
Open

fix: compute PF, ZF, SF after AAA and AAS instructions#2300
huehuehuehueing wants to merge 1 commit intounicorn-engine:masterfrom
huehuehuehueing:fix/aaa-aas-flags

Conversation

@huehuehuehueing
Copy link
Copy Markdown

Summary

  • helper_aaa and helper_aas only updated CF and AF, leaving PF, ZF, SF, and OF stale from the prior instruction
  • Real x86 CPUs consistently set PF/ZF/SF based on the masked AL result after AAA/AAS, despite Intel documenting them as "undefined"
  • The sibling helpers helper_daa/helper_das in the same file already compute these flags correctly

Fix

Compute ZF, PF, SF from result AL after AAA/AAS, using the same approach as DAA/DAS.

Test plan

  • Added test_x86_aaa_flags -- verifies PF, ZF, SF, CF after AAA with adjustment
  • Added test_x86_aas_flags -- verifies PF, ZF, SF, CF after AAS with adjustment
  • Both tests use a preceding INC to set PF to a conflicting value, confirming AAA/AAS overwrites it
  • Full x86 test suite passes (55/55)

helper_aaa and helper_aas only updated CF and AF in eflags, leaving
PF, ZF, SF, and OF stale from the prior instruction. While Intel
documents these flags as undefined after AAA/AAS, real x86 CPUs
consistently set them based on the masked AL result. The sibling
helpers helper_daa and helper_das in the same file already compute
these flags correctly.

This causes observable divergence when code branches on PF (e.g.
JP/JNP) after AAA, which occurs in real-world binaries.

The fix computes ZF, PF, SF from the result AL after AAA/AAS, using
the same approach as DAA/DAS. Two regression tests verify the correct
flag values.

Signed-off-by: Larry H <l.gr@dartmouth.edu>
@wtdcode
Copy link
Copy Markdown
Member

wtdcode commented Mar 10, 2026

Looks good to me. Could you send to dev branch?

@huehuehuehueing
Copy link
Copy Markdown
Author

Looks good to me. Could you send to dev branch?

@wtdcode Done: #2311

@huehuehuehueing
Copy link
Copy Markdown
Author

The failing CI pipelines are all due to 403/503 errors from whatever external resources you use, obviously rate limited. @wtdcode any updates?

@Antelox
Copy link
Copy Markdown
Contributor

Antelox commented Mar 24, 2026

The failing CI pipelines are all due to 403/503 errors from whatever external resources you use, obviously rate limited. @wtdcode any updates?

Hi, as @wtdcode already said, please change branch target to dev

@huehuehuehueing
Copy link
Copy Markdown
Author

huehuehuehueing commented Mar 24, 2026

The failing CI pipelines are all due to 403/503 errors from whatever external resources you use, obviously rate limited. @wtdcode any updates?

Hi, as @wtdcode already said, please change branch target to dev

@Antelox I commented earlier pointing to the PR that merges cleanly with dev. See #2311

Right here and above my comment re broken CI: #2300 (comment)

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants