Incorrect stack adjustment size when decoding "push r16" in x86 (32bit) mode

[xusheng6]

For instruciton "push bx" (6653), when xed decodes it in 32bit mode, the stack adjustment is reported as 32bits (4 bytes).

See ASZ0=32 in the following output:

$ obj/wkit/examples/obj/xed-ex1 6653
Attempting to decode: 66 53 
iclass PUSH category PUSH   ISA-extension BASE  ISA-set I86
instruction-length 2
operand-width 16
effective-operand-width 16
effective-address-width 32
stack-address-width 32
iform-enum-name PUSH_GPRv_50
iform-enum-name-dispatch (zero based) 4
iclass-max-iform-dispatch 11
Nominal opcode position 1
Nominal opcode 0x53
Operands
#   TYPE               DETAILS        VIS  RW       OC2 BITS BYTES NELEM ELEMSZ   ELEMTYPE   REGCLASS
#   ====               =======        ===  ==       === ==== ===== ===== ======   ========   ========
0   REG0               REG0=BX   EXPLICIT   R         V   16     2     1     16        INT        GPR
1   REG1        REG1=STACKPUSH SUPPRESSED  RW       SPW   16     2     1     16        INT     PSEUDO
2   MEM0           (see below) SUPPRESSED   W       SPW   16     2     1     16        INT    INVALID
3  BASE0             BASE0=ESP SUPPRESSED  RW       SSZ   32     4     1     32        INT        GPR
Memory Operands
  0 written SEG= SS BASE= ESP/GPR  ASZ0=32
  MemopBytes = 2
ATTRIBUTES: FIXED_BASE0 SCALABLE STACKPUSH0 
66-OSZ PREFIX
ANY 66 PREFIX
Number of legacy prefixes: 1 
ISA SET: [I86]

However, this is wrong. I used a debugger to verify the stack adjustment is actually 2 bytes, not 4 bytes. This is now causing the downstream lifting error in the Binary NInja x86 arch plugin: https://github.com/Vector35/binaryninja-api/issues/4028.

FYI, this is how we are handling the lifting of the push instruction: https://github.com/Vector35/arch-x86/blob/ce54c270537a36255608cd1f67df64f971558d0e/il.cpp#L2829-L2844. xed_decoded_inst_get_memop_address_width is reporting 32 where I believe it should return 16

[kkhalail]

The output shows that the the RSP width (stack adjustment pointer) is 4 bytes determined by the operating mode, and then rightly shows that the actual operation width is 2-bytes, which considers the mode and prefixes ... We believe that in 32-bit mode, the RSP is 4-bytes, and even if < 4-byte operations are being done via PUSH/POP, the RSP (or ESP rather) is a 4-byte value. When going to memory to push/pop the value in question, the address will be a full 4-byte value, MEM[RSP[31:0]] when the stack is adjusted by two bytes. Therefore, we don’t see anything wrong with the current output, per-se.

[xusheng6]

Yeah it seems to be an error on our end. I have fixed the code by using xed_decoded_inst_get_memory_operand_length to get the size of MemopBytes: https://github.com/Vector35/binaryninja-api/compare/351238c300838d39ca896eeb7d180a35cc12998f...a9a3f1ab55e18b345be37001c66cfa9455494b4a#diff-b8cfd89c1f7cb01bf7d9746fa11621207a6f623d00691a11976d22db1de1bfd9