mirror of
https://github.com/FEX-Emu/FEX.git
synced 2026-10-27 18:00:48 +01:00
OpcodeDispatcher: Fixes SHLD by 16 behaviour
We were assuming that SHLD undefined behaviour matches SHL, but the specification actually changes a `ge` comparison to `gt`, which means a shift of 16 isn't UB! Thanks to the impeccable @OFFTKP in #5842 for bringing this up as it took a bit for me to figure out what was actually wrong here. I modified their unittest to cover more just to ensure we don't break it.
This commit is contained in:
4 files changed
+60
-4
No files matched your search
@@ -1526,7 +1526,7 @@ void OpDispatchBuilder::SHLDImmediateOp(OpcodeArgs) {
|
||||
Res = _Extr(OpSizeFromSrc(Op), Dest, Src, Size - Shift);
|
||||
}
|
||||
|
||||
CalculateFlags_ShiftLeftImmediate(OpSizeFromSrc(Op), Res, Dest, Shift);
|
||||
CalculateFlags_ShiftLeftImmediate(OpSizeFromSrc(Op), Res, Dest, Shift, true);
|
||||
CalculateDeferredFlags();
|
||||
StoreResultGPR(Op, Res);
|
||||
} else if (Shift == 0 && Size == 32) {
|
||||
|
||||
@@ -2375,7 +2375,7 @@ private:
|
||||
void CalculateFlags_MUL(IR::OpSize SrcSize, Ref Res, Ref High);
|
||||
void CalculateFlags_UMUL(Ref High);
|
||||
void CalculateFlags_Logical(IR::OpSize SrcSize, Ref Res);
|
||||
void CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift);
|
||||
void CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift, bool DoubleWide = false);
|
||||
void CalculateFlags_ShiftRightImmediate(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift);
|
||||
void CalculateFlags_ShiftRightDoubleImmediate(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift);
|
||||
void CalculateFlags_ShiftRightImmediateCommon(IR::OpSize SrcSize, Ref Res, Ref Src1, uint64_t Shift);
|
||||
|
||||
@@ -432,7 +432,7 @@ void OpDispatchBuilder::CalculateFlags_Logical(IR::OpSize SrcSize, Ref Res) {
|
||||
SetNZP_ZeroCV(SrcSize, Res);
|
||||
}
|
||||
|
||||
void OpDispatchBuilder::CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Ref UnmaskedRes, Ref Src1, uint64_t Shift) {
|
||||
void OpDispatchBuilder::CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Ref UnmaskedRes, Ref Src1, uint64_t Shift, bool DoubleWide) {
|
||||
// No flags changed if shift is zero
|
||||
if (Shift == 0) {
|
||||
return;
|
||||
@@ -447,8 +447,12 @@ void OpDispatchBuilder::CalculateFlags_ShiftLeftImmediate(IR::OpSize SrcSize, Re
|
||||
// Extract the last bit shifted in to CF. Shift is already masked, but for
|
||||
// 8/16-bit it might be >= SrcSizeBits, in which case CF is cleared. There's
|
||||
// nothing to do in that case since we already cleared CF above.
|
||||
//
|
||||
// - Double-wide shift has UB when shift is GREATER-THAN operand.
|
||||
// - Single-wide shift has UB when shift is GREATER-THAN-EQUAL operand.
|
||||
const auto SrcSizeBits = IR::OpSizeAsBits(SrcSize);
|
||||
if (Shift < SrcSizeBits) {
|
||||
const bool ShouldSetCF = DoubleWide ? (Shift <= SrcSizeBits) : (Shift < SrcSizeBits);
|
||||
if (ShouldSetCF) {
|
||||
SetCFDirect(Src1, SrcSizeBits - Shift, true);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
%ifdef CONFIG
|
||||
{
|
||||
"RegData": {
|
||||
"RAX": "1",
|
||||
"R15": "1",
|
||||
"R14": "1",
|
||||
"R13": "1",
|
||||
"R12": "1"
|
||||
}
|
||||
}
|
||||
%endif
|
||||
|
||||
; FEX-Emu had a bug where operand-sized shifts with SHLD/SHRD wasn't matching behaviour.
|
||||
; We were expecting SAL/SAR/SHL/SHR undefined behaviour semantics but a `ge` comparison changes to `gt`.
|
||||
|
||||
; Test with shld and immediate
|
||||
mov eax, 1
|
||||
mov edx, 0xbee8
|
||||
mov rcx, 0
|
||||
clc
|
||||
shld ax, dx, 16
|
||||
setc cl
|
||||
mov r15, rcx
|
||||
|
||||
; Test with shld and CL
|
||||
mov eax, 1
|
||||
mov edx, 0xbee8
|
||||
mov rcx, 16
|
||||
clc
|
||||
shld ax, dx, cl
|
||||
setc cl
|
||||
mov r14, rcx
|
||||
|
||||
; Test with shrd and immediate
|
||||
mov eax, 0x8eeb
|
||||
mov edx, 1
|
||||
mov rcx, 0
|
||||
clc
|
||||
shrd ax, dx, 16
|
||||
setc cl
|
||||
mov r13, rcx
|
||||
|
||||
; Test with shrd and CL
|
||||
mov eax, 0x8eeb
|
||||
mov edx, 1
|
||||
mov rcx, 16
|
||||
clc
|
||||
shrd ax, dx, cl
|
||||
setc cl
|
||||
mov r12, rcx
|
||||
|
||||
hlt
|
||||
Reference in new issue
Block a user