Lifting: Decode x86 16-bit addressing modes correctly by zardus · Pull Request #581 · angr/pyvex

zardus · 2026-09-06T15:47:50Z

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Problem

cmp dx, (%di) reads %bp. This is 3b 15 at 0xf9c98 in tests/i386/bios.bin.elf, a SeaBIOS image, lifted in the real mode a BIOS runs in — pyvex.IRSB(b"\x3b\x15", 0xf9c98, arch, opt_level=0, num_inst=1).pp() on a copy of ARCH_X86 whose vex_archinfo["x86_cr0"] is 0xFFFFFFFE:

IRSB {
   [ temp declarations elided ]

   00 | ------ IMark(0xf9c98, 2, 0) ------
   01 | t4 = GET:I16(ebp)
   02 | t5 = 16Uto32(t4)
   03 | t3 = t5
   04 | t2 = GET:I16(edx)
   05 | t1 = LDle:I16(t3)
   06 | t0 = Sub16(t2,t1)
   [ 07-13 elided: the condition codes and %eip ]
   NEXT: PUT(eip) = 0x000f9c9a; Ijk_Boring
}

Line 01 is ebp. The instruction names %di.

It does not fault, does not decline, and comes back Ijk_Boring with the right length. Of the 24 memory forms of a 16-bit ModRM byte, one is right, eleven read the wrong base register like this, and twelve come back Ijk_NoDecode with size 0; put a segment override in front and all 24 refuse. A refusal ends the block at zero bytes, so it gets noticed; a wrong base register does not. The bytes reported in 2017 at #80 still show it: 67 20 74 6f is and %dh, 0x6f(%si), and on master it lifts cleanly reading %sp. Only the sanityCheckFail that issue was closed on is gone.

Both routes in are live. 67 in 32-bit mode is one. The other has no prefix: libVEX sends every memory ModRM through this code when archinfo->x86_cr0 & 1 is clear, and angr sets that from a guest mov cr0 through engines/vex/heavy/dirty.py and engines/vex/lifter.py -- test_cfgemulated.py's CFGEmulated run over this file already takes that route four times. Over the 4,323 such instructions in that image's .text, 3,521 refuse, 687 compute the address from the wrong registers, 66 come out the wrong length, 2 emit no load or store to check, and 47 show no defect under those checks -- one of which takes the wrong immediate anyway.

Root cause

Entirely in the submodule. disAMode16 in priv/guest_x86_toIR.c passes the raw r/m field to getIReg, which numbers registers ax cx dx bx sp bp si di, while a 16-bit r/m field means (%bx,%si) (%bx,%di) (%bp,%si) (%bp,%di) (%si) (%di) (%bp) (%bx), so (%si) reads %sp and (%bx) reads %di. The base+index forms have no implementation at all, a negative 8-bit displacement fails mkU16's assertion, and a segment override reaches handleSegOverride as an Ity_I16. Separately, lengthAMode dispatches on the processor mode while disAMode dispatches on the address size, so a 67-prefixed instruction's immediate is read from the wrong byte — and lengthAMode16's own lengths are wrong for 17 of the 32 mod/rm combinations, which bites in real mode with no prefix: 83 3e 20 df 00, cmpw $0, 0xdf20, takes 0xffdf instead of 0.

Fix

Bump the vex submodule to angr/vex#97, which rewrites disAMode16 and lengthAMode16 around mod and r/m with the 16-bit table in one place, masks the displacement back to 16 bits, widens the effective address before the segment base is added, and points lengthAMode at current_sz_addr. That bump and tests/test_x86_addr16.py are the whole diff.

What a 16-bit address size means for the instructions that take no ModRM byte is deliberately still missing: MOVS, CMPS, STOS, LODS and SCAS address off %esi/%edi, LOOP and JCXZ count in %ecx, XLAT indexes off %ebx, the A0-A3 moffs forms take a 32-bit offset, and in real mode PUSH and POP address the stack through 32-bit %esp. That is a separate change.

Testing

