Page MenuHomePhabricator

Correctly assemble unsized call instructions.
Needs ReviewPublic

Authored by jrmuizel on Jul 21 2014, 7:07 PM.


Group Reviewers

This adds a new kind X86MemOperand that will only match MemOperands with no
size if the default size (i.e. mode) matches the size of the memory operand.
This keeps us from matching the 16 bit call instruction when we're in 32 bit

Diff Detail

Event Timeline

jrmuizel updated this revision to Diff 11735.Jul 21 2014, 7:07 PM
jrmuizel retitled this revision from to Correctly assemble unsized call instructions..
jrmuizel updated this object.
jrmuizel edited the test plan for this revision. (Show Details)
jrmuizel added a subscriber: Unknown Object (MLST).
rnk added a subscriber: rnk.Jul 28 2014, 4:09 PM

Oops, I wrote these comments and never sent them. :(


I assume this is intended to be uncommented?


Returning true for Size == 0 seems suspect to me. What tests fail if we remove that clause? My guess is that we start failing to match instructions like 'mov %eax, (%ebx)', because now isMem32 will return false for '(%ebx)'.

I think maybe the real fix is to remove this clause, and then add an 'isMemUnsized()' predicate, which the asm matcher uses to figure out memory operand sizes when the size is implied by the at&t size suffix (movll, movw) or the size of another operand (mov %eax, (%ebx)).


What do we do for other instructions that take a single memory operand, like 'inc' and 'dec'?

jrmuizel added inline comments.Jul 28 2014, 4:43 PM

Correct. We start to fail to match mov eax, [ebx].

The asm matcher currently only looks at one operand at a time, from left to right so I'm not sure how we'd do the matching for something like mov [ebx], eax?


Currently they probably end up as incw, decw. mov [eax], 1 ends up as mov word ptr [eax] 1. In MSVC they all become 'byte pointer' accesses, other assemblers including ml seem to reject them.

lhames added a subscriber: lhames.Jan 13 2016, 3:20 PM

Ping - what's the state of this? Can we close it?