search

nepx / halfix

x86 PC emulator that runs both natively and in the browser, via WebAssembly
https://nepx.github.io/halfix-demo/
GNU General Public License v3.0
669 stars 86 forks source link

Wrong CF value for corner-case shift #7

Closed DCNick3 closed 4 years ago

DCNick3 commented 4 years ago

Assembly snippet:

mov $0x88, %al
sar $0x09, %al

Set CF on real hardware (and as per intel manual), but your emulator resets it.

Screenshot_20200706_152154

I haven't dug deep into the flags logic for this, so I'm not sure how to fix this problem.

nepx commented 4 years ago

Hi, thanks for the bug report!

The issue was that cpu.lop1 (last operand 1, used for flags calculation) wasn't being sign extended. The actual sar operation uses a sign-extended value [(int8_t)((((int8_t)(*dest)) >> cpu.lop2)); -- really ugly, but gets the job done], but the value used during flags calculation, cpu.lop1 (last operand 1), wasn't being sign extended. shr and sar use the same flags calculation logic internally, an unsigned shift (cpu.lop1 >> (cpu.lop2 - 1) & 1;, in eflags.c), so sign extending cpu.lop1 would be absolutely necessary in this case.

The bug should be fixed on both 8- and 16-bit shifts: 

mov al, 0x88
sar al, 0x09

and 

mov ax, 0x8888
sar ax, 0x11

both set the carry flag now.