tests/test_x86_addr16.py walks all 24 memory forms of the 16-bit ModRM byte at each of the three mod values and asserts that each reads exactly the registers the r/m table names and lifts to the right length; then the negative 8-bit displacement, the segment override, the immediate, and the real-mode route that needs no prefix, with the bare 16-bit literal address and an unprefixed 32-bit amode as controls. Five of its seven tests fail on the merge base and all seven pass here. Swapping only libpyvex.so under one fixed angr tree leaves angr's tests/analyses/cfg at the same counts with nothing failing on either side, test_cfg_6 -- the BIOS binary above -- included.

The file lifts byte strings rather than loading a fixture. Real objects do contain these encodings -- bios.bin.elf holds all 24 forms -- but only the ecosystem job has a sibling checkout. The four-leg Test matrix and Test (Pyodide) each take this repository and its submodule and nothing else, and pyvex has no conftest.py and no skip guard, so a test opening an angr/binaries fixture would not skip on those five, it would fail. No pyvex test loads one today. Validation: #581 (comment)

Merge order: angr/vex#97 first. This head's vex submodule is 5e25551, that pull request's head and one commit ahead of vex master, so merging this half alone would point pyvex master's submodule at a commit that is not on vex master. If it is squashed rather than merged, the submodule needs re-pointing at the squashed commit first. angr/vex is not in the CI repository list, so the sync: line below is a note to the reader, not an instruction to CI.

sync: angr/vex#97

🤖 Generated with Claude Code

session: sharpen

zardus · 2026-09-06T15:48:03Z

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Validation record for head 859613bb72436dd9e44f87a9bae3b23247c02d54 against baseline 2324a19e803d4141e7649a939e668b46ec27c3fb.

