Skip to content

x86: fix xadd reg,reg writeback order for the same-register case - #85

Open
DORA-B wants to merge 1 commit into
angr:masterfrom
DORA-B:fix/x86_xadd_same_reg
Open

x86: fix xadd reg,reg writeback order for the same-register case#85
DORA-B wants to merge 1 commit into
angr:masterfrom
DORA-B:fix/x86_xadd_same_reg

Conversation

@DORA-B

@DORA-B DORA-B commented Jul 26, 2026

Copy link
Copy Markdown

dis_xadd_G_E wrote E (the sum) before G (the old E value) in the reg-reg case. When the two operands are the same register (xadd %eax,%eax), the second write (G := old E) then clobbered the sum, so the register kept its original value instead of 2*eax. The Intel semantics are TEMP := SRC+DEST; SRC := DEST; DEST := TEMP, i.e. the destination write is last and must win.

Swap the two writes so G (old E) is written first and E (the sum) last, matching the amd64 front-end (which already orders them G-then-E and is unaffected). For distinct registers the order is irrelevant, so only the degenerate same-register case changes.

Impact

Silent wrong result (no decode error, no exception) for the degenerate
same-register form. Concretely xadd %eax,%eax leaves EAX unchanged instead of
doubling it; any following use of EAX or a flag-consuming branch then diverges.
Distinct-register xadd is unaffected (write order is irrelevant there).

Reproduction (before the fix)

import angr, claripy
# 0F C1 C0 = xadd %eax, %eax
st = angr.load_shellcode(b'\x0f\xc1\xc0', 'x86').factory.blank_state(addr=0)
st.regs.eax = claripy.BVV(5, 32)
s = st.step(num_inst=1).successors[0]
print(s.solver.eval(s.regs.eax))     # angr: 5   ; hardware: 10

Root cause

dis_xadd_G_E, reg-reg case (epartIsReg(rm)):

assign( tmpd,  getIReg(sz, eregOfRM(rm)) );          // tmpd  = old E
assign( tmpt0, getIReg(sz, gregOfRM(rm)) );          // tmpt0 = old G
assign( tmpt1, binop(mkSizedOp(ty,Iop_Add8),
                     mkexpr(tmpd), mkexpr(tmpt0)) );  // tmpt1 = sum
setFlags_DEP1_DEP2( Iop_Add8, tmpd, tmpt0, ty );
putIReg(sz, eregOfRM(rm), mkexpr(tmpt1));            // E := sum       <-- written first
putIReg(sz, gregOfRM(rm), mkexpr(tmpd));             // G := old E     <-- clobbers sum when E==G

Both source reads are correctly latched into temps, so the flags and tmpt1 are
right; only the order of the two writes is wrong. When E == G, the final
G := old E wins and the sum is lost. The amd64 front-end's dis_xadd_G_E
already writes G first, then E, and is unaffected.

dis_xadd_G_E wrote E (the sum) before G (the old E value) in the reg-reg case.
When the two operands are the same register (`xadd %eax,%eax`), the second
write (G := old E) then clobbered the sum, so the register kept its original
value instead of 2*eax. The Intel semantics are TEMP := SRC+DEST; SRC := DEST;
DEST := TEMP, i.e. the destination write is last and must win.

Swap the two writes so G (old E) is written first and E (the sum) last, matching
the amd64 front-end (which already orders them G-then-E and is unaffected). For
distinct registers the order is irrelevant, so only the degenerate same-register
case changes.
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.

1 participant