Skip to content

x86: bound reads both of its operands - #3109

Merged
Rot127 merged 1 commit into
capstone-engine:nextfrom
phix33:x86-att-bound-access
Oct 8, 2026
Merged

Rot127 merged 1 commit into
capstone-engine:nextfrom
phix33:x86-att-bound-access

Conversation

@phix33

@phix33 phix33 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor
  • 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

Detailed description

The bound instruction is currently marks its register operand as written to but that is incorrect. BOUND only reads its operands: it compares the value in the register against a pair of bounds stored in memory. If the value is outside those bounds, it raises a #BR exception. It does not write a result anywhere.

This incorrect access information also causes confusing results between Intel and AT&T syntax:

$ cstool -d x32 "62 03" | grep -E 'access|Registers'
        operands[0].access: WRITE
        operands[1].access: READ
    Registers read: ebx
    Registers modified: eax

$ cstool -d x32+att "62 03" | grep -E 'access|Registers'
        operands[0].access: READ
        operands[1].access: WRITE
    Registers read: eax ebx

The fix is to mark both operands as read-only for the X86_BOUNDS16rm and X86_BOUNDS32rm entries in X86MappingInsnOp.inc and X86MappingInsnOp_reduce.inc.

The AT&T printer normally reverses the operand-access information because AT&T syntax generally reverses the operand order compared with Intel syntax. However, bound is unusual because its AT&T representation keeps the same operand order as Intel syntax. (enter, lcall, and ljmp behave similarly, but their mapping rows do not contain access information.)

Once both bound operands are correctly marked as reads, reversing their access information makes no difference because both values are identical. Therefore, there is no need to change the AT&T printer's general reversal behavior.

Test plan

Add tests to tests/details/x86.yaml for:

  • 62 03 using Intel syntax, which exercises BOUNDS32rm.
  • 62 03 using AT&T syntax, also exercising BOUNDS32rm.
  • 66 62 03, which exercises BOUNDS16rm.

The tests check that both operands are marked as reads and that regs_read contains the correct registers. All three tests fail on the current next branch. If either mapping row is reverted after the fix, only the tests that use that particular instruction form fail, confirming that both rows are covered. cstest cannot assert that regs_write is empty, so there is no direct test for the absence of the Registers modified output. Instead, this is indirectly covered by checking that the register operand is marked as read rather than write.

(Note that the corresponding entries in X86MappingInsnOp_reduce.inc were checked separately using cstool from a build with CAPSTONE_X86_REDUCE=ON because cstest does not link against that configuration).

Closing issues

The same two incorrect mapping rows also exist in v5 and need the same fix.

@codecov

codecov Bot commented Oct 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (next@992e6d7). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #3109   +/-   ##
=======================================
  Coverage        ?   61.09%           
=======================================
  Files           ?      243           
  Lines           ?    56756           
  Branches        ?    10063           
=======================================
  Hits            ?    34675           
  Misses          ?    16871           
  Partials        ?     5210           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Rot127 Rot127 left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@phix33 Please be aware that it is no longer permitted to generate PR descriptions with AI. See our CONTRIBUTING.md guidelines

I will request next time to rewrite it by hand.

@Rot127
Rot127 merged commit b8c68d4 into capstone-engine:next Oct 8, 2026
61 checks passed
@phix33

phix33 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Weirdly I did write it by hand. I'll try harder next time.

@Rot127

Rot127 commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Weirdly I did write it by hand. I'll try harder next time.

Sorry, I didn't mean to offend you.
I was assuming it was AI because the whole paragraph about AT&T syntax was irrelevant for the fix. Because if both operands are READ, then the order doesn't matter anymore.

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