Rebased because pyvex master 2324a19 ("Support AVX512 lifting", #584) moved the vex submodule pointer to e3062871, which conflicted with this branch's own bump -- git merge-tree named vex and nothing else. Taking either side alone loses something, so the vex half moved with it: angr/vex#97 replays onto vex master as 5e255516e977171c07fb0bd45dcaf5c08b80596c, with git range-diff reporting its one commit unchanged, priv/guest_x86_toIR.c at blob 0c061974996a977d06594590f66782ab3bd99fa7 on both heads, and none of the 47 files the AVX512 import touched being that one. tests/test_x86_addr16.py is at blob 40a8d2925c7606f472843f9c1a99a0ad14c47231, the same as the previous head; the submodule pointer is the only published line that moved. The figures below were taken at those two revisions.

Regression: pytest --import-mode=append -q tests/test_x86_addr16.py -- 5 of 7 fail on the baseline, 7 pass on the head. The five are test_addr16_modrm_reads_the_right_registers, test_addr16_negative_disp8, test_addr16_segment_override, test_addr16_immediate_follows_the_16_bit_amode and test_addr16_in_real_mode_needs_no_prefix. The other two, test_addr16_bare_literal_address and test_addr32_is_unchanged, pass on both sides and are there as controls
Full suite: pytest --import-mode=append -q tests -- 88 passed and 5 failed on the baseline, 93 passed on the head. pyvex master's own tree at 2324a19 is 86 passed, so the seven added by this branch are the new file's
Lint/type: the repository's full pre-commit hook set clean over all files, no rewrite; run-ci-diff-checks.py, which reproduces angr/ci-settings' merge-base comparison, gives pylint 10.00/10.00 and pyright 0 errors on tests/test_x86_addr16.py
Guards: check-stale-pins.py --checkout <this tree> --rev HEAD exit 0; check-test-inputs.py -- "1 checkout(s) add no binary or manufactured input outside angr/binaries", exit 0
vex build: make -f Makefile-gcc -j 6 all on the rebased submodule head, exit 0, and no warning names guest_x86_toIR.c -- the two the build does emit are master's own, in guest_amd64_toIR_AVX512.c. gcc -c -DPYVEX -Ipub -Ipriv -Wall -Werror=implicit-function-declaration on the changed file alone, exit 0 on the head and on the base, with a file calling an undeclared function rejected by the same command as the control that the flag speaks. That check is here because the base move 8000 d by an import, which is where a clean replay can still stop compiling
Workspace gate: the complete local gate for this exact head, all fourteen suites selected. Thirteen passed, among them pyvex, cle, angr, archinfo, pypcode, pysoot, the Rust suite, the headless GUI suite, the full pre-commit hook set over every checkout, and the test-input and packaging checks. The fourteenth is this workspace's own harness suite, and it failed on six tests belonging to an unrelated internal tool that another job was editing while the run was in flight; the gate's worktree check named those same files and nothing else. Each of the six passes at that tool's committed revision, run one at a time, so neither failure is reachable from this change -- which touches one test file and a submodule pointer

Consumer check, and what it varies. The two arms are nix-built environments that differ in libVEX and nothing else: diff -rq -x __pycache__ over the whole of each one's site-packages reports four files -- pyvex/lib/libpyvex.so, angr/unicornlib.so, which links against it, and the two RECORD files that hash them. Every .py file is byte-identical. The -x matters and is not a convenience: without it the same command reports 3,284 differences, because a .pyc embeds the store path it was compiled under. Both arms give pytest --import-mode=append -q tests/analyses/cfg -n 4 = 238 passed, 9 skipped, 28 subtests passed, from angr at 23b470d9f. angr's tests/analyses/cfg/test_cfgemulated.py -k test_cfg_6, the CFGEmulated run over the BIOS image this change is about, passes on both arms.

The 24 memory forms of a 16-bit ModRM byte, 67 8b <modrm> [disp] at each of mod 0, 1 and 2, and the same again behind a 64 segment override. On the baseline, without an override, 1 is correct, 11 decode and read the wrong base register and 12 come back Ijk_NoDecode with size 0; with an override all 24 refuse. On the head all 24 are correct in both cases. 67 20 74 6f -- the reproducer from #80, closed in 2017 -- lifts Ijk_Boring size 4 on both sides, reading guest offset 24 (%sp) on the baseline and 32 (%si) on the head.

Exhaustive opcode differential. Every combination of the 0x67 prefix, the 0x0F escape, all 256 opcode bytes and all 256 ModRM bytes, followed by the fixed 12-byte tail 11 22 33 44 55 66 77 88 99 AA BB CC -- 262,144 lifts per side with ArchX86, opt_level=0, max_inst=1, compared on jumpkind, size and a hash of the statement list. Run in both processor modes, because which is in force decides whether this code is reached at all: the default x86_cr0 is protected mode, where only a 0x67 prefix reaches it, and x86_cr0 = 0xFFFFFFFE is real mode, where every memory ModRM does:

	Cases	Differing, protected	Differing, real
The byte `0x67` appears nowhere in the instruction	130,050	0	32,897
`0x67` present, in any byte position	132,094	42,962	1,722

The top-left cell is the no-regression control: in the mode where this change is not supposed to be reachable without the prefix, nothing without the prefix moves. In protected mode 84 of the 42,962 differences carry their 0x67 in the opcode or ModRM slot rather than as a prefix. The three cells that are not the control depend on the tail, which is why the tail is written out above: twelve 90 bytes instead gives 32,861, 42,892 and 166, in the same order as the table, and moves the 84 to 67. The control cell is 0 under both tails.

Real-mode census over tests/i386/bios.bin.elf, blob 4bbc57934f4e9c103585b3c32bb4072e0f9e708a -- the same bytes at angr/binaries fc07821c89535534979b02760e7e1bfc35faf690, at 0166109eb1fa0aec5baefb12403a487268f0e9ca, and at the 003e82a2 this record used to cite. Linear sweep in capstone CS_MODE_16 over .text (0xf9c10, 25,584 bytes), restarting one byte past each stall: 10,461 instructions, of which 4,323 carry a memory operand and no 0x67 address-size prefix, so they reach disAMode16 through current_sz_addr alone. Each lifted at its real address with vex_archinfo["x86_cr0"] = 0xFFFFFFFE, opt_level=0, max_inst=1. The rows are a partition, in this order:

Result	Baseline	Head
`Ijk_NoDecode`, size 0	3,521	470
Decoded, instruction length disagrees with capstone	66	66
Decoded, right length, no load or store in the IR to check	2	11
Decoded, right length, address computed from the wrong registers	687	57
Decoded, right length, address computed from the right registers	47	3,719

The prefix is read from capstone's decoded prefix slots, not by scanning the instruction's bytes for 0x67. Scanning gives 4,311 instead of 4,323: twelve instructions carry the byte as a displacement or an immediate and still reach this code
The register rows are a dataflow check, not a text match: the guest registers reaching the address temp of a load or store, followed back through the temp definitions, must be exactly the base and index capstone names. Only the eight general-purpose registers are counted
Per instruction, the two arms differ deterministically: 3,051 go from Ijk_NoDecode to decoding and 0 go the other way; 641 of the baseline's 687 address-register errors are corrected, so the head's 57 are the 46 that carry over plus 11 that the baseline refused outright and that now decode into that row; 0 instructions leave the last row
The last row does not cover immediates, and one of the baseline's 47 has a wrong one: 83 3e 20 df 00 at 0xfe05d, cmpw $0, 0xdf20, takes 0xffdf where the head takes 0

Caveats. tests/test_x86_addr16.py lifts byte strings; it loads nothing, so it runs on all five Test legs -- the four-OS matrix and Test (Pyodide) -- as well as in ecosystem / Test (N), not only where a sibling checkout exists. No corpus decompilation sweep was run: the snapshot corpus is x86-64 CGC binaries, which libVEX lifts in guest_amd64_toIR.c; this change is in guest_x86_toIR.c, so those objects cannot reach it with or without a 0x67 prefix and silence there would not be coverage. The angr/vex half's own numbers are in that pull request's record and are not repeated here.

zardus · 2026-09-06T15:48:05Z

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Three instructions from the .text of tests/i386/bios.bin.elf, a SeaBIOS image, lifted at their real addresses with ArchX86, opt_level=0, max_inst=1 and vex_archinfo["x86_cr0"] = 0xFFFFFFFE. That is the mode the code runs in on hardware, and the mode angr reaches through a guest mov cr0: angr/tests/analyses/cfg/test_cfgemulated.py's CFGEmulated run over this file makes 4 of its lift_vex calls that way, all at 0xfe29c; the rest are in protected mode, where only a 0x67 prefix reaches this code. That run makes 6,405 calls in total against angr 23b470d9f -- the total moves with angr master, the 4 does not. Guest offsets: 8 EAX, 16 EDX, 17 DH, 20 EBX, 28 EBP, 32 ESI, 36 EDI. Each block is str(stmt) over irsb.statements; the NEXT expression is not a statement, so it is not shown.

Before — the first two decode and read the wrong base register; the third does not decode:

angr/vex master `e3062871`

; 3b15  cmp dx, word ptr [di]   at 0xf9c98
; jumpkind Ijk_Boring, size 2 (the instruction is 2 bytes)
    ------ IMark(0xf9c98, 2, 0) ------
    t4 = GET:I16(offset=28)
    t5 = 16Uto32(t4)
    t3 = t5
    t2 = GET:I16(offset=16)
    t1 = LDle:I16(t3)
    t0 = Sub16(t2,t1)
    PUT(offset=40) = 0x00000005
    t6 = 16Uto32(t2)
    PUT(offset=44) = t6
    t7 = 16Uto32(t1)
    PUT(offset=48) = t7
    PUT(offset=52) = 0x00000000
    PUT(offset=68) = 0x000f9c9a

; 00b70500  add byte ptr [bx + 5], dh   at 0xf9c13
; jumpkind Ijk_Boring, size 4 (the instruction is 4 bytes)
    ------ IMark(0xf9c13, 4, 0) ------
    t6 = GET:I16(offset=36)
    t5 = Add16(t6,0x0005)
    t4 = t5
    t7 = 16Uto32(t4)
    t3 = t7
    t2 = LDle:I8(t3)
    t1 = GET:I8(offset=17)
    t0 = Add8(t2,t1)
    STle(t3) = t0
    PUT(offset=40) = 0x00000001
    t8 = 8Uto32(t2)
    PUT(offset=44) = t8
    t9 = 8Uto32(t1)
    PUT(offset=48) = t9
    PUT(offset=52) = 0x00000000
    PUT(offset=68) = 0x000f9c17

; 0300  add ax, word ptr [bx + si]   at 0xf9c11
; jumpkind Ijk_NoDecode, size 0 (the instruction is 2 bytes)

After — [di] reads EDI, [bx+5] reads EBX, and [bx+si] decodes as the sum of EBX and ESI:

with this change

; 3b15  cmp dx, word ptr [di]   at 0xf9c98
; jumpkind Ijk_Boring, size 2 (the instruction is 2 bytes)
    ------ IMark(0xf9c98, 2, 0) ------
    t5 = GET:I16(offset=36)
    t4 = 16Uto32(t5)
    t3 = t4
    t2 = GET:I16(offset=16)
    t1 = LDle:I16(t3)
    t0 = Sub16(t2,t1)
    PUT(offset=40) = 0x00000005
    t6 = 16Uto32(t2)
    PUT(offset=44) = t6
    t7 = 16Uto32(t1)
    PUT(offset=48) = t7
    PUT(offset=52) = 0x00000000
    PUT(offset=68) = 0x000f9c9a

; 00b70500  add byte ptr [bx + 5], dh   at 0xf9c13
; jumpkind Ijk_Boring, size 4 (the instruction is 4 bytes)
    ------ IMark(0xf9c13, 4, 0) ------
    t6 = GET:I16(offset=20)
    t5 = Add16(t6,0x0005)
    t4 = 16Uto32(t5)
    t3 = t4
    t2 = LDle:I8(t3)
    t1 = GET:I8(offset=17)
    t0 = Add8(t2,t1)
    STle(t3) = t0
    PUT(offset=40) = 0x00000001
    t7 = 8Uto32(t2)
    PUT(offset=44) = t7
    t8 = 8Uto32(t1)
    PUT(offset=48) = t8
    PUT(offset=52) = 0x00000000
    PUT(offset=68) = 0x000f9c17

; 0300  add ax, word ptr [bx + si]   at 0xf9c11
; jumpkind Ijk_Boring, size 2 (the instruction is 2 bytes)
    ------ IMark(0xf9c11, 2, 0) ------
    t6 = GET:I16(offset=32)
    t7 = GET:I16(offset=20)
    t5 = Add16(t7,t6)
    t4 = 16Uto32(t5)
    t3 = t4
    t2 = GET:I16(offset=8)
    t1 = LDle:I16(t3)
    t0 = Add16(t2,t1)
    PUT(offset=40) = 0x00000002
    t8 = 16Uto32(t2)
    PUT(offset=44) = t8
    t9 = 16Uto32(t1)
    PUT(offset=48) = t9
    PUT(offset=52) = 0x00000000
    PUT(offset=8) = t0
    PUT(offset=68) = 0x000f9c13

angr-bot · 2026-09-06T16:11:18Z

Corpus decompilation diffs can be found at angr/dec-snapshots@master...angr/pyvex_581

libVEX sends a ModRM byte through disAMode16 whenever the address size is 16 bits -- under a 0x67 prefix in 32-bit mode, and with no prefix at all when cr0.PE is clear -- and that function never applied the 16-bit r/m table. `67 8b 47 10`, which is mov 0x10(%bx),%eax, lifted to Ijk_Boring reading %di, and the four base+index forms did not decode at all. Bump the vex submodule to the rewrite of disAMode16 and lengthAMode16. tests/test_x86_addr16.py walks all 24 memory forms of the 16-bit ModRM byte and asserts each reads the registers the r/m table names, then covers the negative 8-bit displacement, the segment override, the immediate that lengthAMode locates, and the real-mode route in that needs no prefix. Five of its seven tests fail on master.

zardus force-pushed the feature/x86-addr16 branch from 4b142af to 41006c1 Compare September 8, 2026 09:24

zardus force-pushed the feature/x86-addr16 branch from 41006c1 to 859613b Compare September 9, 2026 04:57

Navigation Menu

Repository navigation

Lifting: Decode x86 16-bit addressing modes correctly - #581

Lifting: Decode x86 16-bit addressing modes correctly#581
zardus wants to merge 1 commit into
masterfrom
feature/x86-addr16

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Uh oh!

Problem

Root cause

Fix

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants